Odinmonkey broke powerpc (and probably all non-ion platforms)




6 years ago
6 years ago


(Reporter: gaston, Assigned: gaston)


Firefox Tracking Flags

(Not tracked)



(1 attachment, 1 obsolete attachment)



6 years ago
Probably because AsmJS.cpp is used/compiled inconditionally, while it should be within some ENABLE_ION in js/src/Makefile.in :

In file included from /home/landry/src/mozilla-central/js/src/ion/AsmJSModule.h:12:0,
                 from /home/landry/src/mozilla-central/js/src/ion/AsmJS.cpp:11:
/home/landry/src/mozilla-central/js/src/ion/RegisterSets.h:461:2: error: #error "Bad architecture"
/home/landry/src/mozilla-central/js/src/ion/RegisterSets.h:497:2: error: #error "Bad architecture"
/home/landry/src/mozilla-central/js/src/ion/RegisterSets.h:547:2: error: #error "Bad architecture"
In file included from /home/landry/src/mozilla-central/js/src/ion/RegisterSets.h:11:0,
                 from /home/landry/src/mozilla-central/js/src/ion/AsmJSModule.h:12,
                 from /home/landry/src/mozilla-central/js/src/ion/AsmJS.cpp:11:
/home/landry/src/mozilla-central/js/src/ion/Registers.h:31:13: error: 'Registers' does not name a type
/home/landry/src/mozilla-central/js/src/ion/Registers.h:32:13: error: 'Codes' does not name a type
/home/landry/src/mozilla-central/js/src/ion/Registers.h:33:13: error: 'Registers' in namespace 'js::ion' does not name a type
/home/landry/src/mozilla-central/js/src/ion/Registers.h:34:5: error: 'Code' does not name a type
/home/landry/src/mozilla-central/js/src/ion/Registers.h:41:5: error: 'Code' does not name a type


6 years ago
Blocks: 840282

Comment 1

6 years ago
Posted patch wip patch (obsolete) — Splinter Review
This wip patch allows me to build js/src. Not sure it's the best way to go...

Building m-c to check the whole thing.
Assignee: general → landry
Attachment #725745 - Flags: feedback?(luke)
My goal with having AsmJS.cpp always be built is that we can always call CompileAsmJS from the parser so that it can always issue a warning when "use asm" is present.  The goal is non-silent failure.

Instead of the patch you posted, can you instead put the relevant #includes inside AsmJS.cpp inside the existing #ifdef JS_ASMJS?  (JS_ASMJS is a restriction of JS_ION; see AsmJS.h.)

Comment 3

6 years ago
This one allows js to build, and all jsapi-tests but one pass (but it also failed before) so this should be better.

Not really sure where to #ifdef out stuff in AsmJSLink.cpp.....
Attachment #725745 - Attachment is obsolete: true
Attachment #725745 - Flags: feedback?(luke)
Attachment #725757 - Flags: review?(luke)
I realized, looking at this patch that the way I have it set up will just continually break powerpc and you'll keep having to file these bugs.  I think your first approach was the right way to go; sorry for the misdirection!  Two nits on that patch: switch #if ENABLE_ION for #ifdef JS_ION and I think you'll want to implement isAsmJSCompilationAvailable() in TestingFunctions.cpp #ifndef JS_ION so that isAsmJSCompilationAvailable() is always defined and usable.
Could you confirm that this cset fixes --disable-ion for you?

Comment 6

6 years ago
I had a slightly similar cset in my mq, built it last night but ffx badly segfaults at startup. Of course trying to load the core in gdb or run firefox in gdb blows gdb too, so it's hardly debuggable. The js engine itself should be fine, since all jsapi-tests but one passes fine - so the crash might be unrelated.

I also have patches from bugs 849253 and 817042 in my queue (the former is also needed to fix a build failure), so i'll retry tonight with a clean tree and only 849253 on top of inbound tip.
I am able to build on ppc64 with these changes (m-c from the 23'rd) + 851859 , 849253(+ extra fixes I mention on this bug that didn't make the commit) and 846986

Firefox starts fine and I can view an initial page without issues.  Firefox often hangs when I try to view a second or third page but I think that is unrelated to Odinmonkey. I will kick off an updated build on my ppc32 machine where things tend to work better.
The ppc32 build works on m-c with your patch(with the same other fixes applied as above) and doesn't hang.
Comment on attachment 725757 [details] [diff] [review]
#ifdef out ion includes / asmjs code

clearing, since I think this patch is out of date
Attachment #725757 - Flags: review?(luke)

Comment 10

6 years ago
this got fixed by the no bug cset... no idea which target/milestone this was though
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.