commit 3de5137e2e0adea3c9a7dbed9230fc781ae794d3
parent 42c61dc9b52d47cf151f252282c36bc545be3921
Author: Atila Butkovits <abutkovits@mozilla.com>
Date: Wed, 26 Nov 2025 08:31:28 +0200
Revert "Bug 2001383 - Allow using another object's iterator indices r=iain,nbp" for causing Btime failures.
This reverts commit 2a24c97ea9432e51819e72de7ca9da408b1a5fdf.
Diffstat:
9 files changed, 48 insertions(+), 197 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
@@ -1,38 +0,0 @@
-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,11 +18585,15 @@ void CodeGenerator::visitValueToIterator(LValueToIterator* lir) {
callVM<Fn, ValueToIterator>(lir);
}
-void CodeGenerator::emitIteratorHasIndicesAndBranch(Register iterator,
- Register object,
- Register temp,
- Register temp2,
- Label* 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());
+
// Check that the iterator has indices available.
Address nativeIterAddr(iterator,
PropertyIteratorObject::offsetOfIteratorSlot());
@@ -18604,39 +18608,6 @@ void CodeGenerator::emitIteratorHasIndicesAndBranch(Register iterator,
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);
@@ -18698,6 +18669,7 @@ void CodeGenerator::visitLoadSlotByIteratorIndex(
visitLoadSlotByIteratorIndexCommon(object, indexScratch, kindScratch, result);
}
+#ifndef JS_CODEGEN_X86
void CodeGenerator::visitLoadSlotByIteratorIndexIndexed(
LLoadSlotByIteratorIndexIndexed* lir) {
Register object = ToRegister(lir->object());
@@ -18712,6 +18684,7 @@ void CodeGenerator::visitLoadSlotByIteratorIndexIndexed(
visitLoadSlotByIteratorIndexCommon(object, indexScratch, kindScratch, result);
}
+#endif
void CodeGenerator::visitStoreSlotByIteratorIndexCommon(Register object,
Register indexScratch,
@@ -18783,6 +18756,7 @@ void CodeGenerator::visitStoreSlotByIteratorIndex(
visitStoreSlotByIteratorIndexCommon(object, indexScratch, kindScratch, value);
}
+#ifndef JS_CODEGEN_X86
void CodeGenerator::visitStoreSlotByIteratorIndexIndexed(
LStoreSlotByIteratorIndexIndexed* lir) {
Register object = ToRegister(lir->object());
@@ -18797,6 +18771,7 @@ 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,10 +273,6 @@ 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,36 +5145,14 @@ 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;
}
-
-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;
-}
+#endif
bool jit::OptimizeIteratorIndices(const MIRGenerator* mir, MIRGraph& graph) {
bool changed = false;
@@ -5220,8 +5198,7 @@ bool jit::OptimizeIteratorIndices(const MIRGenerator* mir, MIRGraph& graph) {
continue;
}
- // Given the following structure (that occurs inside for-in loops or
- // when iterating a scalar-replaced Object.keys result):
+ // Given the following structure (that occurs inside for-in loops):
// obj: some object
// iter: ObjectToIterator <obj>
// iterLoad: IteratorMore <iter> | LoadIteratorElement <iter, index>
@@ -5238,33 +5215,11 @@ 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;
- MObjectToIterator* otherIter = nullptr;
+#ifndef JS_CODEGEN_X86
MDefinition* iterElementIndex = nullptr;
+#endif
if (idVal->isIteratorMore()) {
auto* iterNext = idVal->toIteratorMore();
@@ -5277,7 +5232,8 @@ bool jit::OptimizeIteratorIndices(const MIRGenerator* mir, MIRGraph& graph) {
SkipIterObjectUnbox(receiver)) {
continue;
}
- } else if (supportObjectKeys && SkipBox(idVal)->isLoadIteratorElement()) {
+#ifndef JS_CODEGEN_X86
+ } else if (SkipBox(idVal)->isLoadIteratorElement()) {
auto* iterLoad = SkipBox(idVal)->toLoadIteratorElement();
if (!iterLoad->iter()->isObjectToIterator()) {
@@ -5287,31 +5243,16 @@ bool jit::OptimizeIteratorIndices(const MIRGenerator* mir, MIRGraph& graph) {
iter = iterLoad->iter()->toObjectToIterator();
if (SkipIterObjectUnbox(iter->object()) !=
SkipIterObjectUnbox(receiver)) {
- if (!setValue) {
- otherIter = FindObjectToIteratorUse(SkipIterObjectUnbox(receiver));
- }
-
- if (!otherIter) {
- continue;
- }
+ continue;
}
iterElementIndex = iterLoad->index();
+#endif
} else {
continue;
}
- 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* indicesCheck =
+ MIteratorHasIndices::New(graph.alloc(), iter->object(), iter);
MInstruction* replacement;
if (ins->isHasOwnCache() || ins->isMegamorphicHasProp()) {
MOZ_ASSERT(!setValue);
@@ -5319,6 +5260,7 @@ 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);
@@ -5326,9 +5268,14 @@ 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);
@@ -5336,6 +5283,10 @@ 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,14 +2810,6 @@
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,21 +1283,6 @@ 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)));
@@ -6286,6 +6271,7 @@ void LIRGenerator::visitStoreSlotByIteratorIndex(
add(lir, ins);
}
+#ifndef JS_CODEGEN_X86
void LIRGenerator::visitLoadSlotByIteratorIndexIndexed(
MLoadSlotByIteratorIndexIndexed* ins) {
auto* lir = new (alloc()) LLoadSlotByIteratorIndexIndexed(
@@ -6301,18 +6287,13 @@ 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,10 +7531,6 @@ 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);
}
@@ -7854,6 +7850,7 @@ 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);
@@ -7863,6 +7860,7 @@ 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,14 +2826,6 @@
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
@@ -2852,6 +2844,12 @@
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
@@ -2875,6 +2873,7 @@
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,11 +2499,9 @@ bool RangeAnalysis::addRangeAssertions() {
// MIsNoIter is fused with the MTest that follows it and emitted as
// LIsNoIterAndBranch. Similarly, MIteratorHasIndices is fused to
- // become LIteratorHasIndicesAndBranch and IteratorsMatchAndHaveIndices
- // becomes LIteratorsMatchAndHaveIndicesAndBranch. Skip them to avoid
- // complicating lowering.
- if (ins->isIsNoIter() || ins->isIteratorHasIndices() ||
- ins->isIteratorsMatchAndHaveIndices()) {
+ // become LIteratorHasIndicesAndBranch. Skip them to avoid complicating
+ // lowering.
+ if (ins->isIsNoIter() || ins->isIteratorHasIndices()) {
MOZ_ASSERT(ins->hasOneUse());
continue;
}
