Closed Bug 1000632 Opened 6 years ago Closed 3 years ago
Monkey: further optimize the Odin to Ion FFI calls .
Bug 898963 comment 38 confirms that setActive can be simplified. This simplifies the Odin to Ion FFI call instruction sequences.
Here's a patch to try an implement the setActive simplification. Unfortunately it has not been a success and crashes, e.g. asm.js/testBullet.js
You will also need this. Since if we go this route, we need to initialize everything upfront.
A little blind hacking gives this as a reduced setActive that avoids crashing the jit-tests. Keeping the IonTop related code seem necessary to avoid a crash. Need to explore the real issue.
Attachment #8411837 - Attachment is obsolete: true
I might understand the failure here. The invariant we must preserve is that, at all times in asm.js code and when initially calling Ion from asm.js is: ionTop == ionTop on entry to asm.js. However, asm.js calls Ion and Ion returns to asm.js, ionTop may have been clobbered by calls to C++ from Ion code. Normally ~JitActivation makes this ok but we're explicitly skipping that. If this is correct, then it suggests that we do need the ionTop = prevIonTop on the "Disable Activation" path, but we can omit the prevIonTop = ionTop store on the "Enable Activation" path.
This is something we might still want to have; though I wonder if it would be superseded by work to make ion->wasm calls faster too, since that would merge the JitActivation with the WasmActivation, iirc.
Priority: -- → P5
Actually, these optimizations ended up going into 75fbd8a85688 as part of bug 1288483 recently.
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1288483
You need to log in before you can comment on or make changes to this bug.