1042833 - Remove JS_ION ifdef

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Brian Hackett [Laid off!]]

Mentions of the JS_ION ifdef are sprinkled around nearly 500 places in the code base, and the --disable-ion build keeps breaking because so many features are gated on whether JS_ION is enabled.  While we need to continue supporting builds where the JITs are disabled it would be nice if this was handled a bit more elegantly.  Maybe have a backend set by --disable-ion which MOZ_CRASH'es everywhere and an IsIonEnabled() method so we avoid trying to compile anything?  While this might require more code, it should minimize the differences between --enable-ion and --disable-ion builds and we should only get build breakage when changing the interfaces used by IonMacroAssembler and similar classes.

Comment 1

[Brian Hackett [Laid off!]]

Attachment: add 'none' architecture

This patch changes JS_ION so that it is always defined, and adds a 'none' architecture with associated files that are compiled as the target when --disable-ion is used.  The 'none' architecture crashes anytime anyone tries to use it, so there are a few places where we test for this architecture (via JS_CODEGEN_NONE) before deciding to JIT compile anything.  If this patch lands, as a followup we can just remove all references to JS_ION.

Comment 2

[Jan de Mooij [:jandem]]

Comment on attachment 8462940 [details] [diff] [review]
add 'none' architecture

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

Nice, thanks for doing this!

This can/will break the --disable-ion build in new ways (when methods are added to the macro-assembler for instance), but whenever this happens it should be more trivial to fix than most of the current --disable-ion bustage in random parts of the tree. It's certainly much nicer to have most of the --disable-ion code in its own directory than having #ifdef JS_ION everywhere.

::: js/src/moz.build
@@ +359,5 @@
>              ]
> +else:
> +    UNIFIED_SOURCES += [
> +        'jit/none/Trampoline-none.cpp'
> +    ]

Indent this block, so that it comes after elif CONFIG['JS_CODEGEN_ARM']

Comment 3

[Brian Hackett [Laid off!]]

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

Try push (I don't know if this will need a clobber):

https://tbpl.mozilla.org/?tree=Try&rev=9bac8808a7f8

Comment 4

[Brian Hackett [Laid off!]]

Attachment: remove JS_ION

This patch removes all references to JS_ION

Comment 5

[Carsten Book [:Tomcat]]

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

Comment 6

[Jan de Mooij [:jandem]]

Comment on attachment 8463713 [details] [diff] [review]
remove JS_ION

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

r+++, would review again.

Comment 7

[Brian Hackett [Laid off!]]

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

Comment 8

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Pushed a long-overdue support for --disable-ion via fuzzing rev 6170cde887c2.

Comment 9

[Carsten Book [:Tomcat]]

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