1040785 - Ion/Odin various cleanups

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Benjamin Bouvier [:bbouvier] (inactive)]

A few things I saw while coding on SIMD these last few days.

Comment 1

[Benjamin Bouvier [:bbouvier] (inactive)]

Attachment: Factor out InvokeFromAsmJS functions

InvokeFromAsmJS functions have a lot of repeated code that we can easily factor
out.

Comment 2

[Benjamin Bouvier [:bbouvier] (inactive)]

Attachment: Move LInterruptCheck from the platform-specific LIR headers to the platform-independent one

The interrupt check LIR isn't actually platform dependent.

Comment 3

[Benjamin Bouvier [:bbouvier] (inactive)]

Attachment: Remove unused inc32 and dec32

These two functions aren't used so far.

Comment 4

[Benjamin Bouvier [:bbouvier] (inactive)]

https://tbpl.mozilla.org/?tree=Try&rev=15774c512961

Comment 5

[Luke Wagner [:luke]]

Comment on attachment 8458688 [details] [diff] [review]
Factor out InvokeFromAsmJS functions

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

Nice!

::: js/src/jit/AsmJSModule.cpp
@@ +478,1 @@
>  InvokeFromAsmJS_Ignore(JSContext *cx, int32_t exitIndex, int32_t argc, Value *argv);

This change is more dangerous since the compiler has the flexibility to only set/clear the low byte of the return register (leaving the upper 3/7 bytes trashed). Could you revert it?

Comment 6

[Benjamin Bouvier [:bbouvier] (inactive)]

(In reply to Benjamin Bouvier [:bbouvier] from comment #4)
> https://tbpl.mozilla.org/?tree=Try&rev=15774c512961

Cannot reproduce locally. Could this issue in FFI be due to TryEnablingIon returning a bool?

https://tbpl.mozilla.org/?tree=Try&rev=7ab364978a8c

Comment 7

[Benjamin Bouvier [:bbouvier] (inactive)]

(In reply to Benjamin Bouvier [:bbouvier] from comment #6)
> (In reply to Benjamin Bouvier [:bbouvier] from comment #4)
> > https://tbpl.mozilla.org/?tree=Try&rev=15774c512961
> 
> Cannot reproduce locally. Could this issue in FFI be due to TryEnablingIon
> returning a bool?
Yes. Added a comment above TryEnablingIon, to explain why we return an int32_t rather than a bool in this function and all InvokeFromAsmJS_* functions.

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/016386a3e1b4
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/6047f97d5986
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/e4c96102ba90

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/016386a3e1b4
https://hg.mozilla.org/mozilla-central/rev/6047f97d5986
https://hg.mozilla.org/mozilla-central/rev/e4c96102ba90