commit 288840fe5c1198a36b9294f7a07df25dc76b865d
parent eb924ab3efd367691af60c7388d00416886b85bb
Author: alexical <dothayer@mozilla.com>
Date: Thu, 27 Nov 2025 06:22:35 +0000
Bug 2001383 - Allow using another object's iterator indices r=iain,nbp
Differential Revision: https://phabricator.services.mozilla.com/D273459
Diffstat:
9 files changed, 197 insertions(+), 48 deletions(-)
diff --git a/js/src/jit-test/tests/ion/iterator-indices-borrowed-iterator.js b/js/src/jit-test/tests/ion/iterator-indices-borrowed-iterator.js
@@ -0,0 +1,38 @@
+function test(obj, otherObj) {
+ var total = 0;
+ var keys = Object.keys(obj);
+ var otherKeys = Object.keys(otherObj);
+ if (keys.length != otherKeys.length) {
+ return -1;
+ }
+ for (var i = 0; i < keys.length; i++) {
+ var s = keys[i];
+ if (otherObj.hasOwnProperty(s)) {
+ total += otherObj[s];
+ }
+ }
+
+ return total;
+}
+
+var arr = [];
+var arr2 = [];
+for (var i = 0; i < 20; i++) {
+ var obj = {};
+ var obj2 = {};
+ for (var j = 0; j < i; j++) {
+ obj["x_" + i + "_" + j] = 1;
+ obj2["x_" + i + "_" + j] = 2;
+ }
+ arr.push(obj);
+ arr2.push(obj2);
+}
+
+with ({}) {}
+for (var i = 0; i < 2000; i++) {
+ var idx = i % arr.length;
+ assertEq(test(arr[idx], arr2[idx]), idx * 2);
+}
+
+assertEq(-1, test({a: 1, b: 1, c: 1}, {a: 2, b: 2}));
+assertEq(4, test({a: 1, b: 1, c: 1}, {a: 2, b: 2, d: 2}));
+\ No newline at end of file
diff --git a/js/src/jit/CodeGenerator.cpp b/js/src/jit/CodeGenerator.cpp
@@ -18585,15 +18585,11 @@ void CodeGenerator::visitValueToIterator(LValueToIterator* lir) {
callVM<Fn, ValueToIterator>(lir);
}
-void CodeGenerator::visitIteratorHasIndicesAndBranch(
- LIteratorHasIndicesAndBranch* lir) {
- Register iterator = ToRegister(lir->iterator());
- Register object = ToRegister(lir->object());
- Register temp = ToRegister(lir->temp0());
- Register temp2 = ToRegister(lir->temp1());
- Label* ifTrue = getJumpLabelForBranch(lir->ifTrue());
- Label* ifFalse = getJumpLabelForBranch(lir->ifFalse());
-
+void CodeGenerator::emitIteratorHasIndicesAndBranch(Register iterator,
+ Register object,
+ Register temp,
+ Register temp2,
+ Label* ifFalse) {
// Check that the iterator has indices available.
Address nativeIterAddr(iterator,
PropertyIteratorObject::offsetOfIteratorSlot());
@@ -18608,6 +18604,39 @@ void CodeGenerator::visitIteratorHasIndicesAndBranch(
masm.loadPtr(objShapeAddr, temp);
masm.branchTestObjShape(Assembler::NotEqual, object, temp, temp2, object,
ifFalse);
+}
+
+void CodeGenerator::visitIteratorHasIndicesAndBranch(
+ LIteratorHasIndicesAndBranch* lir) {
+ Register iterator = ToRegister(lir->iterator());
+ Register object = ToRegister(lir->object());
+ Register temp = ToRegister(lir->temp0());
+ Register temp2 = ToRegister(lir->temp1());
+ Label* ifTrue = getJumpLabelForBranch(lir->ifTrue());
+ Label* ifFalse = getJumpLabelForBranch(lir->ifFalse());
+
+ emitIteratorHasIndicesAndBranch(iterator, object, temp, temp2, ifFalse);
+
+ if (!isNextBlock(lir->ifTrue()->lir())) {
+ masm.jump(ifTrue);
+ }
+}
+
+void CodeGenerator::visitIteratorsMatchAndHaveIndicesAndBranch(
+ LIteratorsMatchAndHaveIndicesAndBranch* lir) {
+ Register iterator = ToRegister(lir->iterator());
+ Register otherIterator = ToRegister(lir->otherIterator());
+ Register object = ToRegister(lir->object());
+ Register temp = ToRegister(lir->temp0());
+ Register temp2 = ToRegister(lir->temp1());
+ Label* ifTrue = getJumpLabelForBranch(lir->ifTrue());
+ Label* ifFalse = getJumpLabelForBranch(lir->ifFalse());
+
+ // Check that the iterators match, and then we can use either iterator
+ // as a basis as if this were visitIteratorHasIndicesAndBranch
+ masm.branchPtr(Assembler::NotEqual, iterator, otherIterator, ifFalse);
+
+ emitIteratorHasIndicesAndBranch(iterator, object, temp, temp2, ifFalse);
if (!isNextBlock(lir->ifTrue()->lir())) {
masm.jump(ifTrue);
@@ -18669,7 +18698,6 @@ void CodeGenerator::visitLoadSlotByIteratorIndex(
visitLoadSlotByIteratorIndexCommon(object, indexScratch, kindScratch, result);
}
-#ifndef JS_CODEGEN_X86
void CodeGenerator::visitLoadSlotByIteratorIndexIndexed(
LLoadSlotByIteratorIndexIndexed* lir) {
Register object = ToRegister(lir->object());
@@ -18684,7 +18712,6 @@ void CodeGenerator::visitLoadSlotByIteratorIndexIndexed(
visitLoadSlotByIteratorIndexCommon(object, indexScratch, kindScratch, result);
}
-#endif
void CodeGenerator::visitStoreSlotByIteratorIndexCommon(Register object,
Register indexScratch,
@@ -18756,7 +18783,6 @@ void CodeGenerator::visitStoreSlotByIteratorIndex(
visitStoreSlotByIteratorIndexCommon(object, indexScratch, kindScratch, value);
}
-#ifndef JS_CODEGEN_X86
void CodeGenerator::visitStoreSlotByIteratorIndexIndexed(
LStoreSlotByIteratorIndexIndexed* lir) {
Register object = ToRegister(lir->object());
@@ -18771,7 +18797,6 @@ void CodeGenerator::visitStoreSlotByIteratorIndexIndexed(
visitStoreSlotByIteratorIndexCommon(object, indexScratch, kindScratch, value);
}
-#endif
void CodeGenerator::visitSetPropertyCache(LSetPropertyCache* ins) {
LiveRegisterSet liveRegs = ins->safepoint()->liveRegs();
diff --git a/js/src/jit/CodeGenerator.h b/js/src/jit/CodeGenerator.h
@@ -273,6 +273,10 @@ class CodeGenerator final : public CodeGeneratorSpecific {
void emitInstanceOf(LInstruction* ins, Register protoReg);
+ void emitIteratorHasIndicesAndBranch(Register iterator, Register object,
+ Register temp, Register temp2,
+ Label* ifFalse);
+
#ifdef DEBUG
void emitAssertResultV(const ValueOperand output, const MDefinition* mir);
void emitAssertGCThingResult(Register input, const MDefinition* mir);
diff --git a/js/src/jit/IonAnalysis.cpp b/js/src/jit/IonAnalysis.cpp
@@ -5145,14 +5145,36 @@ static MDefinition* SkipIterObjectUnbox(MDefinition* ins) {
return ins;
}
-#ifndef JS_CODEGEN_X86
static MDefinition* SkipBox(MDefinition* ins) {
if (ins->isBox()) {
return ins->toBox()->input();
}
return ins;
}
-#endif
+
+static MObjectToIterator* FindObjectToIteratorUse(MDefinition* ins) {
+ for (MUseIterator use(ins->usesBegin()); use != ins->usesEnd(); use++) {
+ if (!(*use)->consumer()->isDefinition()) {
+ continue;
+ }
+ MDefinition* def = (*use)->consumer()->toDefinition();
+ if (def->isGuardIsNotProxy()) {
+ MObjectToIterator* recursed = FindObjectToIteratorUse(def);
+ if (recursed) {
+ return recursed;
+ }
+ } else if (def->isUnbox()) {
+ MObjectToIterator* recursed = FindObjectToIteratorUse(def);
+ if (recursed) {
+ return recursed;
+ }
+ } else if (def->isObjectToIterator()) {
+ return def->toObjectToIterator();
+ }
+ }
+
+ return nullptr;
+}
bool jit::OptimizeIteratorIndices(const MIRGenerator* mir, MIRGraph& graph) {
bool changed = false;
@@ -5198,7 +5220,8 @@ bool jit::OptimizeIteratorIndices(const MIRGenerator* mir, MIRGraph& graph) {
continue;
}
- // Given the following structure (that occurs inside for-in loops):
+ // Given the following structure (that occurs inside for-in loops or
+ // when iterating a scalar-replaced Object.keys result):
// obj: some object
// iter: ObjectToIterator <obj>
// iterLoad: IteratorMore <iter> | LoadIteratorElement <iter, index>
@@ -5215,11 +5238,33 @@ bool jit::OptimizeIteratorIndices(const MIRGenerator* mir, MIRGraph& graph) {
// 3. If the property access is a SetProp, then we can use the contents
// of the indices array to find the correct slots faster than the
// megamorphic cache.
+ //
+ // In some cases involving Object.keys, we can also end up with a pattern
+ // like this:
+ //
+ // obj1: some object
+ // obj2: some object
+ // iter1: ObjectToIterator <obj1>
+ // iter2: ObjectToIterator <obj2>
+ // iterLoad: LoadIteratorElement <iter1>
+ // access: GetElem <obj2> <iterLoad>
+ //
+ // This corresponds to `obj2[Object.keys(obj1)[index]]`. In the general
+ // case we can't do much with this, but if obj1 and obj2 have the same
+ // shape, then we may reuse the iterator, in which case iter1 == iter2.
+ // In that case, we can optimize the access as if it were using iter2,
+ // at the cost of a single comparison to see if iter1 == iter2.
+#ifdef JS_CODEGEN_X86
+ // The ops required for this want more registers than is convenient on
+ // x86
+ bool supportObjectKeys = false;
+#else
+ bool supportObjectKeys = true;
+#endif
MObjectToIterator* iter = nullptr;
-#ifndef JS_CODEGEN_X86
+ MObjectToIterator* otherIter = nullptr;
MDefinition* iterElementIndex = nullptr;
-#endif
if (idVal->isIteratorMore()) {
auto* iterNext = idVal->toIteratorMore();
@@ -5232,8 +5277,7 @@ bool jit::OptimizeIteratorIndices(const MIRGenerator* mir, MIRGraph& graph) {
SkipIterObjectUnbox(receiver)) {
continue;
}
-#ifndef JS_CODEGEN_X86
- } else if (SkipBox(idVal)->isLoadIteratorElement()) {
+ } else if (supportObjectKeys && SkipBox(idVal)->isLoadIteratorElement()) {
auto* iterLoad = SkipBox(idVal)->toLoadIteratorElement();
if (!iterLoad->iter()->isObjectToIterator()) {
@@ -5243,16 +5287,31 @@ bool jit::OptimizeIteratorIndices(const MIRGenerator* mir, MIRGraph& graph) {
iter = iterLoad->iter()->toObjectToIterator();
if (SkipIterObjectUnbox(iter->object()) !=
SkipIterObjectUnbox(receiver)) {
- continue;
+ if (!setValue) {
+ otherIter = FindObjectToIteratorUse(SkipIterObjectUnbox(receiver));
+ }
+
+ if (!otherIter || !otherIter->block()->dominates(ins->block())) {
+ continue;
+ }
}
iterElementIndex = iterLoad->index();
-#endif
} else {
continue;
}
- MInstruction* indicesCheck =
- MIteratorHasIndices::New(graph.alloc(), iter->object(), iter);
+ MOZ_ASSERT_IF(iterElementIndex, supportObjectKeys);
+ MOZ_ASSERT_IF(otherIter, supportObjectKeys);
+
+ MInstruction* indicesCheck = nullptr;
+ if (otherIter) {
+ indicesCheck = MIteratorsMatchAndHaveIndices::New(
+ graph.alloc(), otherIter->object(), iter, otherIter);
+ } else {
+ indicesCheck =
+ MIteratorHasIndices::New(graph.alloc(), iter->object(), iter);
+ }
+
MInstruction* replacement;
if (ins->isHasOwnCache() || ins->isMegamorphicHasProp()) {
MOZ_ASSERT(!setValue);
@@ -5260,7 +5319,6 @@ bool jit::OptimizeIteratorIndices(const MIRGenerator* mir, MIRGraph& graph) {
} else if (ins->isMegamorphicLoadSlotByValue() ||
ins->isGetPropertyCache()) {
MOZ_ASSERT(!setValue);
-#ifndef JS_CODEGEN_X86
if (iterElementIndex) {
replacement = MLoadSlotByIteratorIndexIndexed::New(
graph.alloc(), receiver, iter, iterElementIndex);
@@ -5268,14 +5326,9 @@ bool jit::OptimizeIteratorIndices(const MIRGenerator* mir, MIRGraph& graph) {
replacement =
MLoadSlotByIteratorIndex::New(graph.alloc(), receiver, iter);
}
-#else
- replacement =
- MLoadSlotByIteratorIndex::New(graph.alloc(), receiver, iter);
-#endif
} else {
MOZ_ASSERT(ins->isMegamorphicSetElement() || ins->isSetPropertyCache());
MOZ_ASSERT(setValue);
-#ifndef JS_CODEGEN_X86
if (iterElementIndex) {
replacement = MStoreSlotByIteratorIndexIndexed::New(
graph.alloc(), receiver, iter, iterElementIndex, setValue);
@@ -5283,10 +5336,6 @@ bool jit::OptimizeIteratorIndices(const MIRGenerator* mir, MIRGraph& graph) {
replacement = MStoreSlotByIteratorIndex::New(graph.alloc(), receiver,
iter, setValue);
}
-#else
- replacement = MStoreSlotByIteratorIndex::New(graph.alloc(), receiver,
- iter, setValue);
-#endif
}
if (!block->wrapInstructionInFastpath(ins, replacement, indicesCheck)) {
diff --git a/js/src/jit/LIROps.yaml b/js/src/jit/LIROps.yaml
@@ -2810,6 +2810,14 @@
iterator: WordSized
num_temps: 2
+- name: IteratorsMatchAndHaveIndicesAndBranch
+ successors: [ifTrue, ifFalse]
+ operands:
+ object: WordSized
+ iterator: WordSized
+ otherIterator: WordSized
+ num_temps: 2
+
# Patchable jump to stubs generated for a SetProperty cache.
- name: SetPropertyCache
operands:
diff --git a/js/src/jit/Lowering.cpp b/js/src/jit/Lowering.cpp
@@ -1283,6 +1283,21 @@ void LIRGenerator::visitTest(MTest* test) {
return;
}
+ if (opd->isIteratorsMatchAndHaveIndices()) {
+ MOZ_ASSERT(opd->isEmittedAtUses());
+
+ MDefinition* object = opd->toIteratorsMatchAndHaveIndices()->object();
+ MDefinition* iterator = opd->toIteratorsMatchAndHaveIndices()->iterator();
+ MDefinition* otherIterator =
+ opd->toIteratorsMatchAndHaveIndices()->otherIterator();
+ LIteratorsMatchAndHaveIndicesAndBranch* lir =
+ new (alloc()) LIteratorsMatchAndHaveIndicesAndBranch(
+ ifTrue, ifFalse, useRegister(object), useRegister(iterator),
+ useRegister(otherIterator), temp(), temp());
+ add(lir, test);
+ return;
+ }
+
switch (opd->type()) {
case MIRType::Double:
add(new (alloc()) LTestDAndBranch(ifTrue, ifFalse, useRegister(opd)));
@@ -6271,7 +6286,6 @@ void LIRGenerator::visitStoreSlotByIteratorIndex(
add(lir, ins);
}
-#ifndef JS_CODEGEN_X86
void LIRGenerator::visitLoadSlotByIteratorIndexIndexed(
MLoadSlotByIteratorIndexIndexed* ins) {
auto* lir = new (alloc()) LLoadSlotByIteratorIndexIndexed(
@@ -6287,13 +6301,18 @@ void LIRGenerator::visitStoreSlotByIteratorIndexIndexed(
useRegister(ins->index()), useBox(ins->value()), temp(), temp());
add(lir, ins);
}
-#endif
void LIRGenerator::visitIteratorHasIndices(MIteratorHasIndices* ins) {
MOZ_ASSERT(ins->hasOneUse());
emitAtUses(ins);
}
+void LIRGenerator::visitIteratorsMatchAndHaveIndices(
+ MIteratorsMatchAndHaveIndices* ins) {
+ MOZ_ASSERT(ins->hasOneUse());
+ emitAtUses(ins);
+}
+
void LIRGenerator::visitSetPropertyCache(MSetPropertyCache* ins) {
MOZ_ASSERT(ins->object()->type() == MIRType::Object);
diff --git a/js/src/jit/MIR.cpp b/js/src/jit/MIR.cpp
@@ -7531,6 +7531,10 @@ AliasSet MIteratorHasIndices::getAliasSet() const {
return AliasSet::Load(AliasSet::ObjectFields);
}
+AliasSet MIteratorsMatchAndHaveIndices::getAliasSet() const {
+ return AliasSet::Load(AliasSet::ObjectFields);
+}
+
AliasSet MAllocateAndStoreSlot::getAliasSet() const {
return AliasSet::Store(AliasSet::ObjectFields | AliasSet::DynamicSlot);
}
@@ -7850,7 +7854,6 @@ AliasSet MStoreSlotByIteratorIndex::getAliasSet() const {
AliasSet::DynamicSlot | AliasSet::Element);
}
-#ifndef JS_CODEGEN_X86
AliasSet MLoadSlotByIteratorIndexIndexed::getAliasSet() const {
return AliasSet::Load(AliasSet::ObjectFields | AliasSet::FixedSlot |
AliasSet::DynamicSlot | AliasSet::Element);
@@ -7860,7 +7863,6 @@ AliasSet MStoreSlotByIteratorIndexIndexed::getAliasSet() const {
return AliasSet::Store(AliasSet::ObjectFields | AliasSet::FixedSlot |
AliasSet::DynamicSlot | AliasSet::Element);
}
-#endif
MDefinition* MGuardInt32IsNonNegative::foldsTo(TempAllocator& alloc) {
MOZ_ASSERT(index()->type() == MIRType::Int32);
diff --git a/js/src/jit/MIROps.yaml b/js/src/jit/MIROps.yaml
@@ -2826,6 +2826,14 @@
result_type: Boolean
alias_set: custom
+- name: IteratorsMatchAndHaveIndices
+ operands:
+ object: Object
+ iterator: Object
+ otherIterator: Object
+ result_type: Boolean
+ alias_set: custom
+
- name: LoadSlotByIteratorIndex
operands:
object: Object
@@ -2844,12 +2852,6 @@
generate_lir: true
lir_temps: 2
-
-# The following two instructions need too many registers on x86. We could
-# spill or do some cheeky stuff potentially but it's likely not worth the
-# complexity
-#ifndef JS_CODEGEN_X86
-
# Okay this is a confusing name, but essentially, the above ops load/store a
# slot of an object using the PropertyIndex associated with the *current* key
# for the iterator. The below op uses the PropertyIndex associated with the
@@ -2873,7 +2875,6 @@
alias_set: custom
generate_lir: true
lir_temps: 2
-#endif
# Load the private value expando from a DOM proxy. The target is stored in the
# proxy object's private slot.
diff --git a/js/src/jit/RangeAnalysis.cpp b/js/src/jit/RangeAnalysis.cpp
@@ -2499,9 +2499,11 @@ bool RangeAnalysis::addRangeAssertions() {
// MIsNoIter is fused with the MTest that follows it and emitted as
// LIsNoIterAndBranch. Similarly, MIteratorHasIndices is fused to
- // become LIteratorHasIndicesAndBranch. Skip them to avoid complicating
- // lowering.
- if (ins->isIsNoIter() || ins->isIteratorHasIndices()) {
+ // become LIteratorHasIndicesAndBranch and IteratorsMatchAndHaveIndices
+ // becomes LIteratorsMatchAndHaveIndicesAndBranch. Skip them to avoid
+ // complicating lowering.
+ if (ins->isIsNoIter() || ins->isIteratorHasIndices() ||
+ ins->isIteratorsMatchAndHaveIndices()) {
MOZ_ASSERT(ins->hasOneUse());
continue;
}
