The default bug view has changed. See this FAQ.

Jaegermonkey Sparc back-end broken on sparc64

RESOLVED FIXED in mozilla13

Status

()

Core
JavaScript Engine
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: gaston, Assigned: gaston)

Tracking

Trunk
mozilla13
Sun
OpenBSD
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: wanted-standalone-js)

Attachments

(1 attachment, 4 obsolete attachments)

(Assignee)

Description

6 years ago
Created attachment 528948 [details] [diff] [review]
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.
(Assignee)

Updated

6 years ago
Blocks: 650665

Comment 1

6 years ago
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.
(Assignee)

Updated

6 years ago
Attachment #528948 - Attachment is patch: true
Attachment #528948 - Attachment mime type: text/x-patch → text/plain
(Assignee)

Comment 2

6 years ago
(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 :)

Updated

6 years ago
Whiteboard: wanted-standalone-js
(Assignee)

Comment 3

6 years ago
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.
(Assignee)

Comment 4

6 years ago
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....
(Assignee)

Comment 5

6 years ago
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

6 years ago
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.
(Assignee)

Comment 7

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

Comment 8

6 years ago
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.
(Assignee)

Comment 9

6 years ago
Created attachment 540254 [details] [diff] [review]
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...
Attachment #528948 - Attachment is obsolete: true
Attachment #540254 - Flags: review?(wes)

Comment 10

6 years ago
$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.
(Assignee)

Comment 11

6 years ago
Created attachment 551303 [details] [diff] [review]
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...
Attachment #540254 - Attachment is obsolete: true
Attachment #540254 - Flags: review?(wes)
Attachment #551303 - Flags: review?(wes)
(Assignee)

Comment 12

6 years ago
Created attachment 555540 [details] [diff] [review]
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...)
Attachment #551303 - Attachment is obsolete: true
Attachment #551303 - Flags: review?(wes)
Attachment #555540 - Flags: review?(leon.sha)
(Assignee)

Comment 13

5 years ago
Created attachment 589964 [details] [diff] [review]
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 ?
Attachment #555540 - Attachment is obsolete: true
Attachment #555540 - Flags: review?(leon.sha)
Attachment #589964 - Flags: review?(leon.sha)

Updated

5 years ago
Attachment #589964 - Flags: review?(leon.sha) → review+
(Assignee)

Updated

5 years ago
Keywords: checkin-needed

Updated

5 years ago
Assignee: general → landry
https://hg.mozilla.org/integration/mozilla-inbound/rev/a2ce6d40b9b3
Whiteboard: wanted-standalone-js → [inbound]wanted-standalone-js

Updated

5 years ago
Keywords: checkin-needed
Whiteboard: [inbound]wanted-standalone-js → wanted-standalone-js
Target Milestone: --- → mozilla13
https://hg.mozilla.org/mozilla-central/rev/a2ce6d40b9b3
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
(Assignee)

Comment 16

5 years ago
I wouldnt mark the bug as 'fixed' since jm is still broken at runtime on sparc64, but oh well :)
You need to log in before you can comment on or make changes to this bug.