The interpreter engine for the core JavaScript language, independent of the browser's object model. File ONLY core JavaScript language bugs in this category. For bugs involving browser objects such as "window" and "document", use the "DOM" component. For bugs involving calls between JavaScript and C++, use the "XPConnect" component.

Since a while (sorry, cant exactly bisect where, maybe last yarr update in https://bugzilla.mozilla.org/show_bug.cgi?id=625600 ?), mozilla-central doesn't build on OpenBSD/powerpc (and maybe other oses..), because jit is now required and no jit exists on ppc. https://bugzilla.mozilla.org/show_bug.cgi?id=638056 is back... Firefox 5 works.
First, i need a patch for js/src/ctypes/libffi/configure, but that one could be pushed upstream first...
Without any configure option or --disable-tracejit --disable-methodjit, the build fails with :
error: #error "The cacheFlush support is missing on this platform."
If i provide an empty cacheFlush() implem for WTF_CPU_PPC, it fails with :
error: #error "The MacroAssembler is not supported on this platform."
probably because ENABLE_JIT=1 is inconditionally added to CPPFLAGS in js/src/Makefile.in, and it's now a requirement. Nanojit has no support for ppc.
And finally, if i make the ENABLE_JIT=1 conditional to ENABLE_TRACEJIT, the build fails at :
/home/landry/src/mozilla-central/build/js/src/jsregexpinlines.h:494:
error: 'codeBlock' was not declared in this scope
because codeBlock is used outside ENABLE_YARR_JIT.
If i patch all that, the build finally painfully succeeds, but make package fails, and the binary produced doesnt work. I've yet to debug it...

As a sidenote, all those issues are probably present in mozilla-aurora, since i was already having similar issues before the last branch move... and i've yet to check mozilla-beta too.
Things are not in good shape for powerpc :(

(In reply to comment #2)
> Comment on attachment 545222[details][diff][review] [review]
> recognize powerpc-*-openbsd* as powerpc-*-freebsd* in libffi
>
> Stealing. Can you push this yourself?
Nope, i only have a l1 account.. and maybe it should go into configure.ac too, no ? Unless we don't plan to regenerate configure with autoconf...

Comment on attachment 545225[details][diff][review]
only add ENABLE_JIT=1 if ENABLE_TRACEJIT is set
Review of attachment 545225[details][diff][review]:
-----------------------------------------------------------------
::: js/src/Makefile.in
@@ +1052,5 @@
>
> # Needed to "configure" it correctly. Unfortunately these
> # flags wind up being applied to all code in js/src, not just
> # the code in js/src/assembler.
> +ifdef ENABLE_TRACEJIT
This might not be the right test, since afaik, methodjit also uses js/src/assembler.
Note that considering the comment, it might be worth adding the flags to js/src/assembler only. I can help with that if necessary.

(In reply to comment #9)
> Comment on attachment 545225[details][diff][review] [review]
> only add ENABLE_JIT=1 if ENABLE_TRACEJIT is set
>
> Review of attachment 545225[details][diff][review] [review]:
> -----------------------------------------------------------------
>
> ::: js/src/Makefile.in
> @@ +1052,5 @@
> >
> > # Needed to "configure" it correctly. Unfortunately these
> > # flags wind up being applied to all code in js/src, not just
> > # the code in js/src/assembler.
> > +ifdef ENABLE_TRACEJIT
>
> This might not be the right test, since afaik, methodjit also uses
> js/src/assembler.
Yes, I think this should go by platform, or by having any of the 3 JITs (trace, method, regex) enabled.
> Note that considering the comment, it might be worth adding the flags to
> js/src/assembler only. I can help with that if necessary.

Comment on attachment 545224[details][diff][review]
provide an empty implem for cacheFlush on ppc
Actually, I probably can't give an r+ on this without knowing something about PPC -- are you sure that you don't have to do anything to maintain coherence? I always thought x86 was fairly "special" in that way.

Another data point .. mozilla-beta is fine on ppc without those patches (only the libffi one is needed there)
That's probably broken in mozilla-aurora too given that the migration was recent.. at least that narrows the window.

As i finally found out, m-c packages and runs fine on openbsd/macppc with the patch from https://bugzilla.mozilla.org/show_bug.cgi?id=650749, and the attached patches here : the libffi fix, which gal already reviewed (but i cant commit it myself), and the #ifdef ENABLE_TRACEJIT one, for which i'm awaiting feedback on the alternative approach i proposed in comment #13. It finally seems the empty cacheFlush() implem is not needed.
I'd be really happy if all those pieces can get commited before the next m-c->m-a shift, that'll allow me to concentrate on other openbsd patches.
Mandatory screenshot : http://rhaalovely.net/~landry/stuff/ffx-8.0a1-openbsd-macppc.png

(In reply to Landry Breuil from comment #20)
> As i finally found out, m-c packages and runs fine on openbsd/macppc with
> the patch from https://bugzilla.mozilla.org/show_bug.cgi?id=650749, and the
> attached patches here : the libffi fix, which gal already reviewed (but i
> cant commit it myself),
For this kind of thing, you can add the checkin-needed keyword. Don't worry about it on this one, I'll just land it.

Created attachment 553404[details][diff][review]
Only add -DENABLE_JIT=1 when a jit is effectively enabled
I ended up only doing a test on ENABLE_TRACEJIT/ENABLE_METHODJIT, and moved the regex jit part to where the arch test is already done. This is untested, but you get the idea. If needed, i can push it to try-server, and i'll give it a roll on my sparc64/macppc buildslaves.

Comment on attachment 545224[details][diff][review]
provide an empty implem for cacheFlush on ppc
That patch seems now unneeded, no cacheFlush implem is needed on ppc with the other patches. I'll deobsolete if needed..

Thinking a bit more about this... wouldn't it be better/clearer if CXXFLAGS+= -DENABLE_JIT=1 was moved to the existing ifdef ENABLE_TRACEJIT/ENABLE_METHODJIT blocks like it's done for regex jit, instead of doing a specific ifdef block for that ? The end result will be the same... what do you thing about it ?
As for the -DENABLE_ASSEMBLER=1, i still don't understand why it's needed inconditionally.

Created attachment 554619[details][diff][review]
Only add -DENABLE_JIT=1 to CXXFLAGS if a jit is enabled
Same patch in the concept, moving the CXXFLAGS addition to existing ifdef blocks for the sake of clarity/simplicity.
Dunno if i can carry the r+...

(In reply to Landry Breuil from comment #30)
> Created attachment 554619[details][diff][review]
> Only add -DENABLE_JIT=1 to CXXFLAGS if a jit is enabled
>
> Same patch in the concept, moving the CXXFLAGS addition to existing ifdef
> blocks for the sake of clarity/simplicity.
>
> Dunno if i can carry the r+...
With this one you're effectively adding -DENABLE_JIT=1 twice for the tier-1 architectures...
I think I prefer the old one, though it could be simpler with something like
ifneq (,$(ENABLE_TRACEJIT)$(ENABLE_METHODJIT))
CXXFLAGS += -DENABLE_JIT=1
endif
though that'd still add it twice for other architectures, since there's also the yarr one. BTW, why do you need that one?

(In reply to Mike Hommey [:glandium] from comment #31)
> (In reply to Landry Breuil from comment #30)
> > Created attachment 554619[details][diff][review]
> > Only add -DENABLE_JIT=1 to CXXFLAGS if a jit is enabled
> >
> > Same patch in the concept, moving the CXXFLAGS addition to existing ifdef
> > blocks for the sake of clarity/simplicity.
> >
> > Dunno if i can carry the r+...
>
> With this one you're effectively adding -DENABLE_JIT=1 twice for the tier-1
> architectures...
Indeed, that should be avoided
> I think I prefer the old one, though it could be simpler with something like
> ifneq (,$(ENABLE_TRACEJIT)$(ENABLE_METHODJIT))
> CXXFLAGS += -DENABLE_JIT=1
> endif
>
> though that'd still add it twice for other architectures, since there's also
> the yarr one. BTW, why do you need that one?
I have no idea how they are intermixed, i've just done it because i was told to :)
Setting checkin-needed for the previous r+'ed patch..

(In reply to Landry Breuil from comment #34)
> I have no idea how they are intermixed, i've just done it because i was told
> to :)
I would understand if it was set in the else part of the if/else, but at that position, it doesn't make much sense to me. You're effectively adding ENABLE_JIT=1 to !arm, !sparc, !x86.

Comment on attachment 649781[details][diff][review]
properly land as a separate patch + configure.ac fix
Not sure I can review this (I think khuey is you best bet). Can you also regenerate configure? When I do I see lots of dummy changes like:
-#line 10756 "configure"
+#line 10757 "configure"

Comment on attachment 649790[details][diff][review]
properly land as a separate patch + configure.ac fix + regen configure
I think you forgot to update js/src/ctypes/patches-libffi/05-bug-670719. BTW, I wonder if the best thing to is to just not include configure in it as it is a generated file.

(In reply to Rafael Ávila de Espíndola (:espindola) from comment #44)
> Comment on attachment 649790[details][diff][review]
> properly land as a separate patch + configure.ac fix + regen configure
>
> I think you forgot to update js/src/ctypes/patches-libffi/05-bug-670719.
> BTW, I wonder if the best thing to is to just not include configure in it as
> it is a generated file.
Right.. that made me realize patches-libffi/01-bug-670719.patch was already here, but hg log shows it came from bug 682180. Oh my.. note that all other patches touch configure _and_ configure.ac, but patches 00, 02 and 04 didnt seem to have regen'ed configure, so we'd need a more general stance on it.

Comment on attachment 649790[details][diff][review]
properly land as a separate patch + configure.ac fix + regen configure
Review of attachment 649790[details][diff][review]:
-----------------------------------------------------------------
Please avoid to change the configure executable bit. Also, considering the configure.ac changes, there should be no line changes in configure, so that must come from another patch. Better find what.

> Please avoid to change the configure executable bit. Also, considering the
> configure.ac changes, there should be no line changes in configure, so that
> must come from another patch. Better find what.
I think someone just patched configured instead of running autoconf, which is why you get all the line number changes.

I'm a bit lost here.. what would be acceptable ? A cset only changing configure/configure.ac powerpc-*-freebsd*) line (without the +x mode change) + adding 05-bug-670719 ? would that work out ? Or should i finally rerun autoconf and keep the line changes in configure ?

You need to figure out what else is making autoconf change line numbers, considering your patch doesn't add or remove a line. It must be some other patch. The simplest might just be to start afresh (revert all patches and refresh autoconf) and for each patch, apply, run autoconf, and refresh the patch with the associated configure changes. IOW, each patch should contain the configure changes that correspond to their own configure.ac changes.

Reverting all patches is not that easy :
00-base.patch doesnt unapply,
Patching file js/src/ctypes/libffi/src/arm/sysv.S using Plan A...
Hunk #1 succeeded at 137.
Hunk #2 failed at 225.
Hunk #3 succeeded at 301 (offset 4 lines).
1 out of 3 hunks failed--saving rejects to js/src/ctypes/libffi/src/arm/sysv.S.rej
configure.ac changes 3 lines, configure changes the same but adds
- fix_srcfile_path='`cygpath -w "$srcfile"`'
+ fix_srcfile_path=''
dunno if that comes from an autoconf run
01 is broken as it only changes configure. This one is my fault, it needs to change configure.ac accordingly instead of adding another 05-patch for it. It shouldnt cause line changes.
02 seems to be the guilty one : it removes a line and adds two lines to configure.ac, but configure only has one line changed
03 doesnt unapply, all the line changes dont match:
Patching file configure using Plan A...
Hunk #1 succeeded at 752 (offset -43 lines).
Hunk #2 failed at 4739.
Hunk #3 failed at 5951.
Hunk #4 failed at 7804.
Hunk #5 failed at 8143.
Hunk #6 failed at 8248.
Hunk #7 failed at 8303.
Hunk #8 failed at 11106.
Hunk #9 failed at 11202.
Hunk #10 succeeded at 12404 with fuzz 2 (offset -2243 lines).
8 out of 10 hunks failed--saving rejects to configure.rej
I think the linechanges from 03 should move to 02.
04 only changes one line in both configure and configure.ac.

After a bit more reflection ... patch 03 is indeed valid, since it adds the 'sys_symbol_underscore' line to configure, and thus shifts all configure lines. But the configure change in http://hg.mozilla.org/mozilla-central/rev/90a869f23e11 doesnt contain the line changes. So they're only in 03-bug-712594.patch but not in the configure file in hg, and thus reappears when one runs autoconf.
I can provide 3 patches :
- one only containing the line changes from 03-bug-712594.patch
- one fixing 02-bug-682180.patch to also add the comment line to configure
- one fixing 01-bug-670719.patch to also change configure.ac
would that work ?