Odinmonkey: reduce out-of-line bounds-checking code size on x86

RESOLVED WONTFIX

Status

()

Core
JavaScript Engine: JIT
P5
enhancement
RESOLVED WONTFIX
3 years ago
2 years ago

People

(Reporter: sunfish, Assigned: sunfish)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Assignee)

Description

3 years ago
Right now, on x86, when a heap access index is out of bounds, we branch to an out-of-line stub that implements appropriate out-of-bounds semantics. Especially when there are non-zero offsets (courtesy of bug 986981), this code can be several instructions per heap access.

Combining ideas from Alon and Luke, here's a new approach: each out-of-line stub can consist of a single call instruction. The callee is some special-purpose code which is shared for all accesses which simply reads the return address and then makes a full call into C++ code to handle the out-of-bounds case.

We have C++ code to do this already, it just needs to be refactored out of AsmJSSignalHandlers.cpp. One thing it needs is to get the pc of the out-of-bounds access. Through the stub we'll have captured the pc of the out-of-line code, so we'll need a mechanism to map back to the inline access.

That mapping can be done by putting all the out-of-line stubs in a contiguous region, so that given an address in the middle, we can compute the index, which can then identify an AsmJSHeapAccess instance.

Some nice properties of this approach:
 - no signal handling needed
 - it should be generalizable to ARM, where we may even be able to do a conditional call inline and skip the out-of-line part
 - it can permit greater code reuse between x86, x64, and arm
Nice proposal! (for the record, bug 869606 was also reducing the size of x86 out-of-bounds code sections by sharing them, and also combining two instructions into one for some CPU wins)
WebAssembly OOB accesses just throw, so this would be an asm.js optimization, and I am not sure we want to invest too much time in those?
Flags: needinfo?(luke)
Priority: -- → P5

Comment 3

2 years ago
Agreed
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Flags: needinfo?(luke)
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.