Warp: Transpile LoadDenseElementExistsResult and StoreDenseElementHole
Categories
(Core :: JavaScript Engine: JIT, task)
Tracking
()
Tracking | Status | |
---|---|---|
firefox81 | --- | fixed |
People
(Reporter: anba, Assigned: anba)
References
(Blocks 1 open bug)
Details
Attachments
(5 files)
Assignee | ||
Comment 1•4 years ago
|
||
StoreDenseElementHole
and ArrayPush
are both preceded by shape guards and
are only attached when the object has a writable array length. That means the
generated code doesn't actually need to test NONWRITABLE_ARRAY_LENGTH
,
because it's implied by the shape guard: Changing the array length to
non-writable through either js::ArraySetLength
or js::SetIntegrityLevel
already changes the shape and thus invalidates the shape guard.
The same applies to the non-extensible bits, so we can assert that, too.
Drive-by change:
Remove the thisarray->lengthIsWritable()
assertion in tryAttachArrayPush
because it's tested just a few lines earlier.
Assignee | ||
Comment 2•4 years ago
|
||
The code generator uses a signed value and the value is implicitly coerced to
int32_t
anyway (via Int32Value
), so we might as well directly use int32_t
for that function.
Depends on D87560
Assignee | ||
Comment 3•4 years ago
|
||
Using MLoadElement
with needsHoleCheck=true
to trigger a bailout when a
hole is encountered was slightly slower than the new MGuardElementNotHole
instruction from this patch, even though the MLoadElement
from a JSOp::In
can be merged with a subsequent MLoadElement
from a JSOp::GetElem
. This
sequence happens for the common pattern of if (i in array) val = array[i]
.
The magic value check in LGuardElementNotHole
works without unboxing the
value, so it's possible using separate instructions leads to fewer register
dependencies (when compared to merging the MLoadElement
), which helps
speculative execution. And because the loaded value may still be in a cache,
it doesn't matter to load it repeatedly.
Depends on D87561
Assignee | ||
Comment 4•4 years ago
|
||
MStoreElement
with needsHoleCheck=false
matches the Baseline implementation
of StoreDenseElementHole
. MStoreElementHole
handles more cases than the
Baseline (and Ion) implementation of StoreDenseElementHole, namely it also allows to set elements for
index > initializedLength`.
The index > initializedLength
restriction for StoreDenseElementHole
may be
lifted in the future when it's no longer necessary to call noteHasDenseAdd()
for Ion.
Depends on D87562
Assignee | ||
Comment 5•4 years ago
|
||
Neither MLoadElementAndUnbox nor MSpectreMaskIndex can GC, so we don't need a
MKeepAliveObject instruction.
Depends on D87563
Pushed by nerli@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/87344933e180 Part 1: Replace non-writable array length failure paths with assertions. r=jandem https://hg.mozilla.org/integration/autoland/rev/4bb5a802a5c0 Part 2: Change `OperatorInI` function to take int32 instead of uint32. r=jandem https://hg.mozilla.org/integration/autoland/rev/10db5a2060fd Part 3: Transpile LoadDenseElementExistsResult. r=jandem https://hg.mozilla.org/integration/autoland/rev/55d2b4523aa0 Part 4: Transpile StoreDenseElementHole. r=jandem https://hg.mozilla.org/integration/autoland/rev/d17ecc15076d Part 5: MLoadElementAndUnbox and MSpectreMaskIndex don't need MKeepAliveObject. r=jandem
Comment 7•4 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/87344933e180
https://hg.mozilla.org/mozilla-central/rev/4bb5a802a5c0
https://hg.mozilla.org/mozilla-central/rev/10db5a2060fd
https://hg.mozilla.org/mozilla-central/rev/55d2b4523aa0
https://hg.mozilla.org/mozilla-central/rev/d17ecc15076d
Description
•