653551 - Jaegermonkey Sparc back-end broken on sparc64

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Landry Breuil (:gaston)]

Attachment: dont enable yarr/jit on sparc64

The last tm->m-c merge broke OpenBSD/sparc64 builds, due to the landing of https://bugzilla.mozilla.org/show_bug.cgi?id=610323. Now by default ENABLE_TRACEJIT is set on sparc64, and the build fails later on with pain and prejudice, see http://buildbot.rhaalovely.net/builders/mozilla-central-sparc64/builds/1/steps/build/logs/stdio.

Previously i only needed to set --disable-tracejit in .mozconfig, now i also need a hack in wtf/Platform.h (see attached) to avoid ENABLE_JIT & ENABLE_YARR_JIT being set to 1.

I'm all willing to test diffs/patches to fix that issue, and if finally the switch to 128-bits jsval is done we might finally have firefox working on sparc64.

btw you might want to recheck the ifeq entering in assembler/ in js/src/Makefile.in, the merge has reset it to ifeq (,$(filter arm% %86 x86_64,$(TARGET_CPU))) while tm only had ifeq (,$(filter-out powerpc,$(TARGET_CPU))) if i'm not mistaked. Dunno if that was the intent.

Comment 1

[Leon Sha]

The patch look OK for me.
I noticed that for yarr in Makefile.in, the list has been changed from black list to white list. I'll try to fix this.

Comment 2

[Landry Breuil (:gaston)]

(In reply to comment #1)
> The patch look OK for me.

Well rather than just disabling it for sparc64, i'd prefer making it work, but i'm not the one able to write such code. I can only test diffs :)

Comment 3

[Landry Breuil (:gaston)]

The last commits to wtf/Platform.h (likely http://hg.mozilla.org/mozilla-central/rev/cc36a234d0d6) renders the patch useless, as it seems now JIT is now only enabled by default on x86 and 86_64/mips/arm and not sparc. I'll just need to recheck things in my builds on sparc64.

Comment 4

[Landry Breuil (:gaston)]

Hm no, this still blows, as can be seen on http://buildbot.rhaalovely.net/builders/mozilla-central-sparc64/builds/56/steps/build/logs/stdio. I'll need to find how to enable the sparc backend only on sparc32....

Comment 5

[Landry Breuil (:gaston)]

Rechecking, but it seems http://hg.mozilla.org/mozilla-central/rev/482d2f967ed9 enables assembler for sparc64 too, whereas it shouldn't match.. testing that now.
Or i should find another way to disable jit.

Comment 6

[Wesley W. Garland]

Landry;

AFAIK Sparc64 support isn't being actively developed by anybody (other than Andrew P.), and the JITs only support the V8+ arch  (32-bit words but sparc v9 instructions).  

So - "Sparc" in SpiderMonkey seems to pretty much mean "sparc v8+". Once upon a time there was differentiation between sparc and ultrasparc, but this only allowed support for v8 instructions for jslock.c, and did not work with the JITs.  v8 is pretty much dead, Sun hasn't shipped any sun4c/sun4m machines in like 20 years.

So - this lack of 32/64-bit differentiation means that you're likely to bump into these issues until differentiation is made in the autoconf code and pushed forward.  If you have the wherewithal to write such a patch, I would be happy to test it on Solaris with GCC for you.

Comment 7

[Landry Breuil (:gaston)]

Ugh. I think js/src/Makefile.in line 1055 bites me hard. Unconditionally adding -DENABLE_ASSEMBLER=1 -DENABLE_JIT=1 .... testing yet another fix.

Comment 8

[Landry Breuil (:gaston)]

making -DENABLE_ASSEMBLER=1 -DENABLE_JIT=1 conditional fails later on with the following :

js/src/yarr/wtfbridge.h:268: error: ISO C++ forbids declaration of 'ExecutableAllocator' with no type

introduced by http://hg.mozilla.org/mozilla-central/rev/1f0635e935d9

ExecutableAllocator is defined within ENABLE_ASSEMBLER.... that stuff is too intermixed. Seems noone tried building with jit disabled since a while.

Comment 9

[Landry Breuil (:gaston)]

Attachment: Fix build on sparc64

Here's a patch that at least fixes the build for me on OpenBSD/sparc64 (make package still fails because it tries to run some js code in GENERATE_CACHE iirc, which sigbuses, but that's another story)
it does three things :
- enable tracejit/methodjit only on sparc (v8+?) in js/src/configure.in and not sparc64
- fix js/src/jsregexpinlines.h to use codeblock only if ENABLE_YARR_JIT is defined, as it's declared as is in the class
- and finally, to be discussed, add -DENABLE_JIT=1 to CPPFLAGS only if ENABLE_TRACEJIT is set.

For the latter, i'd like first to understand why that ugly CPPFLAGS hacky tweak is done.. and then, if the 'fix' has to be applied for sparc64 only, for !ENABLE_TRACEJIT only, or if ENABLE_METHODJIT has to be there too.

Thanks for feedback/comment on the logic...

Comment 10

[Leon Sha]

$target in configure.in comes from config.guess. For some 64 bit OS on Sparc, the $target is not sparc64-* for sure. Something like "if test ! "$HAVE_64BIT_OS"; then" would be better.

Comment 11

[Landry Breuil (:gaston)]

Attachment: Fix sparc64

Not sure about that, you know probably better than me... but here's another patch using that HAVE_64BIT_OS test, and removes the ENABLE_YARR_JIT #ifdef since it's already fixed in m-c (but not in aurora)

I'd love to get that tested on solaris to see if it doesnt change the behaviour there...

Comment 12

[Landry Breuil (:gaston)]

Attachment: Don't enable jits on sparc64

Now that bug 670719 has landed, js/src/Makefile.in part of the patch can go away. Still left is the HAVE_64BIT_OS chunk... doing builds this night, will confirm later if this is now enough to let m-c build on sparc64 (even if it's still broken at cache generation/runtime/js...)

Comment 13

[Landry Breuil (:gaston)]

Attachment: Don't enable JITs on sparc64

And here's the patch i'm using now to build m-c on sparc64, even if it's useless... any chance this can be considered ?

Comment 14

[Makoto Kato [:m_kato]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a2ce6d40b9b3

Comment 15

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/a2ce6d40b9b3

Comment 16

[Landry Breuil (:gaston)]

I wouldnt mark the bug as 'fixed' since jm is still broken at runtime on sparc64, but oh well :)