Closed Bug 1040785 Opened 5 years ago Closed 5 years ago

Ion/Odin various cleanups

Categories

(Core :: JavaScript Engine: JIT, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla33

People

(Reporter: bbouvier, Assigned: bbouvier)

Details

Attachments

(3 files)

A few things I saw while coding on SIMD these last few days.
InvokeFromAsmJS functions have a lot of repeated code that we can easily factor
out.
Attachment #8458688 - Flags: review?(luke)
The interrupt check LIR isn't actually platform dependent.
Attachment #8458689 - Flags: review?(jdemooij)
These two functions aren't used so far.
Attachment #8458690 - Flags: review?(jdemooij)
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?
Attachment #8458688 - Flags: review?(luke) → review+
(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
Attachment #8458689 - Flags: review?(jdemooij) → review+
Attachment #8458690 - Flags: review?(jdemooij) → review+
(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
You need to log in before you can comment on or make changes to this bug.