Closed Bug 1659694 Opened 4 years ago Closed 4 years ago

Warp: Transpile LoadDenseElementExistsResult and StoreDenseElementHole

Categories

(Core :: JavaScript Engine: JIT, task)

task

Tracking

()

RESOLVED FIXED
81 Branch
Tracking Status
firefox81 --- fixed

People

(Reporter: anba, Assigned: anba)

References

(Blocks 1 open bug)

Details

Attachments

(5 files)

No description provided.

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.

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

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

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 forindex > 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

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
Regressions: 1660599
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: