Closed Bug 990154 Opened 6 years ago Closed 6 years ago

Bug 981693 broke archs without ENABLE_ASSEMBLER

Categories

(Core :: JavaScript Engine: JIT, defect)

Sun
OpenBSD
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla31
Tracking Status
firefox29 --- unaffected
firefox30 --- fixed
firefox31 --- fixed

People

(Reporter: gaston, Assigned: gaston)

References

Details

Attachments

(2 files)

While building js shell on sparc64.... probably affects aurora too since that landed when 30 was in m-c. Couldnt find if already reported.

eg++ -o Unified_cpp_js_src_shell0.o -c  -fvisibility=hidden -DEXPORT_JS_API -DIMPL_MFBT -DMOZ_GLUE_IN_PROGRAM -DNO_NSPR_10_SUPPORT -I/home/buildslave/mozilla-central-sparc64/build/js/src/shell -I. -I/home/buildslave/mozilla-central-sparc64/build/js/src/shell/.. -I.. -I../../../dist/include  -I/data/obj/buildslave/m-c/dist/include/nspr    -I/home/buildslave/mozilla-central-sparc64/build/modules/zlib/src    -fPIC   -DMOZILLA_CLIENT -include ../../../js/src/js-confdefs.h -MD -MP -MF .deps/Unified_cpp_js_src_shell0.o.pp  -Wall -Wpointer-arith -Woverloaded-virtual -Werror=return-type -Werror=int-to-pointer-cast -Wtype-limits -Wempty-body -Werror=conversion-null -Wsign-compare -Wno-invalid-offsetof -fno-rtti -fno-exceptions -fno-math-errno -std=gnu++0x -pthread -pipe  -DNDEBUG -DTRIMMED -g -O -fomit-frame-pointer     /data/obj/buildslave/m-c/js/src/shell/Unified_cpp_js_src_shell0.cpp
In file included from /home/buildslave/mozilla-central-sparc64/build/js/src/shell/../jsscript.h:25:0,
                 from /home/buildslave/mozilla-central-sparc64/build/js/src/shell/../vm/Runtime.h:26,
                 from /home/buildslave/mozilla-central-sparc64/build/js/src/shell/../jscntxt.h:15,
                 from /home/buildslave/mozilla-central-sparc64/build/js/src/shell/js.cpp:41,
                 from /data/obj/buildslave/m-c/js/src/shell/Unified_cpp_js_src_shell0.cpp:2:
/home/buildslave/mozilla-central-sparc64/build/js/src/shell/../jit/IonCode.h:62:18: error: 'JSC::CodeKind' has not been declared
             JSC::CodeKind kind)
                  ^
/home/buildslave/mozilla-central-sparc64/build/js/src/shell/../jit/IonCode.h:139:57: error: 'JSC::CodeKind' has not been declared
                         JSC::ExecutablePool *pool, JSC::CodeKind kind);
Blocks: 981693
Summary: Bug 981693 broke non-ionmonkey archs → Bug 981693 broke archs without ENABLE_ASSEMBLER
Hardware: x86_64 → Sun
Does this patch let you build?

Can anyone see a reason why we can't include the ExecutableAllocator header for non assembler builds?
Flags: needinfo?(landry)
Testing it along #991966 and #980891 in http://buildbot.rhaalovely.net/builders/mozilla-central-sparc64/builds/761
Flags: needinfo?(landry)
It fails much later on a libcubeb test :
../../../dist/bin/libxul.so.1.0: undefined reference to `I422ToI420'
../../../dist/bin/libxul.so.1.0: undefined reference to `I444ToI420'
../../../dist/bin/libxul.so.1.0: undefined reference to `NV21ToI420'
../../../dist/bin/libxul.so.1.0: undefined reference to `NV12ToI420'

but thats probably a new unrelated failure :)
Attachment #8402064 - Flags: review?(n.nethercote)
Comment on attachment 8402064 [details] [diff] [review]
990154_noassembler.diff

Review of attachment 8402064 [details] [diff] [review]:
-----------------------------------------------------------------

This doesn't feel right to me -- I think you only need CodeKind, but this change exposes ExecutablePool and ExecutableAllocator? I'm not sure... I think jandem would know better than me.
Attachment #8402064 - Flags: review?(n.nethercote) → review?(jdemooij)
Comment on attachment 8402064 [details] [diff] [review]
990154_noassembler.diff

Review of attachment 8402064 [details] [diff] [review]:
-----------------------------------------------------------------

I agree with njn. Can you move the CodeKind enum in ExecutableAllocator.h before the #if ENABLE_ASSEMBLER and see if that works?

namespace JS {
  enum CodeKind { ION_CODE = 0, BASELINE_CODE, REGEXP_CODE, OTHER_CODE };
}

That seems a bit simpler and less likely to introduce new problems.
Attachment #8402064 - Flags: review?(jdemooij)
Will be tested in the next build:
http://buildbot.rhaalovely.net/builders/mozilla-central-sparc64/builds/764

(that one will probably fail during linking libxul with the message in comment 3 but at least we'll know if it fixes js)
I am able to build fine  with attachment 8403460 [details] [diff] [review] on linux ppc64 (also with the skia patches in bug 980891 applied).
I am not seeing the I422ToI420 issue.
I422ToI420 is bug 981780. sparc, mips, aarch64, etc. and Solaris still default to --disable-webrtc.
Comment on attachment 8403460 [details] [diff] [review]
bug-990154-codekind

Lets try to get this to aurora through inbound and central first then...
Attachment #8403460 - Flags: review?(jdemooij)
Comment on attachment 8403460 [details] [diff] [review]
bug-990154-codekind

Review of attachment 8403460 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, thanks.
Attachment #8403460 - Flags: review?(jdemooij) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/4d46754ac133

given that this is NPOTB (all tier1 platforms should have ENABLE_ASSEMBLER, no ?) i wonder if i should ask for approval-aurora or land it directly there...
https://hg.mozilla.org/mozilla-central/rev/4d46754ac133
Assignee: nobody → landry
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
Comment on attachment 8403460 [details] [diff] [review]
bug-990154-codekind

Just in case, formal approval request even if NPOTB-tier1..

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 981693  
User impact if declined: Fx broken on non-asm archs
Testing completed (on m-c, etc.): Fixes build on sparc64
Risk to taking this patch (and alternatives if risky): Puts more stuff outside ENABLE_ASSEMBLER scope, but that's needed now non non-ENABLE_ASSEMBLER archs
String or IDL/UUID changes made by this patch: non
Attachment #8403460 - Flags: approval-mozilla-aurora?
Attachment #8403460 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.