commit 6deebe02578a7cc69dff2ec7686948f2f436f195 parent 51fa453be99b83a0a8125f53292bc71e822559c5 Author: Alexandru Marc <amarc@mozilla.com> Date: Wed, 19 Nov 2025 21:00:32 +0200 Revert (Bug 1992813, Bug 1995077) for causing cpp failures @ Iteration.h This reverts commit 8b5cc877329dc07df7e752d6a78d74dc02961a12. Revert "Bug 1995077 - Integrate with existing iterator indices optimizations r=iain" This reverts commit 07b19b9cbad6cd2cdbe47cd0bc62d9a77b2e2a53. Revert "Bug 1995077 - Scalar replace Object.keys r=iain" This reverts commit f776bb0d5fb34a3c41f4833cefa76bfbbe1938e8. Revert "Bug 1995077 - Track own property count in NativeIterator r=iain" This reverts commit c3b712aa4c06e8716756eee9131a516c51c531b6. Revert "Bug 1992813 - Slim down NativeIterator r=iain" This reverts commit ebb92832be3d5242a721b2a981f17e87b9d6fe61. Diffstat:
48 files changed, 728 insertions(+), 1635 deletions(-)
diff --git a/js/src/jit-test/tests/basic/bug1884706.js b/js/src/jit-test/tests/basic/bug1884706.js @@ -0,0 +1,5 @@ +const arr = new Int32Array(1 << 26); +try { + for (const key in arr) { + } +} catch {} diff --git a/js/src/jit-test/tests/ion/iterator-indices-1.js b/js/src/jit-test/tests/ion/iterator-indices-1.js @@ -8,16 +8,6 @@ function test(obj, expected) { assertEq(count, expected); } -function test2(obj, expected) { - var count = 0; - for (var s of Object.keys(obj)) { - if (obj.hasOwnProperty(s)) { - count++; - } - } - assertEq(count, expected); -} - var arr = []; for (var i = 0; i < 20; i++) { var obj = {}; @@ -32,5 +22,4 @@ with ({}) {} for (var i = 0; i < 2000; i++) { var idx = i % arr.length; test(arr[idx], idx); - test2(arr[idx], idx); } diff --git a/js/src/jit-test/tests/ion/iterator-indices-2.js b/js/src/jit-test/tests/ion/iterator-indices-2.js @@ -6,16 +6,6 @@ function test(obj, expected) { assertEq(actual, expected); } -function test2(obj, expected) { - var count = 0; - for (var s of Object.keys(obj)) { - if (obj.hasOwnProperty(s)) { - count++; - } - } - assertEq(count, expected); -} - var arr = []; for (var i = 0; i < 20; i++) { var obj = {}; @@ -30,5 +20,4 @@ with ({}) {} for (var i = 0; i < 2000; i++) { var idx = i % arr.length; test(arr[idx], idx); - test2(arr[idx], idx); } diff --git a/js/src/jit-test/tests/ion/iterator-indices-3.js b/js/src/jit-test/tests/ion/iterator-indices-3.js @@ -8,16 +8,6 @@ function test(o, deleter) { return result; } -function test2(o, deleter) { - var result = 0; - for (var s of Object.keys(o)) { - if (!o.hasOwnProperty(s)) { continue; } - result += o[s]; - deleter(o); - } - return result; -} - with ({}) {} // Ion-compile |test| with a megamorphic getprop and a generic call. @@ -26,8 +16,6 @@ for (var i = 0; i < 2000; i++) { obj["x" + i] = 1; test(obj, () => 1); test(obj, () => 2); - test2(obj, () => 1); - test2(obj, () => 2); } assertEq(test({x: 1, y: 2}, (o) => delete o.y), 1); @@ -41,15 +29,3 @@ assertEq(test([1,2,3,4], (o) => {o.length = 2}), 3); assertEq(test({x: 1, y: 2, z: 3}, (o) => delete o.x), 6); assertEq(test([1,2,3], (o) => delete o[0]), 6); assertEq(test([1], (o) => {o.length = 0}), 1); - -assertEq(test2({x: 1, y: 2}, (o) => delete o.y), 1); -assertEq(test2([1,2], (o) => delete o[1]), 1); -assertEq(test2([1,2], (o) => {o.length = 0}), 1); - -assertEq(test2({x: 1, y: 2, z: 3}, (o) => delete o.y), 4); -assertEq(test2([1,2,3], (o) => delete o[1]), 4); -assertEq(test2([1,2,3,4], (o) => {o.length = 2}), 3); - -assertEq(test2({x: 1, y: 2, z: 3}, (o) => delete o.x), 6); -assertEq(test2([1,2,3], (o) => delete o[0]), 6); -assertEq(test2([1], (o) => {o.length = 0}), 1); diff --git a/js/src/jit-test/tests/ion/iterator-indices-4.js b/js/src/jit-test/tests/ion/iterator-indices-4.js @@ -6,12 +6,6 @@ function foo(obj) { } } -function foo2(obj) { - for (var key of Object.keys(obj)) { - assertEq(id(obj[key]), obj[key]); - } -} - var arr = []; for (var i = 0; i < 8; i++) { var obj = {["x" + i]: 1}; @@ -22,5 +16,4 @@ with ({}) {} for (var i = 0; i < 1000; i++) { let obj = arr[i % arr.length]; foo(obj); - foo2(obj); } diff --git a/js/src/jit-test/tests/ion/iterator-indices-5.js b/js/src/jit-test/tests/ion/iterator-indices-5.js @@ -16,15 +16,6 @@ function foo(o, trigger) { return result; } -function foo2(o, trigger) { - var result; - for (var key of Object.keys(o)) { - result = o[key]; - bar(o, trigger); - } - return result; -} - var arr = []; for (var i = 0; i < 10; i++) { arr.push({["x" + i]: 0, y: 0}); @@ -33,9 +24,7 @@ for (var i = 0; i < 10; i++) { with ({}) {} for (var i = 0; i < 1000; i++) { for (var o of arr) { - foo(o, false) - foo2(o, false) + foo(o, false) } } print(foo(arr[0], true)); -print(foo2(arr[0], true)); diff --git a/js/src/jit-test/tests/ion/iterator-indices-6.js b/js/src/jit-test/tests/ion/iterator-indices-6.js @@ -15,21 +15,6 @@ function test(o1, o2) { assertEq(count, 2); } -function test2(o1, o2) { - var count = 0; - for (var s1 of Object.keys(o1)) { - for (var s2 of Object.keys(o2)) { - if (Object.hasOwn(o1, s1)) { - count += o1[s1]; - } - if (Object.hasOwn(o2, s2)) { - count += o2[s2]; - } - } - } - assertEq(count, 2); -} - var arr = []; for (var i = 0; i < 20; i++) { arr.push({["x_" + i]: 1}); @@ -40,5 +25,4 @@ for (var i = 0; i < 2000; i++) { var idx1 = i % arr.length; var idx2 = 1 + i % (arr.length - 1); test(arr[idx1], arr[idx2]); - test2(arr[idx1], arr[idx2]); } diff --git a/js/src/jit-test/tests/ion/iterator-indices-7.js b/js/src/jit-test/tests/ion/iterator-indices-7.js @@ -5,13 +5,6 @@ function test(obj, expected) { } assertEq(actual, expected); } -function test2(obj, expected) { - var actual = 0; - for (var s of Object.keys(obj)) { - actual += obj[s]; - } - assertEq(actual, expected); -} var arr = []; var elem_obj = []; @@ -29,5 +22,4 @@ with ({}) {} for (var i = 0; i < 2000; i++) { var idx = i % arr.length; test(arr[idx], idx); - test2(arr[idx], idx); } diff --git a/js/src/jit-test/tests/ion/iterator-indices-8.js b/js/src/jit-test/tests/ion/iterator-indices-8.js @@ -11,42 +11,22 @@ function test(obj) { index++; } } -function test2(obj) { - let index = 0; - for (var s of Object.keys(obj)) { - obj[s] = index; - index++; - } - index = 0; - for (var s of Object.keys(obj)) { - assertEq(obj[s], index); - index++; - } -} var arr = []; -var arr2 = []; var elem_obj = []; -var elem_obj2 = []; 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] = 1; } arr.push(obj); - arr2.push(obj2); elem_obj.push(1); - elem_obj2.push(1); } arr.push(elem_obj); -arr2.push(elem_obj2); with ({}) {} for (var i = 0; i < 2000; i++) { var idx = i % arr.length; test(arr[idx]); - test2(arr2[idx]); } diff --git a/js/src/jit-test/tests/ion/iterator-indices-9.js b/js/src/jit-test/tests/ion/iterator-indices-9.js @@ -16,42 +16,19 @@ function test(obj) { } } -function test2(obj) { - let index = 0; - for (var s of Object.keys(obj)) { - if (s.startsWith("test")) { - obj[s] = index; - } - index++; - } - index = 0; - for (var s of Object.keys(obj)) { - if (s.startsWith("test")) { - assertEq(obj[s], index); - } - index++; - } -} - var arr = []; -var arr2 = []; for (var i = 0; i < 2; i++) { var obj = {}; - var obj2 = {}; for (var j = 0; j < i * 20; j++) { obj["x_" + i + "_" + j] = 1; - obj2["x_" + i + "_" + j] = 1; } obj.testKey = 1; - obj2.testKey = 1; arr.push(obj); - arr2.push(obj2); } with ({}) {} for (var i = 0; i < 2000; i++) { var idx = i % arr.length; test(arr[idx]); - test2(arr2[idx]); } diff --git a/js/src/jit-test/tests/ion/object-keys-04.js b/js/src/jit-test/tests/ion/object-keys-04.js @@ -26,6 +26,13 @@ function objKeysLength(obj, expected, i) { return len; } +// This is the same test as above, except that the branch which is being removed +// cause the introduction of a different resume point to be inserted in the +// middle. At the moment we expect this circustances to to disable the +// optimization. +// +// Removing this limitation would be useful but would require more verification +// when applying the optimization. function objKeysLengthDiffBlock(obj, expected, i) { var keys = Object.keys(obj); if (i >= 99) { @@ -35,19 +42,19 @@ function objKeysLengthDiffBlock(obj, expected, i) { assertEq(arraysEqual(keys, expected), true); } let len = keys.length; - assertRecoveredOnBailout(keys, true); + assertRecoveredOnBailout(keys, false); return len; } // Mutating the object in-between the call from Object.keys and the evaluation -// of the length property should still allow the optimization to take place -// since the iterator we held onto should be from the previous shape of the -// object. +// of the length property should prevent the optimization from being reflected +// as the mutation of the object would cause the a different result of +// Object.keys evaluation. function objKeysLengthMutate0(obj, expected, i) { var keys = Object.keys(obj); obj.foo = 42; let len = keys.length; - assertRecoveredOnBailout(keys, true); + assertRecoveredOnBailout(keys, false); if (i >= 99) { bailout(); assertEq(arraysEqual(keys, expected), true); @@ -58,7 +65,7 @@ function objKeysLengthMutate0(obj, expected, i) { function objKeysLengthMutate1(obj, expected, i) { var keys = Object.keys(obj); let len = keys.length; - assertRecoveredOnBailout(keys, true); + assertRecoveredOnBailout(keys, false); obj.foo = 42; if (i >= 99) { bailout(); @@ -70,7 +77,7 @@ function objKeysLengthMutate1(obj, expected, i) { function objKeysLengthMutate2(obj, expected, i) { var keys = Object.keys(obj); let len = keys.length; - assertRecoveredOnBailout(keys, true); + assertRecoveredOnBailout(keys, false); if (i >= 99) { bailout(); assertEq(arraysEqual(keys, expected), true); @@ -84,6 +91,8 @@ function objKeysLengthMutate3(obj, expected, i) { let len = keys.length; assertRecoveredOnBailout(keys, true); if (i >= 99) { + // When branches are pruned, Warp/Ion is not aware and would recover the + // keys on bailout, and this is fine. obj.foo = 42; bailout(); assertEq(arraysEqual(keys, expected), true); @@ -92,6 +101,9 @@ function objKeysLengthMutate3(obj, expected, i) { } function objKeysLengthMutate4(obj, expected, i) { + // Mutating the objects ahead of keying the keys does not prevent optimizing + // the keys length query, given that all side-effects are already acted by + // the time we query the keys. obj.foo = 42; var keys = Object.keys(obj); let len = keys.length; @@ -111,7 +123,7 @@ function doNotInlineSideEffect() { function objKeysLengthSideEffect0(obj, expected, i) { var keys = Object.keys(obj); let len = keys.length; - assertRecoveredOnBailout(keys, true); + assertRecoveredOnBailout(keys, false); doNotInlineSideEffect(); if (i >= 99) { bailout(); @@ -123,7 +135,7 @@ function objKeysLengthSideEffect0(obj, expected, i) { function objKeysLengthSideEffect1(obj, expected, i) { var keys = Object.keys(obj); let len = keys.length; - assertRecoveredOnBailout(keys, true); + assertRecoveredOnBailout(keys, false); if (i >= 99) { bailout(); assertEq(arraysEqual(keys, expected), true); @@ -158,48 +170,6 @@ function objKeysLengthSideEffect3(obj, expected, i) { return len; } -function objKeysLengthMutateElements0(obj, expected, i) { - var keys = Object.keys(obj); - obj[0] = 42; - obj[1] = 42; - obj[2] = 42; - let len = keys.length; - assertRecoveredOnBailout(keys, true); - if (i >= 99) { - bailout(); - assertEq(arraysEqual(keys, expected), true); - } - return len; -} - -function objKeysLengthMutateElements1(obj, expected, i) { - obj[0] = 42; - var keys = Object.keys(obj); - obj[1] = 42; - obj[2] = 42; - let len = keys.length; - assertRecoveredOnBailout(keys, true); - if (i >= 99) { - bailout(); - assertEq(arraysEqual(keys, expected), true); - } - return len; -} - -function objKeysLengthMutateElements2(obj, expected, i) { - obj[0] = 42; - obj[1] = 42; - var keys = Object.keys(obj); - obj[2] = 42; - let len = keys.length; - assertRecoveredOnBailout(keys, true); - if (i >= 99) { - bailout(); - assertEq(arraysEqual(keys, expected), true); - } - return len; -} - // Check what we are doing as optimizations when the object is fully known and // when it does not escape. // @@ -234,8 +204,5 @@ for (let i = 0; i < 100; i++) { objKeysLengthSideEffect1({...obj}, ["a", "b", "c", "d"], i); objKeysLengthSideEffect2({...obj}, ["a", "b", "c", "d"], i); objKeysLengthSideEffect3({...obj}, ["a", "b", "c", "d"], i); - objKeysLengthMutateElements0({...obj}, ["a", "b", "c", "d"], i); - objKeysLengthMutateElements1({...obj}, ["0", "a", "b", "c", "d"], i); - objKeysLengthMutateElements2({...obj}, ["0", "1", "a", "b", "c", "d"], i); nonEscapedObjKeysLength(i); } diff --git a/js/src/jit-test/tests/ion/object-keys-06.js b/js/src/jit-test/tests/ion/object-keys-06.js @@ -1,40 +0,0 @@ -load(libdir + 'array-compare.js'); // arraysEqual - -// Ion eager fails the test below because we have not yet created any -// Cache IR IC in baseline before running the content of the top-level -// function. -if (getJitCompilerOptions()["ion.warmup.trigger"] <= 100) - setJitCompilerOption("ion.warmup.trigger", 100); - -// This test case checks that we are capable of recovering the Object.keys array -// even if we optimized it away. It also checks that we indeed optimize it away -// as we would expect. - -// Prevent GC from cancelling/discarding Ion compilations. -gczeal(0); - -function objKeysIterate(obj, expected, i) { - var keys = Object.keys(obj); - for (let i = 0; i < keys.length; i++) { - assertEq(keys[i], expected[i]); - } - assertRecoveredOnBailout(keys, true); -} - -function objKeysIterateMutated(obj, expected, i) { - var keys = Object.keys(obj); - obj.foo = 42; - for (let i = 0; i < keys.length; i++) { - assertEq(keys[i], expected[i]); - } - assertRecoveredOnBailout(keys, true); -} - -// Prevent compilation of the top-level. -eval(`${arraysEqual}`); -let obj = {a: 0, b: 1, c: 2, d: 3}; - -for (let i = 0; i < 100; i++) { - objKeysIterate({...obj}, ["a", "b", "c", "d"], i); - objKeysIterateMutated({...obj}, ["a", "b", "c", "d"], i); -} diff --git a/js/src/jit/CacheIR.cpp b/js/src/jit/CacheIR.cpp @@ -10463,15 +10463,8 @@ AttachDecision InlinableNativeIRGenerator::tryAttachObjectKeys() { // Guard against proxies. writer.guardIsNotProxy(argObjId); - Shape* expectedObjKeysShape = - GlobalObject::getArrayShapeWithDefaultProto(cx_); - if (!expectedObjKeysShape) { - cx_->recoverFromOutOfMemory(); - return AttachDecision::NoAction; - } - // Compute the keys array. - writer.objectKeysResult(argObjId, expectedObjKeysShape); + writer.objectKeysResult(argObjId); writer.returnFromIC(); diff --git a/js/src/jit/CacheIRCompiler.cpp b/js/src/jit/CacheIRCompiler.cpp @@ -2517,7 +2517,7 @@ bool CacheIRCompiler::emitIdToStringOrSymbol(ValOperandId resultId, LiveRegisterSet volatileRegs = liveVolatileRegs(); masm.PushRegsInMask(volatileRegs); - using Fn = JSLinearString* (*)(JSContext * cx, int32_t i); + using Fn = JSLinearString* (*)(JSContext* cx, int32_t i); masm.setupUnalignedABICall(scratch); masm.loadJSContext(scratch); masm.passABIArg(scratch); @@ -2943,7 +2943,7 @@ bool CacheIRCompiler::emitStringToAtom(StringOperandId stringId) { LiveRegisterSet save = liveVolatileRegs(); masm.PushRegsInMask(save); - using Fn = JSAtom* (*)(JSContext * cx, JSString * str); + using Fn = JSAtom* (*)(JSContext* cx, JSString* str); masm.setupUnalignedABICall(scratch); masm.loadJSContext(scratch); masm.passABIArg(scratch); @@ -3881,7 +3881,7 @@ static void EmitAllocateBigInt(MacroAssembler& masm, Register result, bool requestMinorGC = initialHeap == gc::Heap::Default; masm.PushRegsInMask(liveSet); - using Fn = void* (*)(JSContext * cx, bool requestMinorGC); + using Fn = void* (*)(JSContext* cx, bool requestMinorGC); masm.setupUnalignedABICall(temp); masm.loadJSContext(temp); masm.passABIArg(temp); @@ -6309,7 +6309,7 @@ void CacheIRCompiler::emitActivateIterator(Register objBeingIterated, #endif // Mark iterator as active. - Address iterFlagsAddr(nativeIter, NativeIterator::offsetOfFlags()); + Address iterFlagsAddr(nativeIter, NativeIterator::offsetOfFlagsAndCount()); masm.storePtr(objBeingIterated, iterObjAddr); masm.or32(Imm32(NativeIterator::Flags::Active), iterFlagsAddr); @@ -6340,7 +6340,7 @@ bool CacheIRCompiler::emitObjectToIteratorResult( Label callVM, done; masm.maybeLoadIteratorFromShape(obj, iterObj, scratch, scratch2, scratch3, - &callVM, /* exclusive = */ true); + &callVM); masm.loadPrivate( Address(iterObj, PropertyIteratorObject::offsetOfIteratorSlot()), @@ -6434,8 +6434,7 @@ bool CacheIRCompiler::emitObjectCreateResult(uint32_t templateObjectOffset) { return true; } -bool CacheIRCompiler::emitObjectKeysResult(ObjOperandId objId, - uint32_t resultShapeOffset) { +bool CacheIRCompiler::emitObjectKeysResult(ObjOperandId objId) { JitSpew(JitSpew_Codegen, "%s", __FUNCTION__); AutoCallVM callvm(masm, this, allocator); @@ -8361,7 +8360,7 @@ bool CacheIRCompiler::emitLoadTypeOfObjectResult(ObjOperandId objId) { LiveRegisterSet save = liveVolatileRegs(); masm.PushRegsInMask(save); - using Fn = JSString* (*)(JSObject * obj, JSRuntime * rt); + using Fn = JSString* (*)(JSObject* obj, JSRuntime* rt); masm.setupUnalignedABICall(scratch); masm.passABIArg(obj); masm.movePtr(ImmPtr(cx_->runtime()), scratch); @@ -9154,7 +9153,7 @@ bool CacheIRCompiler::emitWrapResult() { LiveRegisterSet save = liveVolatileRegs(); masm.PushRegsInMask(save); - using Fn = JSObject* (*)(JSContext * cx, JSObject * obj); + using Fn = JSObject* (*)(JSContext* cx, JSObject* obj); masm.setupUnalignedABICall(scratch); masm.loadJSContext(scratch); masm.passABIArg(scratch); @@ -10024,7 +10023,7 @@ bool CacheIRCompiler::emitCallInt32ToString(Int32OperandId inputId, volatileRegs.takeUnchecked(result); masm.PushRegsInMask(volatileRegs); - using Fn = JSLinearString* (*)(JSContext * cx, int32_t i); + using Fn = JSLinearString* (*)(JSContext* cx, int32_t i); masm.setupUnalignedABICall(result); masm.loadJSContext(result); masm.passABIArg(result); @@ -10059,7 +10058,7 @@ bool CacheIRCompiler::emitCallNumberToString(NumberOperandId inputId, volatileRegs.takeUnchecked(result); masm.PushRegsInMask(volatileRegs); - using Fn = JSString* (*)(JSContext * cx, double d); + using Fn = JSString* (*)(JSContext* cx, double d); masm.setupUnalignedABICall(result); masm.loadJSContext(result); masm.passABIArg(result); @@ -10429,7 +10428,7 @@ bool CacheIRCompiler::emitCallSubstringKernelResult(StringOperandId strId, masm.Push(begin); masm.Push(str); - using Fn = JSString* (*)(JSContext * cx, HandleString str, int32_t begin, + using Fn = JSString* (*)(JSContext* cx, HandleString str, int32_t begin, int32_t len); callvm.call<Fn, SubstringKernel>(); return true; diff --git a/js/src/jit/CacheIROps.yaml b/js/src/jit/CacheIROps.yaml @@ -1188,7 +1188,6 @@ cost_estimate: 6 args: obj: ObjId - resultShape: ShapeField - name: PackedArrayPopResult shared: true diff --git a/js/src/jit/CodeGenerator.cpp b/js/src/jit/CodeGenerator.cpp @@ -16652,24 +16652,6 @@ void CodeGenerator::visitIteratorMore(LIteratorMore* lir) { masm.iteratorMore(obj, output, temp); } -void CodeGenerator::visitIteratorLength(LIteratorLength* lir) { - Register obj = ToRegister(lir->iter()); - Register output = ToRegister(lir->output()); - masm.iteratorLength(obj, output); -} - -void CodeGenerator::visitLoadIteratorElement(LLoadIteratorElement* lir) { - Register obj = ToRegister(lir->iter()); - Register output = ToRegister(lir->output()); - if (lir->index()->isConstant()) { - int32_t index = ToInt32(lir->index()); - masm.iteratorLoadElement(obj, index, output); - } else { - Register index = ToRegister(lir->index()); - masm.iteratorLoadElement(obj, index, output); - } -} - void CodeGenerator::visitIsNoIterAndBranch(LIsNoIterAndBranch* lir) { ValueOperand input = ToValue(lir->input()); Label* ifTrue = getJumpLabelForBranch(lir->ifTrue()); @@ -18488,86 +18470,58 @@ void CodeGenerator::visitObjectToIterator(LObjectToIterator* lir) { Register temp3 = ToRegister(lir->temp2()); using Fn = PropertyIteratorObject* (*)(JSContext*, HandleObject); - OutOfLineCode* ool = nullptr; - - if (lir->mir()->skipRegistration()) { - if (lir->mir()->wantsIndices()) { - ool = oolCallVM<Fn, GetIteratorWithIndicesUnregistered>( - lir, ArgList(obj), StoreRegisterTo(iterObj)); - } else { - ool = oolCallVM<Fn, GetIteratorUnregistered>(lir, ArgList(obj), - StoreRegisterTo(iterObj)); - } - } else { - if (lir->mir()->wantsIndices()) { - ool = oolCallVM<Fn, GetIteratorWithIndices>(lir, ArgList(obj), - StoreRegisterTo(iterObj)); - } else { - ool = oolCallVM<Fn, GetIterator>(lir, ArgList(obj), - StoreRegisterTo(iterObj)); - } - } - -#ifdef DEBUG - if (!lir->mir()->getAliasSet().isStore()) { - MOZ_ASSERT(lir->mir()->skipRegistration()); - Label done; - masm.branchTestObjectIsProxy(false, obj, temp, &done); - masm.assumeUnreachable("ObjectToIterator on a proxy must be a store."); - masm.bind(&done); - } -#endif + OutOfLineCode* ool = (lir->mir()->wantsIndices()) + ? oolCallVM<Fn, GetIteratorWithIndices>( + lir, ArgList(obj), StoreRegisterTo(iterObj)) + : oolCallVM<Fn, GetIterator>( + lir, ArgList(obj), StoreRegisterTo(iterObj)); masm.maybeLoadIteratorFromShape(obj, iterObj, temp, temp2, temp3, - ool->entry(), - !lir->mir()->skipRegistration()); + ool->entry()); Register nativeIter = temp; masm.loadPrivate( Address(iterObj, PropertyIteratorObject::offsetOfIteratorSlot()), nativeIter); - Address iterFlagsAddr(nativeIter, NativeIterator::offsetOfFlags()); if (lir->mir()->wantsIndices()) { // At least one consumer of the output of this iterator has been optimized // to use iterator indices. If the cached iterator doesn't include indices, // but it was marked to indicate that we can create them if needed, then we // do a VM call to replace the cached iterator with a fresh iterator // including indices. - masm.branchTest32(Assembler::NonZero, iterFlagsAddr, - Imm32(NativeIterator::Flags::IndicesSupported), - ool->entry()); + masm.branchNativeIteratorIndices(Assembler::Equal, nativeIter, temp2, + NativeIteratorIndices::AvailableOnRequest, + ool->entry()); } - if (!lir->mir()->skipRegistration()) { - masm.storePtr(obj, Address(nativeIter, - NativeIterator::offsetOfObjectBeingIterated())); - masm.or32(Imm32(NativeIterator::Flags::Active), iterFlagsAddr); + Address iterFlagsAddr(nativeIter, NativeIterator::offsetOfFlagsAndCount()); + masm.storePtr( + obj, Address(nativeIter, NativeIterator::offsetOfObjectBeingIterated())); + masm.or32(Imm32(NativeIterator::Flags::Active), iterFlagsAddr); - Register enumeratorsAddr = temp2; - masm.movePtr(ImmPtr(lir->mir()->enumeratorsAddr()), enumeratorsAddr); - masm.registerIterator(enumeratorsAddr, nativeIter, temp3); + Register enumeratorsAddr = temp2; + masm.movePtr(ImmPtr(lir->mir()->enumeratorsAddr()), enumeratorsAddr); + masm.registerIterator(enumeratorsAddr, nativeIter, temp3); - // Generate post-write barrier for storing to - // |iterObj->objectBeingIterated_|. We already know that |iterObj| is - // tenured, so we only have to check |obj|. - Label skipBarrier; - masm.branchPtrInNurseryChunk(Assembler::NotEqual, obj, temp2, &skipBarrier); - { - LiveRegisterSet save = liveVolatileRegs(lir); - save.takeUnchecked(temp); - save.takeUnchecked(temp2); - save.takeUnchecked(temp3); - if (iterObj.volatile_()) { - save.addUnchecked(iterObj); - } - - masm.PushRegsInMask(save); - emitPostWriteBarrier(iterObj); - masm.PopRegsInMask(save); + // Generate post-write barrier for storing to |iterObj->objectBeingIterated_|. + // We already know that |iterObj| is tenured, so we only have to check |obj|. + Label skipBarrier; + masm.branchPtrInNurseryChunk(Assembler::NotEqual, obj, temp2, &skipBarrier); + { + LiveRegisterSet save = liveVolatileRegs(lir); + save.takeUnchecked(temp); + save.takeUnchecked(temp2); + save.takeUnchecked(temp3); + if (iterObj.volatile_()) { + save.addUnchecked(iterObj); } - masm.bind(&skipBarrier); + + masm.PushRegsInMask(save); + emitPostWriteBarrier(iterObj); + masm.PopRegsInMask(save); } + masm.bind(&skipBarrier); masm.bind(ool->rejoin()); } @@ -18592,14 +18546,13 @@ void CodeGenerator::visitIteratorHasIndicesAndBranch( Address nativeIterAddr(iterator, PropertyIteratorObject::offsetOfIteratorSlot()); masm.loadPrivate(nativeIterAddr, temp); - masm.branchTest32(Assembler::Zero, - Address(temp, NativeIterator::offsetOfFlags()), - Imm32(NativeIterator::Flags::IndicesAvailable), ifFalse); + masm.branchNativeIteratorIndices(Assembler::NotEqual, temp, temp2, + NativeIteratorIndices::Valid, ifFalse); // Guard that the first shape stored in the iterator matches the current // shape of the iterated object. - Address objShapeAddr(temp, NativeIterator::offsetOfObjectShape()); - masm.loadPtr(objShapeAddr, temp); + Address firstShapeAddr(temp, NativeIterator::offsetOfFirstShape()); + masm.loadPtr(firstShapeAddr, temp); masm.branchTestObjShape(Assembler::NotEqual, object, temp, temp2, object, ifFalse); @@ -18608,165 +18561,113 @@ void CodeGenerator::visitIteratorHasIndicesAndBranch( } } -void CodeGenerator::visitLoadSlotByIteratorIndexCommon(Register object, - Register indexScratch, - Register kindScratch, - ValueOperand result) { +void CodeGenerator::visitLoadSlotByIteratorIndex( + LLoadSlotByIteratorIndex* lir) { + Register object = ToRegister(lir->object()); + Register iterator = ToRegister(lir->iterator()); + Register temp = ToRegister(lir->temp0()); + Register temp2 = ToRegister(lir->temp1()); + ValueOperand result = ToOutValue(lir); + + masm.extractCurrentIndexAndKindFromIterator(iterator, temp, temp2); + Label notDynamicSlot, notFixedSlot, done; - masm.branch32(Assembler::NotEqual, kindScratch, + masm.branch32(Assembler::NotEqual, temp2, Imm32(uint32_t(PropertyIndex::Kind::DynamicSlot)), ¬DynamicSlot); - masm.loadPtr(Address(object, NativeObject::offsetOfSlots()), kindScratch); - masm.loadValue(BaseValueIndex(kindScratch, indexScratch), result); + masm.loadPtr(Address(object, NativeObject::offsetOfSlots()), temp2); + masm.loadValue(BaseValueIndex(temp2, temp), result); masm.jump(&done); masm.bind(¬DynamicSlot); - masm.branch32(Assembler::NotEqual, kindScratch, + masm.branch32(Assembler::NotEqual, temp2, Imm32(uint32_t(PropertyIndex::Kind::FixedSlot)), ¬FixedSlot); // Fixed slot - masm.loadValue(BaseValueIndex(object, indexScratch, sizeof(NativeObject)), - result); + masm.loadValue(BaseValueIndex(object, temp, sizeof(NativeObject)), result); masm.jump(&done); masm.bind(¬FixedSlot); #ifdef DEBUG Label kindOkay; - masm.branch32(Assembler::Equal, kindScratch, + masm.branch32(Assembler::Equal, temp2, Imm32(uint32_t(PropertyIndex::Kind::Element)), &kindOkay); masm.assumeUnreachable("Invalid PropertyIndex::Kind"); masm.bind(&kindOkay); #endif // Dense element - masm.loadPtr(Address(object, NativeObject::offsetOfElements()), kindScratch); + masm.loadPtr(Address(object, NativeObject::offsetOfElements()), temp2); Label indexOkay; - Address initLength(kindScratch, ObjectElements::offsetOfInitializedLength()); - masm.branch32(Assembler::Above, initLength, indexScratch, &indexOkay); + Address initLength(temp2, ObjectElements::offsetOfInitializedLength()); + masm.branch32(Assembler::Above, initLength, temp, &indexOkay); masm.assumeUnreachable("Dense element out of bounds"); masm.bind(&indexOkay); - masm.loadValue(BaseObjectElementIndex(kindScratch, indexScratch), result); + masm.loadValue(BaseObjectElementIndex(temp2, temp), result); masm.bind(&done); } -void CodeGenerator::visitLoadSlotByIteratorIndex( - LLoadSlotByIteratorIndex* lir) { - Register object = ToRegister(lir->object()); - Register iterator = ToRegister(lir->iterator()); - Register indexScratch = ToRegister(lir->temp0()); - Register kindScratch = ToRegister(lir->temp1()); - ValueOperand result = ToOutValue(lir); - - masm.extractCurrentIndexAndKindFromIterator(iterator, indexScratch, - kindScratch); - - visitLoadSlotByIteratorIndexCommon(object, indexScratch, kindScratch, result); -} - -#ifndef JS_CODEGEN_X86 -void CodeGenerator::visitLoadSlotByIteratorIndexIndexed( - LLoadSlotByIteratorIndexIndexed* lir) { +void CodeGenerator::visitStoreSlotByIteratorIndex( + LStoreSlotByIteratorIndex* lir) { Register object = ToRegister(lir->object()); Register iterator = ToRegister(lir->iterator()); - Register index = ToRegister(lir->index()); - Register indexScratch = ToRegister(lir->temp0()); - Register kindScratch = ToRegister(lir->temp1()); - ValueOperand result = ToOutValue(lir); - - masm.extractIndexAndKindFromIteratorByIterIndex(iterator, index, kindScratch, - indexScratch); + ValueOperand value = ToValue(lir->value()); + Register temp = ToRegister(lir->temp0()); + Register temp2 = ToRegister(lir->temp1()); - visitLoadSlotByIteratorIndexCommon(object, indexScratch, kindScratch, result); -} -#endif + masm.extractCurrentIndexAndKindFromIterator(iterator, temp, temp2); -void CodeGenerator::visitStoreSlotByIteratorIndexCommon(Register object, - Register indexScratch, - Register kindScratch, - ValueOperand value) { Label notDynamicSlot, notFixedSlot, done, doStore; - masm.branch32(Assembler::NotEqual, kindScratch, + masm.branch32(Assembler::NotEqual, temp2, Imm32(uint32_t(PropertyIndex::Kind::DynamicSlot)), ¬DynamicSlot); - masm.loadPtr(Address(object, NativeObject::offsetOfSlots()), kindScratch); - masm.computeEffectiveAddress(BaseValueIndex(kindScratch, indexScratch), - indexScratch); + masm.loadPtr(Address(object, NativeObject::offsetOfSlots()), temp2); + masm.computeEffectiveAddress(BaseValueIndex(temp2, temp), temp); masm.jump(&doStore); masm.bind(¬DynamicSlot); - masm.branch32(Assembler::NotEqual, kindScratch, + masm.branch32(Assembler::NotEqual, temp2, Imm32(uint32_t(PropertyIndex::Kind::FixedSlot)), ¬FixedSlot); // Fixed slot masm.computeEffectiveAddress( - BaseValueIndex(object, indexScratch, sizeof(NativeObject)), indexScratch); + BaseValueIndex(object, temp, sizeof(NativeObject)), temp); masm.jump(&doStore); masm.bind(¬FixedSlot); #ifdef DEBUG Label kindOkay; - masm.branch32(Assembler::Equal, kindScratch, + masm.branch32(Assembler::Equal, temp2, Imm32(uint32_t(PropertyIndex::Kind::Element)), &kindOkay); masm.assumeUnreachable("Invalid PropertyIndex::Kind"); masm.bind(&kindOkay); #endif // Dense element - masm.loadPtr(Address(object, NativeObject::offsetOfElements()), kindScratch); + masm.loadPtr(Address(object, NativeObject::offsetOfElements()), temp2); Label indexOkay; - Address initLength(kindScratch, ObjectElements::offsetOfInitializedLength()); - masm.branch32(Assembler::Above, initLength, indexScratch, &indexOkay); + Address initLength(temp2, ObjectElements::offsetOfInitializedLength()); + masm.branch32(Assembler::Above, initLength, temp, &indexOkay); masm.assumeUnreachable("Dense element out of bounds"); masm.bind(&indexOkay); - BaseObjectElementIndex elementAddress(kindScratch, indexScratch); - masm.computeEffectiveAddress(elementAddress, indexScratch); + BaseObjectElementIndex elementAddress(temp2, temp); + masm.computeEffectiveAddress(elementAddress, temp); masm.bind(&doStore); - Address storeAddress(indexScratch, 0); + Address storeAddress(temp, 0); emitPreBarrier(storeAddress); masm.storeValue(value, storeAddress); - masm.branchPtrInNurseryChunk(Assembler::Equal, object, kindScratch, &done); - masm.branchValueIsNurseryCell(Assembler::NotEqual, value, kindScratch, &done); + masm.branchPtrInNurseryChunk(Assembler::Equal, object, temp2, &done); + masm.branchValueIsNurseryCell(Assembler::NotEqual, value, temp2, &done); - saveVolatile(kindScratch); + saveVolatile(temp2); emitPostWriteBarrier(object); - restoreVolatile(kindScratch); + restoreVolatile(temp2); masm.bind(&done); } -void CodeGenerator::visitStoreSlotByIteratorIndex( - LStoreSlotByIteratorIndex* lir) { - Register object = ToRegister(lir->object()); - Register iterator = ToRegister(lir->iterator()); - ValueOperand value = ToValue(lir->value()); - Register indexScratch = ToRegister(lir->temp0()); - Register kindScratch = ToRegister(lir->temp1()); - - masm.extractCurrentIndexAndKindFromIterator(iterator, indexScratch, - kindScratch); - - visitStoreSlotByIteratorIndexCommon(object, indexScratch, kindScratch, value); -} - -#ifndef JS_CODEGEN_X86 -void CodeGenerator::visitStoreSlotByIteratorIndexIndexed( - LStoreSlotByIteratorIndexIndexed* lir) { - Register object = ToRegister(lir->object()); - Register iterator = ToRegister(lir->iterator()); - Register index = ToRegister(lir->index()); - ValueOperand value = ToValue(lir->value()); - Register indexScratch = ToRegister(lir->temp0()); - Register kindScratch = ToRegister(lir->temp1()); - - masm.extractIndexAndKindFromIteratorByIterIndex(iterator, index, kindScratch, - indexScratch); - - visitStoreSlotByIteratorIndexCommon(object, indexScratch, kindScratch, value); -} -#endif - void CodeGenerator::visitSetPropertyCache(LSetPropertyCache* ins) { LiveRegisterSet liveRegs = ins->safepoint()->liveRegs(); Register objReg = ToRegister(ins->object()); diff --git a/js/src/jit/CodeGenerator.h b/js/src/jit/CodeGenerator.h @@ -172,14 +172,6 @@ class CodeGenerator final : public CodeGeneratorSpecific { void visitPostWriteBarrierCommon(LPostBarrierType* lir, OutOfLineCode* ool); template <class LPostBarrierType> void visitPostWriteBarrierCommonV(LPostBarrierType* lir, OutOfLineCode* ool); - void visitLoadSlotByIteratorIndexCommon(Register object, - Register indexScratch, - Register kindScratch, - ValueOperand result); - void visitStoreSlotByIteratorIndexCommon(Register object, - Register indexScratch, - Register kindScratch, - ValueOperand value); void emitCallInvokeFunction(LInstruction* call, Register callereg, bool isConstructing, bool ignoresReturnValue, diff --git a/js/src/jit/Ion.cpp b/js/src/jit/Ion.cpp @@ -1084,21 +1084,6 @@ bool OptimizeMIR(MIRGenerator* mir) { } } - if (!JitOptions.disableRecoverIns && - mir->optimizationInfo().scalarReplacementEnabled() && - !JitOptions.disableObjectKeysScalarReplacement) { - JitSpewCont(JitSpew_Escape, "\n"); - if (!ReplaceObjectKeys(mir, graph)) { - return false; - } - mir->spewPass("Replace ObjectKeys"); - AssertGraphCoherency(graph); - - if (mir->shouldCancel("Replace ObjectKeys")) { - return false; - } - } - if (!mir->compilingWasm() && !JitOptions.disableIteratorIndices) { if (!OptimizeIteratorIndices(mir, graph)) { return false; diff --git a/js/src/jit/IonAnalysis.cpp b/js/src/jit/IonAnalysis.cpp @@ -5135,24 +5135,12 @@ bool jit::MakeLoopsContiguous(MIRGraph& graph) { return true; } -static MDefinition* SkipIterObjectUnbox(MDefinition* ins) { - if (ins->isGuardIsNotProxy()) { - ins = ins->toGuardIsNotProxy()->input(); - } +static MDefinition* SkipUnbox(MDefinition* ins) { if (ins->isUnbox()) { - ins = ins->toUnbox()->input(); - } - return ins; -} - -#ifndef JS_CODEGEN_X86 -static MDefinition* SkipBox(MDefinition* ins) { - if (ins->isBox()) { - return ins->toBox()->input(); + return ins->toUnbox()->input(); } return ins; } -#endif bool jit::OptimizeIteratorIndices(const MIRGenerator* mir, MIRGraph& graph) { bool changed = false; @@ -5201,8 +5189,8 @@ bool jit::OptimizeIteratorIndices(const MIRGenerator* mir, MIRGraph& graph) { // Given the following structure (that occurs inside for-in loops): // obj: some object // iter: ObjectToIterator <obj> - // iterLoad: IteratorMore <iter> | LoadIteratorElement <iter, index> - // access: HasProp/GetElem <obj> <iterLoad> + // iterNext: IteratorMore <iter> + // access: HasProp/GetElem <obj> <iterNext> // If the iterator object has an indices array, we can speed up the // property access: // 1. If the property access is a HasProp looking for own properties, @@ -5212,42 +5200,17 @@ bool jit::OptimizeIteratorIndices(const MIRGenerator* mir, MIRGraph& graph) { // 2. If the property access is a GetProp, then we can use the contents // of the indices array to find the correct property faster than // the megamorphic cache. - // 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. - - MObjectToIterator* iter = nullptr; -#ifndef JS_CODEGEN_X86 - MDefinition* iterElementIndex = nullptr; -#endif - if (idVal->isIteratorMore()) { - auto* iterNext = idVal->toIteratorMore(); - - if (!iterNext->iterator()->isObjectToIterator()) { - continue; - } - - iter = iterNext->iterator()->toObjectToIterator(); - if (SkipIterObjectUnbox(iter->object()) != - SkipIterObjectUnbox(receiver)) { - continue; - } -#ifndef JS_CODEGEN_X86 - } else if (SkipBox(idVal)->isLoadIteratorElement()) { - auto* iterLoad = SkipBox(idVal)->toLoadIteratorElement(); + if (!idVal->isIteratorMore()) { + continue; + } + auto* iterNext = idVal->toIteratorMore(); - if (!iterLoad->iter()->isObjectToIterator()) { - continue; - } + if (!iterNext->iterator()->isObjectToIterator()) { + continue; + } - iter = iterLoad->iter()->toObjectToIterator(); - if (SkipIterObjectUnbox(iter->object()) != - SkipIterObjectUnbox(receiver)) { - continue; - } - iterElementIndex = iterLoad->index(); -#endif - } else { + MObjectToIterator* iter = iterNext->iterator()->toObjectToIterator(); + if (SkipUnbox(iter->object()) != SkipUnbox(receiver)) { continue; } @@ -5260,33 +5223,13 @@ 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); - } else { - 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); - } else { - 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/JitOptions.cpp b/js/src/jit/JitOptions.cpp @@ -132,9 +132,6 @@ DefaultJitOptions::DefaultJitOptions() { // Whether the Baseline Interpreter is enabled. SET_DEFAULT(baselineInterpreter, true); - // Whether replacing Object.keys with NativeIterators is globally disabled. - SET_DEFAULT(disableObjectKeysScalarReplacement, false); - #ifdef ENABLE_PORTABLE_BASELINE_INTERP // Whether the Portable Baseline Interpreter is enabled. SET_DEFAULT(portableBaselineInterpreter, false); diff --git a/js/src/jit/JitOptions.h b/js/src/jit/JitOptions.h @@ -70,7 +70,6 @@ struct DefaultJitOptions { bool disableRedundantShapeGuards; bool disableRedundantGCBarriers; bool disableBailoutLoopCheck; - bool disableObjectKeysScalarReplacement; #ifdef ENABLE_PORTABLE_BASELINE_INTERP bool portableBaselineInterpreter; #endif diff --git a/js/src/jit/Lowering.cpp b/js/src/jit/Lowering.cpp @@ -5040,10 +5040,6 @@ void LIRGenerator::visitObjectKeys(MObjectKeys* ins) { assignSafepoint(lir, ins); } -void LIRGenerator::visitObjectKeysFromIterator(MObjectKeysFromIterator* ins) { - MOZ_CRASH("ObjectKeysFromIterator is purely for recovery purposes."); -} - void LIRGenerator::visitObjectKeysLength(MObjectKeysLength* ins) { MOZ_ASSERT(ins->object()->type() == MIRType::Object); MOZ_ASSERT(ins->type() == MIRType::Int32); @@ -6271,24 +6267,6 @@ void LIRGenerator::visitStoreSlotByIteratorIndex( add(lir, ins); } -#ifndef JS_CODEGEN_X86 -void LIRGenerator::visitLoadSlotByIteratorIndexIndexed( - MLoadSlotByIteratorIndexIndexed* ins) { - auto* lir = new (alloc()) LLoadSlotByIteratorIndexIndexed( - useRegister(ins->object()), useRegister(ins->iterator()), - useRegister(ins->index()), temp(), temp()); - defineBox(lir, ins); -} - -void LIRGenerator::visitStoreSlotByIteratorIndexIndexed( - MStoreSlotByIteratorIndexIndexed* ins) { - auto* lir = new (alloc()) LStoreSlotByIteratorIndexIndexed( - useRegister(ins->object()), useRegister(ins->iterator()), - useRegister(ins->index()), useBox(ins->value()), temp(), temp()); - add(lir, ins); -} -#endif - void LIRGenerator::visitIteratorHasIndices(MIteratorHasIndices* ins) { MOZ_ASSERT(ins->hasOneUse()); emitAtUses(ins); @@ -6387,24 +6365,6 @@ void LIRGenerator::visitCloseIterCache(MCloseIterCache* ins) { assignSafepoint(lir, ins); } -void LIRGenerator::visitLoadIteratorElement(MLoadIteratorElement* ins) { - MOZ_ASSERT(ins->iter()->type() == MIRType::Object); - MOZ_ASSERT(ins->index()->type() == MIRType::Int32); - MOZ_ASSERT(ins->type() == MIRType::String); - - auto* lir = new (alloc()) LLoadIteratorElement( - useRegister(ins->iter()), useRegisterOrConstant(ins->index())); - define(lir, ins); -} - -void LIRGenerator::visitIteratorLength(MIteratorLength* ins) { - MOZ_ASSERT(ins->iter()->type() == MIRType::Object); - MOZ_ASSERT(ins->type() == MIRType::Int32); - - auto* lir = new (alloc()) LIteratorLength(useRegister(ins->iter())); - define(lir, ins); -} - void LIRGenerator::visitOptimizeGetIteratorCache( MOptimizeGetIteratorCache* ins) { MDefinition* value = ins->value(); diff --git a/js/src/jit/MIR.cpp b/js/src/jit/MIR.cpp @@ -7849,18 +7849,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); -} - -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/MIR.h b/js/src/jit/MIR.h @@ -9710,7 +9710,6 @@ class MObjectToIterator : public MUnaryInstruction, public ObjectPolicy<0>::Data { NativeIteratorListHead* enumeratorsAddr_; bool wantsIndices_ = false; - bool skipRegistration_ = false; explicit MObjectToIterator(MDefinition* object, NativeIteratorListHead* enumeratorsAddr) @@ -9725,17 +9724,8 @@ class MObjectToIterator : public MUnaryInstruction, TRIVIAL_NEW_WRAPPERS NAMED_OPERANDS((0, object)) - AliasSet getAliasSet() const override { - return skipRegistration_ - ? AliasSet::Load(AliasSet::ObjectFields | AliasSet::Element) - : AliasSet::Store(AliasSet::Any); - } - bool wantsIndices() const { return wantsIndices_; } void setWantsIndices(bool value) { wantsIndices_ = value; } - - bool skipRegistration() const { return skipRegistration_; } - void setSkipRegistration(bool value) { skipRegistration_ = value; } }; class MPostIntPtrConversion : public MUnaryInstruction, diff --git a/js/src/jit/MIROps.yaml b/js/src/jit/MIROps.yaml @@ -2073,16 +2073,13 @@ - name: ObjectKeys operands: object: Object - arguments: - resultShape: Shape* result_type: Object - can_recover: custom -- name: ObjectKeysFromIterator - operands: - iterator: Object - result_type: Object - can_recover: true + # We can recover Object.keys on bailout as long as the object is not escaped + # or that the object is not mutated by any aliased calls in-between the + # instruction and the recovered location, as the recovered keys array might be + # different. + can_recover: custom # Used to fold Object.keys(obj).length into a single operation. # @@ -2800,20 +2797,6 @@ - name: ObjectToIterator gen_boilerplate: false -- name: IteratorLength - operands: - iter: Object - result_type: Int32 - generate_lir: true - -- name: LoadIteratorElement - operands: - iter: Object - index: Int32 - result_type: String - alias_set: none - generate_lir: true - - name: ValueToIterator operands: value: Value @@ -2844,37 +2827,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 -# key indexed by `index`, rather than the current one. -- name: LoadSlotByIteratorIndexIndexed - operands: - object: Object - iterator: Object - index: Int32 - result_type: Value - alias_set: custom - generate_lir: true - lir_temps: 2 - -- name: StoreSlotByIteratorIndexIndexed - operands: - object: Object - iterator: Object - index: Int32 - value: Value - 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. # This is either an UndefinedValue (no expando), ObjectValue (the expando diff --git a/js/src/jit/MacroAssembler.cpp b/js/src/jit/MacroAssembler.cpp @@ -3244,72 +3244,24 @@ void MacroAssembler::extractCurrentIndexAndKindFromIterator(Register iterator, PropertyIteratorObject::offsetOfIteratorSlot()); loadPrivate(nativeIterAddr, outIndex); - // Load the property count into outKind. - load32(Address(outIndex, NativeIterator::offsetOfPropertyCount()), outKind); - - // We need two bits of wiggle room in a u32 here for the logic below. - static_assert(NativeIterator::PropCountLimit <= 1 << 30); - - // Shift up the property count on 64 bit. Ultimately we want - // sizeof(IteratorProperty) * count + sizeof(PropertyIndex) * cursor. - // If we shift up our count to be on the same scale as cursor right now, - // we can do this all with one register. - static_assert(sizeof(IteratorProperty) == sizeof(PropertyIndex) || - sizeof(IteratorProperty) == sizeof(PropertyIndex) * 2); - if constexpr (sizeof(IteratorProperty) > sizeof(PropertyIndex)) { - lshift32(Imm32(1), outKind); - } - - // Add the current cursor. This is a uint32_t which has already been - // incremented in iteration to index the *next* property, so we'll want to - // keep that in mind in our final address calculation. - add32(Address(outIndex, NativeIterator::offsetOfPropertyCursor()), outKind); - - // outKind holds the offset in u32's to our PropertyIndex, so just multiply - // by four, add it to the offset of the first property, and subtract a - // PropertyIndex since we know we already incremented. - load32(BaseIndex(outIndex, outKind, Scale::TimesFour, - NativeIterator::offsetOfFirstProperty() - - int32_t(sizeof(PropertyIndex))), - outIndex); - - // Extract kind. - rshift32(Imm32(PropertyIndex::KindShift), outIndex, outKind); - - // Extract index. - and32(Imm32(PropertyIndex::IndexMask), outIndex); -} - -void MacroAssembler::extractIndexAndKindFromIteratorByIterIndex( - Register iterator, Register inIndex, Register outKind, Register outIndex) { - // Load iterator object - Address nativeIterAddr(iterator, - PropertyIteratorObject::offsetOfIteratorSlot()); - loadPrivate(nativeIterAddr, outIndex); - - // Load the property count into outKind. - load32(Address(outIndex, NativeIterator::offsetOfPropertyCount()), outKind); - - // We need two bits of wiggle room in a u32 here for the logic below. - static_assert(NativeIterator::PropCountLimit <= 1 << 30); - - // Shift up the property count on 64 bit. Ultimately we want - // sizeof(IteratorProperty) * count + sizeof(PropertyIndex) * cursor. - // If we shift up our count to be on the same scale as cursor right now, - // we can do this all with one register. - static_assert(sizeof(IteratorProperty) == sizeof(PropertyIndex) || - sizeof(IteratorProperty) == sizeof(PropertyIndex) * 2); - if constexpr (sizeof(IteratorProperty) > sizeof(PropertyIndex)) { - lshift32(Imm32(1), outKind); - } - - // Add the index - add32(inIndex, outKind); - - // outKind holds the offset in u32's to our PropertyIndex, so just multiply - // by four and add it to the offset of the first property - load32(BaseIndex(outIndex, outKind, Scale::TimesFour, - NativeIterator::offsetOfFirstProperty()), + // Compute offset of propertyCursor_ from propertiesBegin() + loadPtr(Address(outIndex, NativeIterator::offsetOfPropertyCursor()), outKind); + subPtr(Address(outIndex, NativeIterator::offsetOfShapesEnd()), outKind); + + // Compute offset of current index from indicesBegin(). Note that because + // propertyCursor has already been incremented, this is actually the offset + // of the next index. We adjust accordingly below. + size_t indexAdjustment = + sizeof(GCPtr<JSLinearString*>) / sizeof(PropertyIndex); + if (indexAdjustment != 1) { + MOZ_ASSERT(indexAdjustment == 2); + rshift32(Imm32(1), outKind); + } + + // Load current index. + loadPtr(Address(outIndex, NativeIterator::offsetOfPropertiesEnd()), outIndex); + load32(BaseIndex(outIndex, outKind, Scale::TimesOne, + -int32_t(sizeof(PropertyIndex))), outIndex); // Extract kind. @@ -9398,7 +9350,7 @@ void MacroAssembler::branchIfResizableArrayBufferViewInBounds(Register obj, void MacroAssembler::branchIfNativeIteratorNotReusable(Register ni, Label* notReusable) { // See NativeIterator::isReusable. - Address flagsAddr(ni, NativeIterator::offsetOfFlags()); + Address flagsAddr(ni, NativeIterator::offsetOfFlagsAndCount()); #ifdef DEBUG Label niIsInitialized; @@ -9414,6 +9366,17 @@ void MacroAssembler::branchIfNativeIteratorNotReusable(Register ni, Imm32(NativeIterator::Flags::NotReusable), notReusable); } +void MacroAssembler::branchNativeIteratorIndices(Condition cond, Register ni, + Register temp, + NativeIteratorIndices kind, + Label* label) { + Address iterFlagsAddr(ni, NativeIterator::offsetOfFlagsAndCount()); + load32(iterFlagsAddr, temp); + and32(Imm32(NativeIterator::IndicesMask), temp); + uint32_t shiftedKind = uint32_t(kind) << NativeIterator::IndicesShift; + branch32(cond, temp, Imm32(shiftedKind), label); +} + static void LoadNativeIterator(MacroAssembler& masm, Register obj, Register dest) { MOZ_ASSERT(obj != dest); @@ -9443,8 +9406,8 @@ static void LoadNativeIterator(MacroAssembler& masm, Register obj, // Otherwise, jump to |failure|. void MacroAssembler::maybeLoadIteratorFromShape(Register obj, Register dest, Register temp, Register temp2, - Register temp3, Label* failure, - bool exclusive) { + Register temp3, + Label* failure) { // Register usage: // obj: always contains the input object // temp: walks the obj->shape->baseshape->proto->shape->... chain @@ -9480,30 +9443,18 @@ void MacroAssembler::maybeLoadIteratorFromShape(Register obj, Register dest, // Load the native iterator and verify that it's reusable. andPtr(Imm32(~ShapeCachePtr::MASK), dest); LoadNativeIterator(*this, dest, nativeIterator); - - if (exclusive) { - branchIfNativeIteratorNotReusable(nativeIterator, failure); - } - - Label skipIndices; - load32(Address(nativeIterator, NativeIterator::offsetOfPropertyCount()), - temp3); - branchTest32(Assembler::Zero, - Address(nativeIterator, NativeIterator::offsetOfFlags()), - Imm32(NativeIterator::Flags::IndicesAllocated), &skipIndices); - - computeEffectiveAddress(BaseIndex(nativeIterator, temp3, Scale::TimesFour), - nativeIterator); - - bind(&skipIndices); - computeEffectiveAddress(BaseIndex(nativeIterator, temp3, ScalePointer, - NativeIterator::offsetOfFirstProperty()), - nativeIterator); - - Register expectedProtoShape = nativeIterator; + branchIfNativeIteratorNotReusable(nativeIterator, failure); // We have to compare the shapes in the native iterator with the shapes on the - // proto chain to ensure the cached iterator is still valid. + // proto chain to ensure the cached iterator is still valid. The shape array + // always starts at a fixed offset from the base of the NativeIterator, so + // instead of using an instruction outside the loop to initialize a pointer to + // the shapes array, we can bake it into the offset and reuse the pointer to + // the NativeIterator. We add |sizeof(Shape*)| to start at the second shape. + // (The first shape corresponds to the object itself. We don't have to check + // it, because we got the iterator via the shape.) + size_t nativeIteratorProtoShapeOffset = + NativeIterator::offsetOfFirstShape() + sizeof(Shape*); // Loop over the proto chain. At the head of the loop, |shape| is the shape of // the current object, and |iteratorShapes| points to the expected shape of @@ -9530,11 +9481,11 @@ void MacroAssembler::maybeLoadIteratorFromShape(Register obj, Register dest, // Compare the shape of the proto to the expected shape. loadPtr(Address(shapeAndProto, JSObject::offsetOfShape()), shapeAndProto); - loadPtr(Address(expectedProtoShape, 0), temp3); + loadPtr(Address(nativeIterator, nativeIteratorProtoShapeOffset), temp3); branchPtr(Assembler::NotEqual, shapeAndProto, temp3, failure); // Increment |iteratorShapes| and jump back to the top of the loop. - addPtr(Imm32(sizeof(Shape*)), expectedProtoShape); + addPtr(Imm32(sizeof(Shape*)), nativeIterator); jump(&protoLoop); #ifdef DEBUG @@ -9556,17 +9507,15 @@ void MacroAssembler::iteratorMore(Register obj, ValueOperand output, Label iterDone, restart; bind(&restart); Address cursorAddr(outputScratch, NativeIterator::offsetOfPropertyCursor()); - Address cursorEndAddr(outputScratch, NativeIterator::offsetOfPropertyCount()); - load32(cursorAddr, temp); - branch32(Assembler::BelowOrEqual, cursorEndAddr, temp, &iterDone); + Address cursorEndAddr(outputScratch, NativeIterator::offsetOfPropertiesEnd()); + loadPtr(cursorAddr, temp); + branchPtr(Assembler::BelowOrEqual, cursorEndAddr, temp, &iterDone); // Get next string. - BaseIndex propAddr(outputScratch, temp, ScalePointer, - NativeIterator::offsetOfFirstProperty()); - loadPtr(propAddr, temp); + loadPtr(Address(temp, 0), temp); // Increase the cursor. - addPtr(Imm32(1), cursorAddr); + addPtr(Imm32(sizeof(IteratorProperty)), cursorAddr); // Check if the property has been deleted while iterating. Skip it if so. branchTestPtr(Assembler::NonZero, temp, @@ -9581,34 +9530,11 @@ void MacroAssembler::iteratorMore(Register obj, ValueOperand output, bind(&done); } -void MacroAssembler::iteratorLength(Register obj, Register output) { - LoadNativeIterator(*this, obj, output); - load32(Address(output, NativeIterator::offsetOfOwnPropertyCount()), output); -} - -void MacroAssembler::iteratorLoadElement(Register obj, Register index, - Register output) { - LoadNativeIterator(*this, obj, output); - loadPtr(BaseIndex(output, index, ScalePointer, - NativeIterator::offsetOfFirstProperty()), - output); - andPtr(Imm32(int32_t(~IteratorProperty::DeletedBit)), output); -} - -void MacroAssembler::iteratorLoadElement(Register obj, int32_t index, - Register output) { - LoadNativeIterator(*this, obj, output); - loadPtr(Address(output, index * sizeof(IteratorProperty) + - NativeIterator::offsetOfFirstProperty()), - output); - andPtr(Imm32(int32_t(~IteratorProperty::DeletedBit)), output); -} - void MacroAssembler::iteratorClose(Register obj, Register temp1, Register temp2, Register temp3) { LoadNativeIterator(*this, obj, temp1); - Address flagsAddr(temp1, NativeIterator::offsetOfFlags()); + Address flagsAddr(temp1, NativeIterator::offsetOfFlagsAndCount()); // The shared iterator used for for-in with null/undefined is immutable and // unlinked. See NativeIterator::isEmptyIteratorSingleton. @@ -9622,7 +9548,8 @@ void MacroAssembler::iteratorClose(Register obj, Register temp1, Register temp2, storePtr(ImmPtr(nullptr), iterObjAddr); // Reset property cursor. - store32(Imm32(0), Address(temp1, NativeIterator::offsetOfPropertyCursor())); + loadPtr(Address(temp1, NativeIterator::offsetOfShapesEnd()), temp2); + storePtr(temp2, Address(temp1, NativeIterator::offsetOfPropertyCursor())); // Clear deleted bits (only if we have unvisited deletions) Label clearDeletedLoopStart, clearDeletedLoopEnd; @@ -9630,13 +9557,7 @@ void MacroAssembler::iteratorClose(Register obj, Register temp1, Register temp2, Imm32(NativeIterator::Flags::HasUnvisitedPropertyDeletion), &clearDeletedLoopEnd); - load32(Address(temp1, NativeIterator::offsetOfPropertyCount()), temp3); - - computeEffectiveAddress(BaseIndex(temp1, temp3, ScalePointer, - NativeIterator::offsetOfFirstProperty()), - temp3); - computeEffectiveAddress( - Address(temp1, NativeIterator::offsetOfFirstProperty()), temp2); + loadPtr(Address(temp1, NativeIterator::offsetOfPropertiesEnd()), temp3); bind(&clearDeletedLoopStart); and32(Imm32(~uint32_t(IteratorProperty::DeletedBit)), Address(temp2, 0)); diff --git a/js/src/jit/MacroAssembler.h b/js/src/jit/MacroAssembler.h @@ -221,6 +221,8 @@ namespace js { class StaticStrings; class FixedLengthTypedArrayObject; +enum class NativeIteratorIndices : uint32_t; + namespace wasm { class CalleeDesc; class CallSiteDesc; @@ -1112,7 +1114,6 @@ class MacroAssembler : public MacroAssemblerSpecific { // explicitly requested. Instead use branch(Add|Sub|Mul|Neg) to test for // condition flags after performing arithmetic operations. - inline void add32(const Address& src, Register dest) PER_SHARED_ARCH; inline void add32(Register src, Register dest) PER_SHARED_ARCH; inline void add32(Imm32 imm, Register dest) PER_SHARED_ARCH; inline void add32(Imm32 imm, Register src, Register dest) PER_SHARED_ARCH; @@ -5394,17 +5395,16 @@ class MacroAssembler : public MacroAssemblerSpecific { Label* label); void branchIfNativeIteratorNotReusable(Register ni, Label* notReusable); + void branchNativeIteratorIndices(Condition cond, Register ni, Register temp, + NativeIteratorIndices kind, Label* label); void maybeLoadIteratorFromShape(Register obj, Register dest, Register temp, Register temp2, Register temp3, - Label* failure, bool exclusive); + Label* failure); void iteratorMore(Register obj, ValueOperand output, Register temp); void iteratorClose(Register obj, Register temp1, Register temp2, Register temp3); - void iteratorLength(Register obj, Register output); - void iteratorLoadElement(Register obj, Register index, Register output); - void iteratorLoadElement(Register obj, int32_t index, Register output); void registerIterator(Register enumeratorsList, Register iter, Register temp); void prepareOOBStoreElement(Register object, Register index, @@ -5723,10 +5723,6 @@ class MacroAssembler : public MacroAssemblerSpecific { void extractCurrentIndexAndKindFromIterator(Register iterator, Register outIndex, Register outKind); - void extractIndexAndKindFromIteratorByIterIndex(Register iterator, - Register inOutIndex, - Register outKind, - Register scratch); template <typename IdType> #ifdef JS_CODEGEN_X86 diff --git a/js/src/jit/Recover.cpp b/js/src/jit/Recover.cpp @@ -2279,28 +2279,6 @@ bool RObjectKeys::recover(JSContext* cx, SnapshotIterator& iter) const { return true; } -bool MObjectKeysFromIterator::writeRecoverData( - CompactBufferWriter& writer) const { - MOZ_ASSERT(canRecoverOnBailout()); - writer.writeUnsigned(uint32_t(RInstruction::Recover_ObjectKeysFromIterator)); - return true; -} - -RObjectKeysFromIterator::RObjectKeysFromIterator(CompactBufferReader& reader) {} - -bool RObjectKeysFromIterator::recover(JSContext* cx, - SnapshotIterator& iter) const { - Rooted<JSObject*> iterObj(cx, iter.readObject()); - - JSObject* resultKeys = ObjectKeysFromIterator(cx, iterObj); - if (!resultKeys) { - return false; - } - - iter.storeInstructionResult(ObjectValue(*resultKeys)); - return true; -} - bool MObjectState::writeRecoverData(CompactBufferWriter& writer) const { MOZ_ASSERT(canRecoverOnBailout()); writer.writeUnsigned(uint32_t(RInstruction::Recover_ObjectState)); diff --git a/js/src/jit/Recover.h b/js/src/jit/Recover.h @@ -145,7 +145,6 @@ namespace jit { _(Callee) \ _(FunctionEnvironment) \ _(ObjectKeys) \ - _(ObjectKeysFromIterator) \ _(ObjectState) \ _(ArrayState) \ _(AtomicIsLockFree) \ @@ -1006,14 +1005,6 @@ class RObjectKeys final : public RInstruction { SnapshotIterator& iter) const override; }; -class RObjectKeysFromIterator final : public RInstruction { - public: - RINSTRUCTION_HEADER_NUM_OP_(ObjectKeysFromIterator, 1) - - [[nodiscard]] bool recover(JSContext* cx, - SnapshotIterator& iter) const override; -}; - class RObjectState final : public RInstruction { private: uint32_t numSlots_; // Number of slots. diff --git a/js/src/jit/ScalarReplacement.cpp b/js/src/jit/ScalarReplacement.cpp @@ -1262,146 +1262,22 @@ static bool IsArrayEscaped(MInstruction* ins, MInstruction* newArray) { return false; } -// This is just a class designed to extract the common elements across -// several different Array replacement strategies to avoid code duplication. -// There is nothing essential or sacred about it, it just felt like this -// was some pretty basic stuff we often want to do when we're replacing a -// true JS array with something which cheaply approximates it. When -// inheriting from this in the future, please validate that each of its -// core visit functions is safe to do in your new context. -class GenericArrayReplacer : public MDefinitionVisitorDefaultNoop { - protected: - TempAllocator& alloc_; - MInstruction* arr_; - - bool isTargetElements(MDefinition* elements); - void discardInstruction(MInstruction* ins, MDefinition* elements); - void visitLength(MInstruction* ins, MDefinition* elements); - - GenericArrayReplacer(TempAllocator& alloc, MInstruction* arr) - : alloc_(alloc), arr_(arr) {} - - public: - void visitGuardToClass(MGuardToClass* ins); - void visitGuardShape(MGuardShape* ins); - void visitGuardArrayIsPacked(MGuardArrayIsPacked* ins); - void visitUnbox(MUnbox* ins); - void visitCompare(MCompare* ins); - void visitGuardElementsArePacked(MGuardElementsArePacked* ins); -}; - -bool GenericArrayReplacer::isTargetElements(MDefinition* elements) { - return elements->isElements() && elements->toElements()->object() == arr_; -} - -void GenericArrayReplacer::discardInstruction(MInstruction* ins, - MDefinition* elements) { - MOZ_ASSERT(elements->isElements()); - ins->block()->discard(ins); - if (!elements->hasLiveDefUses()) { - elements->block()->discard(elements->toInstruction()); - } -} - -void GenericArrayReplacer::visitGuardToClass(MGuardToClass* ins) { - // Skip guards on other objects. - if (ins->object() != arr_) { - return; - } - MOZ_ASSERT(ins->getClass() == &ArrayObject::class_); - - // Replace the guard with the array object. - ins->replaceAllUsesWith(arr_); - - // Remove the guard. - ins->block()->discard(ins); -} - -void GenericArrayReplacer::visitGuardShape(MGuardShape* ins) { - // Skip guards on other objects. - if (ins->object() != arr_) { - return; - } - - // Replace the guard with the array object. - ins->replaceAllUsesWith(arr_); - - // Remove the guard. - ins->block()->discard(ins); -} - -void GenericArrayReplacer::visitGuardArrayIsPacked(MGuardArrayIsPacked* ins) { - // Skip guards on other objects. - if (ins->array() != arr_) { - return; - } - - // Replace the guard by its object. - ins->replaceAllUsesWith(arr_); - - // Remove original instruction. - ins->block()->discard(ins); -} - -void GenericArrayReplacer::visitUnbox(MUnbox* ins) { - // Skip unrelated unboxes. - if (ins->input() != arr_) { - return; - } - MOZ_ASSERT(ins->type() == MIRType::Object); - - // Replace the unbox with the array object. - ins->replaceAllUsesWith(arr_); - - // Remove the unbox. - ins->block()->discard(ins); -} - -void GenericArrayReplacer::visitCompare(MCompare* ins) { - // Skip unrelated comparisons. - if (ins->lhs() != arr_ && ins->rhs() != arr_) { - return; - } - - bool folded; - MOZ_ALWAYS_TRUE(ins->tryFold(&folded)); - - auto* cst = MConstant::NewBoolean(alloc_, folded); - ins->block()->insertBefore(ins, cst); - - // Replace the comparison with a constant. - ins->replaceAllUsesWith(cst); - - // Remove original instruction. - ins->block()->discard(ins); -} - -void GenericArrayReplacer::visitGuardElementsArePacked( - MGuardElementsArePacked* ins) { - // Skip other array objects. - MDefinition* elements = ins->elements(); - if (!isTargetElements(elements)) { - return; - } - - // Remove original instruction. - discardInstruction(ins, elements); -} - // This class replaces every MStoreElement and MSetInitializedLength by an // MArrayState which emulates the content of the array. All MLoadElement, // MInitializedLength and MArrayLength are replaced by the corresponding value. // // In order to restore the value of the array correctly in case of bailouts, we // replace all reference of the allocation by the MArrayState definition. -class ArrayMemoryView : public GenericArrayReplacer { +class ArrayMemoryView : public MDefinitionVisitorDefaultNoop { public: using BlockState = MArrayState; static const char* phaseName; private: + TempAllocator& alloc_; MConstant* undefinedVal_; MConstant* length_; + MInstruction* arr_; MBasicBlock* startBlock_; BlockState* state_; @@ -1439,9 +1315,15 @@ class ArrayMemoryView : public GenericArrayReplacer { void visitLoadElement(MLoadElement* ins); void visitSetInitializedLength(MSetInitializedLength* ins); void visitInitializedLength(MInitializedLength* ins); + void visitGuardElementsArePacked(MGuardElementsArePacked* ins); void visitArrayLength(MArrayLength* ins); void visitPostWriteBarrier(MPostWriteBarrier* ins); void visitPostWriteElementBarrier(MPostWriteElementBarrier* ins); + void visitGuardShape(MGuardShape* ins); + void visitGuardToClass(MGuardToClass* ins); + void visitGuardArrayIsPacked(MGuardArrayIsPacked* ins); + void visitUnbox(MUnbox* ins); + void visitCompare(MCompare* ins); void visitApplyArray(MApplyArray* ins); void visitConstructArray(MConstructArray* ins); }; @@ -1449,9 +1331,10 @@ class ArrayMemoryView : public GenericArrayReplacer { const char* ArrayMemoryView::phaseName = "Scalar Replacement of Array"; ArrayMemoryView::ArrayMemoryView(TempAllocator& alloc, MInstruction* arr) - : GenericArrayReplacer(alloc, arr), + : alloc_(alloc), undefinedVal_(nullptr), length_(nullptr), + arr_(arr), startBlock_(arr->block()), state_(nullptr), lastResumePoint_(nullptr), @@ -1701,6 +1584,18 @@ void ArrayMemoryView::visitInitializedLength(MInitializedLength* ins) { discardInstruction(ins, elements); } +void ArrayMemoryView::visitGuardElementsArePacked( + MGuardElementsArePacked* ins) { + // Skip other array objects. + MDefinition* elements = ins->elements(); + if (!isArrayStateElements(elements)) { + return; + } + + // Remove original instruction. + discardInstruction(ins, elements); +} + void ArrayMemoryView::visitArrayLength(MArrayLength* ins) { // Skip other array objects. MDefinition* elements = ins->elements(); @@ -1740,6 +1635,78 @@ void ArrayMemoryView::visitPostWriteElementBarrier( ins->block()->discard(ins); } +void ArrayMemoryView::visitGuardShape(MGuardShape* ins) { + // Skip guards on other objects. + if (ins->object() != arr_) { + return; + } + + // Replace the guard by its object. + ins->replaceAllUsesWith(arr_); + + // Remove original instruction. + ins->block()->discard(ins); +} + +void ArrayMemoryView::visitGuardToClass(MGuardToClass* ins) { + // Skip guards on other objects. + if (ins->object() != arr_) { + return; + } + + // Replace the guard by its object. + ins->replaceAllUsesWith(arr_); + + // Remove original instruction. + ins->block()->discard(ins); +} + +void ArrayMemoryView::visitGuardArrayIsPacked(MGuardArrayIsPacked* ins) { + // Skip guards on other objects. + if (ins->array() != arr_) { + return; + } + + // Replace the guard by its object. + ins->replaceAllUsesWith(arr_); + + // Remove original instruction. + ins->block()->discard(ins); +} + +void ArrayMemoryView::visitUnbox(MUnbox* ins) { + // Skip unrelated unboxes. + if (ins->getOperand(0) != arr_) { + return; + } + MOZ_ASSERT(ins->type() == MIRType::Object); + + // Replace the unbox with the array object. + ins->replaceAllUsesWith(arr_); + + // Remove the unbox. + ins->block()->discard(ins); +} + +void ArrayMemoryView::visitCompare(MCompare* ins) { + // Skip unrelated comparisons. + if (ins->lhs() != arr_ && ins->rhs() != arr_) { + return; + } + + bool folded; + MOZ_ALWAYS_TRUE(ins->tryFold(&folded)); + + auto* cst = MConstant::NewBoolean(alloc_, folded); + ins->block()->insertBefore(ins, cst); + + // Replace the comparison with a constant. + ins->replaceAllUsesWith(cst); + + // Remove original instruction. + ins->block()->discard(ins); +} + void ArrayMemoryView::visitApplyArray(MApplyArray* ins) { // Skip other array objects. MDefinition* elements = ins->getElements(); @@ -2604,18 +2571,29 @@ static inline bool IsOptimizableRestInstruction(MInstruction* ins) { return ins->isRest(); } -class RestReplacer : public GenericArrayReplacer { +class RestReplacer : public MDefinitionVisitorDefaultNoop { private: const MIRGenerator* mir_; MIRGraph& graph_; + MInstruction* rest_; - MRest* rest() const { return arr_->toRest(); } - MDefinition* restLength(MInstruction* ins); + TempAllocator& alloc() { return graph_.alloc(); } + MRest* rest() const { return rest_->toRest(); } + bool isRestElements(MDefinition* elements); + void discardInstruction(MInstruction* ins, MDefinition* elements); + MDefinition* restLength(MInstruction* ins); void visitLength(MInstruction* ins, MDefinition* elements); + + void visitGuardToClass(MGuardToClass* ins); + void visitGuardShape(MGuardShape* ins); + void visitGuardArrayIsPacked(MGuardArrayIsPacked* ins); + void visitUnbox(MUnbox* ins); + void visitCompare(MCompare* ins); void visitLoadElement(MLoadElement* ins); void visitArrayLength(MArrayLength* ins); void visitInitializedLength(MInitializedLength* ins); + void visitGuardElementsArePacked(MGuardElementsArePacked* ins); void visitApplyArray(MApplyArray* ins); void visitConstructArray(MConstructArray* ins); @@ -2623,8 +2601,8 @@ class RestReplacer : public GenericArrayReplacer { public: RestReplacer(const MIRGenerator* mir, MIRGraph& graph, MInstruction* rest) - : GenericArrayReplacer(graph.alloc(), rest), mir_(mir), graph_(graph) { - MOZ_ASSERT(IsOptimizableRestInstruction(arr_)); + : mir_(mir), graph_(graph), rest_(rest) { + MOZ_ASSERT(IsOptimizableRestInstruction(rest_)); } bool escapes(MInstruction* ins); @@ -2632,11 +2610,6 @@ class RestReplacer : public GenericArrayReplacer { void assertSuccess(); }; -void RestReplacer::assertSuccess() { - MOZ_ASSERT(arr_->canRecoverOnBailout()); - MOZ_ASSERT(!arr_->hasLiveDefUses()); -} - // Returns false if the rest array object does not escape. bool RestReplacer::escapes(MInstruction* ins) { MOZ_ASSERT(ins->type() == MIRType::Object); @@ -2806,7 +2779,7 @@ bool RestReplacer::escapes(MElements* ins) { // Replacing the rest array object is simpler than replacing an object or array, // because the rest array object does not change state. bool RestReplacer::run() { - MBasicBlock* startBlock = arr_->block(); + MBasicBlock* startBlock = rest_->block(); // Iterate over each basic block. for (ReversePostorderIterator block = graph_.rpoBegin(startBlock); @@ -2830,7 +2803,7 @@ bool RestReplacer::run() { MIR_OPCODE_LIST(MIR_OP) #undef MIR_OP } - if (!alloc_.ensureBallast()) { + if (!graph_.alloc().ensureBallast()) { return false; } } @@ -2840,52 +2813,143 @@ bool RestReplacer::run() { return true; } -void RestReplacer::visitLoadElement(MLoadElement* ins) { - // Skip other array objects. - MDefinition* elements = ins->elements(); - if (!isTargetElements(elements)) { - return; +void RestReplacer::assertSuccess() { + MOZ_ASSERT(rest_->canRecoverOnBailout()); + MOZ_ASSERT(!rest_->hasLiveDefUses()); +} + +bool RestReplacer::isRestElements(MDefinition* elements) { + return elements->isElements() && elements->toElements()->object() == rest_; +} + +void RestReplacer::discardInstruction(MInstruction* ins, + MDefinition* elements) { + MOZ_ASSERT(elements->isElements()); + ins->block()->discard(ins); + if (!elements->hasLiveDefUses()) { + elements->block()->discard(elements->toInstruction()); } +} - MDefinition* index = ins->index(); +void RestReplacer::visitGuardToClass(MGuardToClass* ins) { + // Skip guards on other objects. + if (ins->object() != rest_) { + return; + } + MOZ_ASSERT(ins->getClass() == &ArrayObject::class_); - // Adjust the index to skip any extra formals. - if (uint32_t formals = rest()->numFormals()) { - auto* numFormals = MConstant::NewInt32(alloc_, formals); - ins->block()->insertBefore(ins, numFormals); + // Replace the guard with the array object. + ins->replaceAllUsesWith(rest_); - auto* add = MAdd::New(alloc_, index, numFormals, TruncateKind::Truncate); - ins->block()->insertBefore(ins, add); + // Remove the guard. + ins->block()->discard(ins); +} - index = add; +void RestReplacer::visitGuardShape(MGuardShape* ins) { + // Skip guards on other objects. + if (ins->object() != rest_) { + return; } - auto* loadArg = MGetFrameArgument::New(alloc_, index); + // Replace the guard with the array object. + ins->replaceAllUsesWith(rest_); - ins->block()->insertBefore(ins, loadArg); - ins->replaceAllUsesWith(loadArg); + // Remove the guard. + ins->block()->discard(ins); +} + +void RestReplacer::visitGuardArrayIsPacked(MGuardArrayIsPacked* ins) { + // Skip guards on other objects. + if (ins->array() != rest_) { + return; + } + + // Replace the guard by its object. + ins->replaceAllUsesWith(rest_); // Remove original instruction. - discardInstruction(ins, elements); + ins->block()->discard(ins); } -MDefinition* RestReplacer::restLength(MInstruction* ins) { - // Compute |Math.max(numActuals - numFormals, 0)| for the rest array length. +void RestReplacer::visitUnbox(MUnbox* ins) { + // Skip unrelated unboxes. + if (ins->input() != rest_) { + return; + } + MOZ_ASSERT(ins->type() == MIRType::Object); - auto* numActuals = rest()->numActuals(); + // Replace the unbox with the array object. + ins->replaceAllUsesWith(rest_); + + // Remove the unbox. + ins->block()->discard(ins); +} + +void RestReplacer::visitCompare(MCompare* ins) { + // Skip unrelated comparisons. + if (ins->lhs() != rest_ && ins->rhs() != rest_) { + return; + } + + bool folded; + MOZ_ALWAYS_TRUE(ins->tryFold(&folded)); + + auto* cst = MConstant::NewBoolean(alloc(), folded); + ins->block()->insertBefore(ins, cst); + + // Replace the comparison with a constant. + ins->replaceAllUsesWith(cst); + + // Remove original instruction. + ins->block()->discard(ins); +} + +void RestReplacer::visitLoadElement(MLoadElement* ins) { + // Skip other array objects. + MDefinition* elements = ins->elements(); + if (!isRestElements(elements)) { + return; + } + + MDefinition* index = ins->index(); + + // Adjust the index to skip any extra formals. + if (uint32_t formals = rest()->numFormals()) { + auto* numFormals = MConstant::NewInt32(alloc(), formals); + ins->block()->insertBefore(ins, numFormals); + + auto* add = MAdd::New(alloc(), index, numFormals, TruncateKind::Truncate); + ins->block()->insertBefore(ins, add); + + index = add; + } + + auto* loadArg = MGetFrameArgument::New(alloc(), index); + + ins->block()->insertBefore(ins, loadArg); + ins->replaceAllUsesWith(loadArg); + + // Remove original instruction. + discardInstruction(ins, elements); +} + +MDefinition* RestReplacer::restLength(MInstruction* ins) { + // Compute |Math.max(numActuals - numFormals, 0)| for the rest array length. + + auto* numActuals = rest()->numActuals(); if (uint32_t formals = rest()->numFormals()) { - auto* numFormals = MConstant::NewInt32(alloc_, formals); + auto* numFormals = MConstant::NewInt32(alloc(), formals); ins->block()->insertBefore(ins, numFormals); - auto* length = MSub::New(alloc_, numActuals, numFormals, MIRType::Int32); + auto* length = MSub::New(alloc(), numActuals, numFormals, MIRType::Int32); length->setTruncateKind(TruncateKind::Truncate); ins->block()->insertBefore(ins, length); - auto* zero = MConstant::NewInt32(alloc_, 0); + auto* zero = MConstant::NewInt32(alloc(), 0); ins->block()->insertBefore(ins, zero); - auto* minmax = MMinMax::NewMax(alloc_, length, zero, MIRType::Int32); + auto* minmax = MMinMax::NewMax(alloc(), length, zero, MIRType::Int32); ins->block()->insertBefore(ins, minmax); return minmax; @@ -2898,7 +2962,7 @@ void RestReplacer::visitLength(MInstruction* ins, MDefinition* elements) { MOZ_ASSERT(ins->isArrayLength() || ins->isInitializedLength()); // Skip other array objects. - if (!isTargetElements(elements)) { + if (!isRestElements(elements)) { return; } @@ -2919,17 +2983,28 @@ void RestReplacer::visitInitializedLength(MInitializedLength* ins) { visitLength(ins, ins->elements()); } +void RestReplacer::visitGuardElementsArePacked(MGuardElementsArePacked* ins) { + // Skip other array objects. + MDefinition* elements = ins->elements(); + if (!isRestElements(elements)) { + return; + } + + // Remove original instruction. + discardInstruction(ins, elements); +} + void RestReplacer::visitApplyArray(MApplyArray* ins) { // Skip other array objects. MDefinition* elements = ins->getElements(); - if (!isTargetElements(elements)) { + if (!isRestElements(elements)) { return; } auto* numActuals = restLength(ins); auto* apply = - MApplyArgs::New(alloc_, ins->getSingleTarget(), ins->getFunction(), + MApplyArgs::New(alloc(), ins->getSingleTarget(), ins->getFunction(), numActuals, ins->getThis(), rest()->numFormals()); apply->setBailoutKind(ins->bailoutKind()); if (!ins->maybeCrossRealm()) { @@ -2951,14 +3026,14 @@ void RestReplacer::visitApplyArray(MApplyArray* ins) { void RestReplacer::visitConstructArray(MConstructArray* ins) { // Skip other array objects. MDefinition* elements = ins->getElements(); - if (!isTargetElements(elements)) { + if (!isRestElements(elements)) { return; } auto* numActuals = restLength(ins); auto* construct = MConstructArgs::New( - alloc_, ins->getSingleTarget(), ins->getFunction(), numActuals, + alloc(), ins->getSingleTarget(), ins->getFunction(), numActuals, ins->getThis(), ins->getNewTarget(), rest()->numFormals()); construct->setBailoutKind(ins->bailoutKind()); if (!ins->maybeCrossRealm()) { @@ -3823,282 +3898,6 @@ WasmStructMemoryView::WasmStructMemoryView(TempAllocator& alloc, state_(nullptr), oom_(false) {} -static inline bool IsOptimizableObjectKeysInstruction(MInstruction* ins) { - return ins->isObjectKeys(); -} - -class ObjectKeysReplacer : public GenericArrayReplacer { - private: - const MIRGenerator* mir_; - MIRGraph& graph_; - MObjectToIterator* objToIter_ = nullptr; - - MObjectKeys* objectKeys() const { return arr_->toObjectKeys(); } - - MDefinition* objectKeysLength(MInstruction* ins); - void visitLength(MInstruction* ins, MDefinition* elements); - - void visitLoadElement(MLoadElement* ins); - void visitArrayLength(MArrayLength* ins); - void visitInitializedLength(MInitializedLength* ins); - - bool escapes(MElements* ins); - - public: - ObjectKeysReplacer(const MIRGenerator* mir, MIRGraph& graph, - MInstruction* arr) - : GenericArrayReplacer(graph.alloc(), arr), mir_(mir), graph_(graph) { - MOZ_ASSERT(IsOptimizableObjectKeysInstruction(arr_)); - } - - bool escapes(MInstruction* ins); - bool run(MInstructionIterator& outerIterator); - void assertSuccess(); -}; - -// Returns false if the Object.keys array object does not escape. -bool ObjectKeysReplacer::escapes(MInstruction* ins) { - MOZ_ASSERT(ins->type() == MIRType::Object); - - JitSpewDef(JitSpew_Escape, "Check Object.keys array\n", ins); - JitSpewIndent spewIndent(JitSpew_Escape); - - // Check all uses to see whether they can be supported without allocating an - // ArrayObject for the Object.keys parameter. - for (MUseIterator i(ins->usesBegin()); i != ins->usesEnd(); i++) { - MNode* consumer = (*i)->consumer(); - - // If a resume point can observe this instruction, we can only optimize - // if it is recoverable. - if (consumer->isResumePoint()) { - if (!consumer->toResumePoint()->isRecoverableOperand(*i)) { - JitSpew(JitSpew_Escape, - "Observable Object.keys array cannot be recovered"); - return true; - } - continue; - } - - MDefinition* def = consumer->toDefinition(); - switch (def->op()) { - case MDefinition::Opcode::Elements: { - auto* elem = def->toElements(); - MOZ_ASSERT(elem->object() == ins); - if (escapes(elem)) { - JitSpewDef(JitSpew_Escape, "is indirectly escaped by\n", def); - return true; - } - break; - } - - case MDefinition::Opcode::GuardShape: { - const Shape* shape = objectKeys()->resultShape(); - MOZ_DIAGNOSTIC_ASSERT(shape); - auto* guard = def->toGuardShape(); - MOZ_DIAGNOSTIC_ASSERT(shape == guard->shape()); - if (shape != guard->shape()) { - JitSpewDef(JitSpew_Escape, "has a non-matching guard shape\n", def); - return true; - } - if (escapes(guard)) { - JitSpewDef(JitSpew_Escape, "is indirectly escaped by\n", def); - return true; - } - break; - } - - case MDefinition::Opcode::GuardToClass: { - auto* guard = def->toGuardToClass(); - if (guard->getClass() != &ArrayObject::class_) { - JitSpewDef(JitSpew_Escape, "has a non-matching class guard\n", def); - return true; - } - if (escapes(guard)) { - JitSpewDef(JitSpew_Escape, "is indirectly escaped by\n", def); - return true; - } - break; - } - - case MDefinition::Opcode::GuardArrayIsPacked: { - // Object.keys arrays are always packed as long as they aren't modified. - auto* guard = def->toGuardArrayIsPacked(); - if (escapes(guard)) { - JitSpewDef(JitSpew_Escape, "is indirectly escaped by\n", def); - return true; - } - break; - } - - case MDefinition::Opcode::Unbox: { - if (def->type() != MIRType::Object) { - JitSpewDef(JitSpew_Escape, "has an invalid unbox\n", def); - return true; - } - if (escapes(def->toInstruction())) { - JitSpewDef(JitSpew_Escape, "is indirectly escaped by\n", def); - return true; - } - break; - } - - // This instruction is supported for |JSOp::OptimizeSpreadCall|. - case MDefinition::Opcode::Compare: { - bool canFold; - if (!def->toCompare()->tryFold(&canFold)) { - JitSpewDef(JitSpew_Escape, "has an unsupported compare\n", def); - return true; - } - break; - } - - // This instruction is a no-op used to test that scalar replacement is - // working as expected. - case MDefinition::Opcode::AssertRecoveredOnBailout: - break; - - default: - JitSpewDef(JitSpew_Escape, "is escaped by\n", def); - return true; - } - } - - JitSpew(JitSpew_Escape, "Object.keys array object is not escaped"); - return false; -} - -bool ObjectKeysReplacer::escapes(MElements* ins) { - JitSpewDef(JitSpew_Escape, "Check Object.keys array elements\n", ins); - JitSpewIndent spewIndent(JitSpew_Escape); - - for (MUseIterator i(ins->usesBegin()); i != ins->usesEnd(); i++) { - // The MIRType::Elements cannot be captured in a resume point as it does not - // represent a value allocation. - MDefinition* def = (*i)->consumer()->toDefinition(); - - switch (def->op()) { - case MDefinition::Opcode::LoadElement: { - MOZ_ASSERT(def->toLoadElement()->elements() == ins); - break; - } - - case MDefinition::Opcode::ArrayLength: - MOZ_ASSERT(def->toArrayLength()->elements() == ins); - break; - - case MDefinition::Opcode::InitializedLength: - MOZ_ASSERT(def->toInitializedLength()->elements() == ins); - break; - - case MDefinition::Opcode::GuardElementsArePacked: - MOZ_ASSERT(def->toGuardElementsArePacked()->elements() == ins); - break; - - default: - JitSpewDef(JitSpew_Escape, "is escaped by\n", def); - return true; - } - } - - JitSpew(JitSpew_Escape, "Object.keys array object is not escaped"); - return false; -} - -bool ObjectKeysReplacer::run(MInstructionIterator& outerIterator) { - MBasicBlock* startBlock = arr_->block(); - - objToIter_ = MObjectToIterator::New(alloc_, objectKeys()->object(), nullptr); - objToIter_->setSkipRegistration(true); - arr_->block()->insertBefore(arr_, objToIter_); - - // Iterate over each basic block. - for (ReversePostorderIterator block = graph_.rpoBegin(startBlock); - block != graph_.rpoEnd(); block++) { - if (mir_->shouldCancel("Scalar replacement of Object.keys array object")) { - return false; - } - - // Iterates over phis and instructions. - // We do not have to visit resume points. Any resume points that capture the - // Object.keys array object will be handled by the Sink pass. - for (MDefinitionIterator iter(*block); iter;) { - // Increment the iterator before visiting the instruction, as the visit - // function might discard itself from the basic block. - MDefinition* def = *iter++; - switch (def->op()) { -#define MIR_OP(op) \ - case MDefinition::Opcode::op: \ - visit##op(def->to##op()); \ - break; - MIR_OPCODE_LIST(MIR_OP) -#undef MIR_OP - } - if (!graph_.alloc().ensureBallast()) { - return false; - } - } - } - - assertSuccess(); - - auto* forRecovery = MObjectKeysFromIterator::New(alloc_, objToIter_); - arr_->block()->insertBefore(arr_, forRecovery); - forRecovery->stealResumePoint(arr_); - arr_->replaceAllUsesWith(forRecovery); - - // We need to explicitly discard the instruction since it's marked as - // effectful and we stole its resume point, which will trip assertion - // failures later. We can't discard the instruction out from underneath - // the iterator though, and we can't do the trick where we increment the - // iterator at the top of the loop because we might discard the *next* - // instruction, so we do this goofiness. - outerIterator--; - arr_->block()->discard(arr_); - - if (!graph_.alloc().ensureBallast()) { - return false; - } - - return true; -} - -void ObjectKeysReplacer::assertSuccess() { - MOZ_ASSERT(!arr_->hasLiveDefUses()); -} - -void ObjectKeysReplacer::visitLoadElement(MLoadElement* ins) { - if (!isTargetElements(ins->elements())) { - return; - } - - auto* load = MLoadIteratorElement::New(alloc_, objToIter_, ins->index()); - ins->block()->insertBefore(ins, load); - - ins->replaceAllUsesWith(load); - discardInstruction(ins, ins->elements()); -} - -void ObjectKeysReplacer::visitLength(MInstruction* ins, MDefinition* elements) { - if (!isTargetElements(elements)) { - return; - } - - auto* newLen = MIteratorLength::New(alloc_, objToIter_); - ins->block()->insertBefore(ins, newLen); - - ins->replaceAllUsesWith(newLen); - discardInstruction(ins, elements); -} - -void ObjectKeysReplacer::visitArrayLength(MArrayLength* ins) { - visitLength(ins, ins->elements()); -} - -void ObjectKeysReplacer::visitInitializedLength(MInitializedLength* ins) { - // The initialized length of an Object.keys array is equal to its length. - visitLength(ins, ins->elements()); -} - bool ScalarReplacement(const MIRGenerator* mir, MIRGraph& graph) { JitSpew(JitSpew_Escape, "Begin (ScalarReplacement)"); @@ -4196,32 +3995,5 @@ bool ScalarReplacement(const MIRGenerator* mir, MIRGraph& graph) { return true; } -bool ReplaceObjectKeys(const MIRGenerator* mir, MIRGraph& graph) { - JitSpew(JitSpew_Escape, "Begin (Object.Keys Replacement)"); - - for (ReversePostorderIterator block = graph.rpoBegin(); - block != graph.rpoEnd(); block++) { - if (mir->shouldCancel("Object.Keys Replacement (main loop)")) { - return false; - } - - for (MInstructionIterator ins = block->begin(); ins != block->end(); - ins++) { - if (IsOptimizableObjectKeysInstruction(*ins)) { - ObjectKeysReplacer replacer(mir, graph, *ins); - if (replacer.escapes(*ins)) { - continue; - } - if (!replacer.run(ins)) { - return false; - } - continue; - } - } - } - - return true; -} - } /* namespace jit */ } /* namespace js */ diff --git a/js/src/jit/ScalarReplacement.h b/js/src/jit/ScalarReplacement.h @@ -15,7 +15,6 @@ class MIRGenerator; class MIRGraph; [[nodiscard]] bool ScalarReplacement(const MIRGenerator* mir, MIRGraph& graph); -[[nodiscard]] bool ReplaceObjectKeys(const MIRGenerator* mir, MIRGraph& graph); } // namespace jit } // namespace js diff --git a/js/src/jit/TypePolicy.cpp b/js/src/jit/TypePolicy.cpp @@ -1072,8 +1072,6 @@ bool ClampPolicy::adjustInputs(TempAllocator& alloc, MInstruction* ins) const { _(MixPolicy<ObjectPolicy<0>, ObjectPolicy<1>, IntPtrPolicy<2>>) \ _(MixPolicy<ObjectPolicy<0>, ObjectPolicy<1>, ObjectPolicy<2>>) \ _(MixPolicy<ObjectPolicy<0>, ObjectPolicy<1>, BoxPolicy<2>>) \ - _(MixPolicy<ObjectPolicy<0>, ObjectPolicy<1>, UnboxedInt32Policy<2>, \ - BoxPolicy<3>>) \ _(MixPolicy<StringPolicy<0>, UnboxedInt32Policy<1>, UnboxedInt32Policy<2>>) \ _(MixPolicy<StringPolicy<0>, ObjectPolicy<1>, StringPolicy<2>>) \ _(MixPolicy<StringPolicy<0>, StringPolicy<1>, StringPolicy<2>>) \ diff --git a/js/src/jit/VMFunctionList-inl.h b/js/src/jit/VMFunctionList-inl.h @@ -205,10 +205,7 @@ namespace jit { _(GetImportOperation, js::GetImportOperation) \ _(GetIntrinsicValue, js::jit::GetIntrinsicValue) \ _(GetIterator, js::GetIterator) \ - _(GetIteratorUnregistered, js::GetIteratorUnregistered) \ _(GetIteratorWithIndices, js::GetIteratorWithIndices) \ - _(GetIteratorWithIndicesUnregistered, \ - js::GetIteratorWithIndicesUnregistered) \ _(GetNonSyntacticGlobalThis, js::GetNonSyntacticGlobalThis) \ _(GetOrCreateModuleMetaObject, js::GetOrCreateModuleMetaObject) \ _(GetPendingExceptionStack, js::GetPendingExceptionStack) \ diff --git a/js/src/jit/VMFunctions.cpp b/js/src/jit/VMFunctions.cpp @@ -1492,27 +1492,6 @@ JSObject* ObjectKeys(JSContext* cx, HandleObject obj) { return argv[0].toObjectOrNull(); } -JSObject* ObjectKeysFromIterator(JSContext* cx, HandleObject iterObj) { - MOZ_RELEASE_ASSERT(iterObj->is<PropertyIteratorObject>()); - NativeIterator* iter = - iterObj->as<PropertyIteratorObject>().getNativeIterator(); - - size_t length = iter->ownPropertyCount(); - Rooted<ArrayObject*> array(cx, NewDenseFullyAllocatedArray(cx, length)); - if (!array) { - return nullptr; - } - - array->ensureDenseInitializedLength(0, length); - - for (size_t i = 0; i < length; ++i) { - array->initDenseElement( - i, StringValue((iter->propertiesBegin() + i)->asString())); - } - - return array; -} - bool ObjectKeysLength(JSContext* cx, HandleObject obj, int32_t* length) { MOZ_ASSERT(!obj->is<ProxyObject>()); return js::obj_keys_length(cx, obj, *length); diff --git a/js/src/jit/VMFunctions.h b/js/src/jit/VMFunctions.h @@ -521,7 +521,6 @@ void JitWasmAnyRefPreWriteBarrier(JSRuntime* rt, wasm::AnyRef* refp); bool ObjectIsCallable(JSObject* obj); bool ObjectIsConstructor(JSObject* obj); JSObject* ObjectKeys(JSContext* cx, HandleObject obj); -JSObject* ObjectKeysFromIterator(JSContext* cx, HandleObject iterObj); bool ObjectKeysLength(JSContext* cx, HandleObject obj, int32_t* length); [[nodiscard]] bool ThrowRuntimeLexicalError(JSContext* cx, diff --git a/js/src/jit/WarpCacheIRTranspiler.cpp b/js/src/jit/WarpCacheIRTranspiler.cpp @@ -4328,12 +4328,10 @@ bool WarpCacheIRTranspiler::emitArrayJoinResult(ObjOperandId objId, return resumeAfter(join); } -bool WarpCacheIRTranspiler::emitObjectKeysResult(ObjOperandId objId, - uint32_t resultShapeOffset) { +bool WarpCacheIRTranspiler::emitObjectKeysResult(ObjOperandId objId) { MDefinition* obj = getOperand(objId); - Shape* resultShape = shapeStubField(resultShapeOffset); - auto* join = MObjectKeys::New(alloc(), obj, resultShape); + auto* join = MObjectKeys::New(alloc(), obj); addEffectful(join); pushResult(join); diff --git a/js/src/jit/arm/MacroAssembler-arm-inl.h b/js/src/jit/arm/MacroAssembler-arm-inl.h @@ -327,14 +327,6 @@ void MacroAssembler::add32(Imm32 imm, const Address& dest) { ma_str(scratch, dest, scratch2); } -void MacroAssembler::add32(const Address& src, Register dest) { - ScratchRegisterScope scratch(*this); - SecondScratchRegisterScope scratch2(*this); - - ma_ldr(src, scratch, scratch2); - ma_add(scratch, dest, SetCC); -} - void MacroAssembler::addPtr(Register src, Register dest) { ma_add(src, dest); } void MacroAssembler::addPtr(Imm32 imm, Register dest) { diff --git a/js/src/jit/arm64/MacroAssembler-arm64-inl.h b/js/src/jit/arm64/MacroAssembler-arm64-inl.h @@ -314,14 +314,6 @@ void MacroAssembler::add32(Imm32 imm, const Address& dest) { Str(scratch32, toMemOperand(dest)); } -void MacroAssembler::add32(const Address& src, Register dest) { - vixl::UseScratchRegisterScope temps(this); - const ARMRegister scratch32 = temps.AcquireW(); - MOZ_ASSERT(scratch32.asUnsized() != src.base); - load32(src, scratch32.asUnsized()); - Add(ARMRegister(dest, 32), ARMRegister(dest, 32), Operand(scratch32)); -} - void MacroAssembler::addPtr(Register src, Register dest) { addPtr(src, dest, dest); } diff --git a/js/src/jit/loong64/MacroAssembler-loong64-inl.h b/js/src/jit/loong64/MacroAssembler-loong64-inl.h @@ -331,13 +331,6 @@ void MacroAssembler::add32(Imm32 imm, const Address& dest) { store32(scratch, dest); } -void MacroAssembler::add32(const Address& src, Register dest) { - UseScratchRegisterScope temps(asMasm()); - Register scratch = temps.Acquire(); - load32(src, scratch); - as_add_w(dest, dest, scratch); -} - void MacroAssembler::addPtr(Imm32 imm, const Address& dest) { UseScratchRegisterScope temps(asMasm()); Register scratch = temps.Acquire(); diff --git a/js/src/jit/mips-shared/MacroAssembler-mips-shared-inl.h b/js/src/jit/mips-shared/MacroAssembler-mips-shared-inl.h @@ -158,13 +158,6 @@ void MacroAssembler::add32(Imm32 imm, const Address& dest) { store32(scratch2, dest); } -void MacroAssembler::add32(const Address& src, Register dest) { - UseScratchRegisterScope temps(*this); - Register scratch2 = temps.Acquire(); - load32(src, scratch2); - as_addu(dest, dest, scratch2); -} - void MacroAssembler::addPtr(Imm32 imm, const Address& dest) { UseScratchRegisterScope temps(*this); Register scratch = temps.Acquire(); diff --git a/js/src/jit/riscv64/MacroAssembler-riscv64-inl.h b/js/src/jit/riscv64/MacroAssembler-riscv64-inl.h @@ -294,14 +294,6 @@ void MacroAssembler::add32(Imm32 imm, const Address& dest) { ma_add32(scratch2, scratch2, imm); store32(scratch2, dest); } - -void MacroAssembler::add32(const Address& src, Register dest) { - UseScratchRegisterScope temps(this); - Register scratch = temps.Acquire(); - load32(src, scratch); - ma_add32(dest, dest, scratch); -} - void MacroAssembler::add64(Register64 src, Register64 dest) { addPtr(src.reg, dest.reg); } diff --git a/js/src/jit/wasm32/MacroAssembler-wasm32-inl.h b/js/src/jit/wasm32/MacroAssembler-wasm32-inl.h @@ -288,8 +288,6 @@ void MacroAssembler::add32(Imm32 imm, Register src, Register dest) { void MacroAssembler::add32(Imm32 imm, const Address& dest) { MOZ_CRASH(); } -void MacroAssembler::add32(const Address& src, Register dest) { MOZ_CRASH(); } - void MacroAssembler::addFloat32(FloatRegister src, FloatRegister dest) { MOZ_CRASH(); } diff --git a/js/src/jit/x86-shared/MacroAssembler-x86-shared-inl.h b/js/src/jit/x86-shared/MacroAssembler-x86-shared-inl.h @@ -199,10 +199,6 @@ void MacroAssembler::byteSwap32(Register reg) { bswapl(reg); } // =============================================================== // Arithmetic instructions -void MacroAssembler::add32(const Address& src, Register dest) { - addl(Operand(src), dest); -} - void MacroAssembler::add32(Register src, Register dest) { addl(src, dest); } void MacroAssembler::add32(Imm32 imm, Register dest) { addl(imm, dest); } diff --git a/js/src/shell/fuzz-flags.txt b/js/src/shell/fuzz-flags.txt @@ -60,8 +60,6 @@ --monomorphic-inlining=always --monomorphic-inlining=never --disable-main-thread-denormals ---object-keys-scalar-replacement=off ---object-keys-scalar-replacement=on # GC-related # These 2 flags can cause the shell to slow down diff --git a/js/src/shell/js.cpp b/js/src/shell/js.cpp @@ -13025,9 +13025,6 @@ bool InitOptionParser(OptionParser& op) { !op.addBoolOption('\0', "disable-main-thread-denormals", "Disable Denormals on the main thread only, to " "emulate WebAudio worklets.") || - !op.addStringOption('\0', "object-keys-scalar-replacement", "on/off", - "Replace Object.keys with a NativeIterators " - "(default: on)") || !op.addBoolOption('\0', "baseline", "Enable baseline compiler (default)") || !op.addBoolOption('\0', "no-baseline", "Disable baseline compiler") || @@ -13992,16 +13989,6 @@ bool SetContextJITOptions(JSContext* cx, const OptionParser& op) { #endif } - if (const char* str = op.getStringOption("object-keys-scalar-replacement")) { - if (strcmp(str, "on") == 0) { - jit::JitOptions.disableObjectKeysScalarReplacement = false; - } else if (strcmp(str, "off") == 0) { - jit::JitOptions.disableObjectKeysScalarReplacement = true; - } else { - return OptionFailure("object-keys-scalar-replacement", str); - } - } - int32_t warmUpThreshold = op.getIntOption("ion-warmup-threshold"); if (warmUpThreshold >= 0) { jit::JitOptions.setNormalIonWarmUpThreshold(warmUpThreshold); diff --git a/js/src/vm/Iteration.cpp b/js/src/vm/Iteration.cpp @@ -61,18 +61,28 @@ static const gc::AllocKind ITERATOR_FINALIZE_KIND = // into this code. void NativeIterator::trace(JSTracer* trc) { TraceNullableEdge(trc, &objectBeingIterated_, "objectBeingIterated_"); - TraceNullableEdge(trc, &iterObj_, "iterObj_"); - TraceNullableEdge(trc, &objShape_, "objShape_"); + TraceNullableEdge(trc, &iterObj_, "iterObj"); // The limits below are correct at every instant of |NativeIterator| // initialization, with the end-pointer incremented as each new shape is // created, so they're safe to use here. - std::for_each(protoShapesBegin(allocatedPropertyCount()), protoShapesEnd(), - [trc](GCPtr<Shape*>& shape) { - TraceEdge(trc, &shape, "iterator_proto_shape"); - }); - - std::for_each(propertiesBegin(), propertiesEnd(), + std::for_each(shapesBegin(), shapesEnd(), [trc](GCPtr<Shape*>& shape) { + TraceEdge(trc, &shape, "iterator_shape"); + }); + + // But as properties must be created *before* shapes, |propertiesBegin()| + // that depends on |shapesEnd()| having its final value can't safely be + // used. Until this is fully initialized, use |propertyCursor_| instead, + // which points at the start of properties even in partially initialized + // |NativeIterator|s. (|propertiesEnd()| is safe at all times with respect + // to the properly-chosen beginning.) + // + // Note that we must trace all properties (not just those not yet visited, + // or just visited, due to |NativeIterator::previousPropertyWas|) for + // |NativeIterator|s to be reusable. + IteratorProperty* begin = + MOZ_LIKELY(isInitialized()) ? propertiesBegin() : propertyCursor_; + std::for_each(begin, propertiesEnd(), [trc](IteratorProperty& prop) { prop.traceString(trc); }); } @@ -94,15 +104,13 @@ class PropertyEnumerator { uint32_t flags_; Rooted<PropertyKeySet> visited_; - uint32_t ownPropertyCount_; - bool enumeratingProtoChain_ = false; enum class IndicesState { // Every property that has been enumerated so far can be represented as a // PropertyIndex, but we are not currently producing a list of indices. If // the state is Valid when we are done enumerating, then the resulting - // iterator can be marked with NativeIterator::Flags::IndicesSupported. + // iterator can be marked as NativeIteratorIndices::AvailableOnRequest. Valid, // Every property that has been enumerated so far can be represented as a @@ -140,7 +148,6 @@ class PropertyEnumerator { bool allocatingIndices() const { return indicesState_ == IndicesState::Allocating; } - uint32_t ownPropertyCount() const { return ownPropertyCount_; } private: template <bool CheckForDuplicates> @@ -670,10 +677,6 @@ bool PropertyEnumerator::snapshot(JSContext* cx) { MOZ_CRASH("non-native objects must have an enumerate op"); } - if (!enumeratingProtoChain_) { - ownPropertyCount_ = props_.length(); - } - if (flags_ & JSITER_OWNONLY) { break; } @@ -735,9 +738,8 @@ JS_PUBLIC_API bool js::GetPropertyKeys(JSContext* cx, HandleObject obj, return enumerator.snapshot(cx); } -static inline void RegisterEnumerator(JSContext* cx, NativeIterator* ni, - HandleObject obj) { - ni->initObjectBeingIterated(*obj); +static inline void RegisterEnumerator(JSContext* cx, NativeIterator* ni) { + MOZ_ASSERT(ni->objectBeingIterated()); // Register non-escaping native enumerators (for-in) with the current // context. @@ -769,29 +771,29 @@ static PropertyIteratorObject* NewPropertyIteratorObject(JSContext* cx) { return res; } -static inline size_t NumTrailingBytes(size_t propertyCount, - size_t protoShapeCount, bool hasIndices) { +static inline size_t NumTrailingBytes(size_t propertyCount, size_t shapeCount, + bool hasIndices) { static_assert(alignof(IteratorProperty) <= alignof(NativeIterator)); static_assert(alignof(GCPtr<Shape*>) <= alignof(IteratorProperty)); static_assert(alignof(PropertyIndex) <= alignof(GCPtr<Shape*>)); size_t result = propertyCount * sizeof(IteratorProperty) + - protoShapeCount * sizeof(GCPtr<Shape*>); + shapeCount * sizeof(GCPtr<Shape*>); if (hasIndices) { result += propertyCount * sizeof(PropertyIndex); } return result; } -static inline size_t AllocationSize(size_t propertyCount, - size_t protoShapeCount, bool hasIndices) { +static inline size_t AllocationSize(size_t propertyCount, size_t shapeCount, + bool hasIndices) { return sizeof(NativeIterator) + - NumTrailingBytes(propertyCount, protoShapeCount, hasIndices); + NumTrailingBytes(propertyCount, shapeCount, hasIndices); } static PropertyIteratorObject* CreatePropertyIterator( JSContext* cx, Handle<JSObject*> objBeingIterated, HandleIdVector props, bool supportsIndices, PropertyIndexVector* indices, - uint32_t cacheableProtoChainLength, uint32_t ownPropertyCount) { + uint32_t cacheableProtoChainLength) { MOZ_ASSERT_IF(indices, supportsIndices); if (props.length() >= NativeIterator::PropCountLimit) { ReportAllocationOverflow(cx); @@ -808,11 +810,6 @@ static PropertyIteratorObject* CreatePropertyIterator( if (numShapes == 0 && hasIndices) { numShapes = 1; } - if (numShapes > NativeIterator::ShapeCountLimit) { - ReportAllocationOverflow(cx); - return nullptr; - } - uint32_t numProtoShapes = numShapes > 0 ? numShapes - 1 : 0; Rooted<PropertyIteratorObject*> propIter(cx, NewPropertyIteratorObject(cx)); if (!propIter) { @@ -820,16 +817,15 @@ static PropertyIteratorObject* CreatePropertyIterator( } void* mem = cx->pod_malloc_with_extra<NativeIterator, uint8_t>( - NumTrailingBytes(props.length(), numProtoShapes, hasIndices)); + NumTrailingBytes(props.length(), numShapes, hasIndices)); if (!mem) { return nullptr; } // This also registers |ni| with |propIter|. bool hadError = false; - new (mem) - NativeIterator(cx, propIter, objBeingIterated, props, supportsIndices, - indices, numShapes, ownPropertyCount, &hadError); + new (mem) NativeIterator(cx, propIter, objBeingIterated, props, + supportsIndices, indices, numShapes, &hadError); if (hadError) { return nullptr; } @@ -853,15 +849,20 @@ NativeIterator::NativeIterator(JSContext* cx, Handle<JSObject*> objBeingIterated, HandleIdVector props, bool supportsIndices, PropertyIndexVector* indices, uint32_t numShapes, - uint32_t ownPropertyCount, bool* hadError) - : objectBeingIterated_(nullptr), + bool* hadError) + : objectBeingIterated_(objBeingIterated), iterObj_(propIter), - objShape_(numShapes > 0 ? objBeingIterated->shape() : nullptr), - // This holds the allocated property count until we're done with - // initialization - propertyCursor_(props.length()), - ownPropertyCount_(ownPropertyCount), - shapesHash_(0) { + // NativeIterator initially acts (before full initialization) as if it + // contains no shapes... + shapesEnd_(shapesBegin()), + // ...and no properties. + propertyCursor_( + reinterpret_cast<IteratorProperty*>(shapesBegin() + numShapes)), + propertiesEnd_(propertyCursor_), + shapesHash_(0), + flagsAndCount_( + initialFlagsAndCount(props.length())) // note: no Flags::Initialized +{ // If there are shapes, the object and all objects on its prototype chain must // be native objects. See CanCompareIterableObjectToCache. MOZ_ASSERT_IF(numShapes > 0, @@ -872,12 +873,6 @@ NativeIterator::NativeIterator(JSContext* cx, bool hasActualIndices = !!indices; MOZ_ASSERT_IF(hasActualIndices, indices->length() == props.length()); - if (hasActualIndices) { - flags_ |= Flags::IndicesAllocated; - } else if (supportsIndices) { - flags_ |= Flags::IndicesSupported; - } - // NOTE: This must be done first thing: The caller can't free `this` on error // because it has GCPtr fields whose barriers have already fired; the // store buffer has pointers to them. Only the GC can free `this` (via @@ -890,9 +885,22 @@ NativeIterator::NativeIterator(JSContext* cx, // shapes, and ensuring that indicesState_.allocated() is true if we've // allocated space for indices. It's OK for the constructor to fail after // that. - size_t nbytes = AllocationSize( - props.length(), numShapes > 0 ? numShapes - 1 : 0, hasActualIndices); + size_t nbytes = AllocationSize(props.length(), numShapes, hasActualIndices); AddCellMemory(propIter, nbytes, MemoryUse::NativeIterator); + if (supportsIndices) { + if (hasActualIndices) { + // If the string allocation fails, indicesAllocated() must be true + // so that this->allocationSize() is correct. Set it to Disabled. It will + // be updated below. + setIndicesState(NativeIteratorIndices::Disabled); + } else { + // This object supports indices (ie it only has own enumerable + // properties), but we didn't allocate them because we haven't seen a + // consumer yet. We mark the iterator so that potential consumers know to + // request a fresh iterator with indices. + setIndicesState(NativeIteratorIndices::AvailableOnRequest); + } + } if (numShapes > 0) { // Construct shapes into the shapes array. Also compute the shapesHash, @@ -903,10 +911,8 @@ NativeIterator::NativeIterator(JSContext* cx, for (uint32_t i = 0; i < numShapes; i++) { MOZ_ASSERT(pobj->is<NativeObject>()); Shape* shape = pobj->shape(); - if (i > 0) { - new (protoShapesEnd()) GCPtr<Shape*>(shape); - protoShapeCount_++; - } + new (shapesEnd_) GCPtr<Shape*>(shape); + shapesEnd_++; shapesHash = mozilla::AddToHash(shapesHash, HashIteratorShape(shape)); pobj = pobj->staticPrototype(); } @@ -920,8 +926,8 @@ NativeIterator::NativeIterator(JSContext* cx, // shape of the iterated object itself (see IteratorHasIndicesAndBranch). // In the former case, assert that we're storing the entire proto chain. MOZ_ASSERT_IF(numShapes > 1, pobj == nullptr); - MOZ_ASSERT(uintptr_t(protoShapesEnd()) == uintptr_t(this) + nbytes); } + MOZ_ASSERT(static_cast<void*>(shapesEnd_) == propertyCursor_); // Allocate any strings in the nursery until the first minor GC. After this // point they will end up getting tenured anyway because they are reachable @@ -945,8 +951,8 @@ NativeIterator::NativeIterator(JSContext* cx, // We write to our IteratorProperty children only here and in // PropertyIteratorObject::trace. Here we do not need a pre-barrier // because we are not overwriting a previous value. - new (propertiesEnd()) IteratorProperty(str); - propertyCount_++; + new (propertiesEnd_) IteratorProperty(str); + propertiesEnd_++; if (maybeNeedGC && gc::IsInsideNursery(str)) { maybeNeedGC = false; cx->runtime()->gc.storeBuffer().putWholeCell(propIter); @@ -958,18 +964,19 @@ NativeIterator::NativeIterator(JSContext* cx, for (size_t i = 0; i < numProps; i++) { *cursor++ = (*indices)[i]; } - flags_ |= Flags::IndicesAvailable; + MOZ_ASSERT(uintptr_t(cursor) == uintptr_t(this) + nbytes); + setIndicesState(NativeIteratorIndices::Valid); } - propertyCursor_ = 0; - flags_ |= Flags::Initialized; + markInitialized(); MOZ_ASSERT(!*hadError); } inline size_t NativeIterator::allocationSize() const { - return AllocationSize(allocatedPropertyCount(), protoShapeCount_, - indicesAllocated()); + size_t numShapes = shapesEnd() - shapesBegin(); + + return AllocationSize(initialPropertyCount(), numShapes, indicesAllocated()); } /* static */ @@ -977,13 +984,12 @@ bool IteratorHashPolicy::match(PropertyIteratorObject* obj, const Lookup& lookup) { NativeIterator* ni = obj->getNativeIterator(); if (ni->shapesHash() != lookup.shapesHash || - ni->protoShapeCount() != lookup.numProtoShapes || - ni->objShape() != lookup.objShape) { + ni->shapeCount() != lookup.numShapes) { return false; } - return ArrayEqual(reinterpret_cast<Shape**>(ni->protoShapesBegin()), - lookup.protoShapes, ni->protoShapeCount()); + return ArrayEqual(reinterpret_cast<Shape**>(ni->shapesBegin()), lookup.shapes, + ni->shapeCount()); } static inline bool CanCompareIterableObjectToCache(JSObject* obj) { @@ -1010,23 +1016,21 @@ static bool CanStoreInIteratorCache(JSObject* obj) { } static MOZ_ALWAYS_INLINE PropertyIteratorObject* LookupInShapeIteratorCache( - JSContext* cx, JSObject* obj, uint32_t* cacheableProtoChainLength, - bool exclusive) { + JSContext* cx, JSObject* obj, uint32_t* cacheableProtoChainLength) { if (!obj->shape()->cache().isIterator() || !CanCompareIterableObjectToCache(obj)) { return nullptr; } PropertyIteratorObject* iterobj = obj->shape()->cache().toIterator(); NativeIterator* ni = iterobj->getNativeIterator(); - MOZ_ASSERT(ni->objShape() == obj->shape()); - if (exclusive && !ni->isReusable()) { + MOZ_ASSERT(*ni->shapesBegin() == obj->shape()); + if (!ni->isReusable()) { return nullptr; } // Verify shapes of proto chain. JSObject* pobj = obj; - for (GCPtr<Shape*>* s = ni->protoShapesBegin(); s != ni->protoShapesEnd(); - s++) { + for (GCPtr<Shape*>* s = ni->shapesBegin() + 1; s != ni->shapesEnd(); s++) { Shape* shape = *s; pobj = pobj->staticPrototype(); if (pobj->shape() != shape) { @@ -1037,17 +1041,16 @@ static MOZ_ALWAYS_INLINE PropertyIteratorObject* LookupInShapeIteratorCache( } } MOZ_ASSERT(CanStoreInIteratorCache(obj)); - *cacheableProtoChainLength = ni->objShape() ? ni->protoShapeCount() + 1 : 0; + *cacheableProtoChainLength = ni->shapeCount(); return iterobj; } static MOZ_ALWAYS_INLINE PropertyIteratorObject* LookupInIteratorCache( - JSContext* cx, JSObject* obj, uint32_t* cacheableProtoChainLength, - bool exclusive) { + JSContext* cx, JSObject* obj, uint32_t* cacheableProtoChainLength) { MOZ_ASSERT(*cacheableProtoChainLength == 0); - if (PropertyIteratorObject* shapeCached = LookupInShapeIteratorCache( - cx, obj, cacheableProtoChainLength, exclusive)) { + if (PropertyIteratorObject* shapeCached = + LookupInShapeIteratorCache(cx, obj, cacheableProtoChainLength)) { return shapeCached; } @@ -1074,8 +1077,8 @@ static MOZ_ALWAYS_INLINE PropertyIteratorObject* LookupInIteratorCache( MOZ_ASSERT(!shapes.empty()); *cacheableProtoChainLength = shapes.length(); - IteratorHashPolicy::Lookup lookup(shapes[0], shapes.begin() + 1, - shapes.length() - 1, shapesHash); + IteratorHashPolicy::Lookup lookup(shapes.begin(), shapes.length(), + shapesHash); auto p = ObjectRealm::get(obj).iteratorCache.lookup(lookup); if (!p) { return nullptr; @@ -1085,7 +1088,7 @@ static MOZ_ALWAYS_INLINE PropertyIteratorObject* LookupInIteratorCache( MOZ_ASSERT(iterobj->compartment() == cx->compartment()); NativeIterator* ni = iterobj->getNativeIterator(); - if (exclusive && !ni->isReusable()) { + if (!ni->isReusable()) { return nullptr; } @@ -1097,13 +1100,13 @@ static MOZ_ALWAYS_INLINE PropertyIteratorObject* LookupInIteratorCache( MOZ_ASSERT(CanStoreInIteratorCache(obj)); NativeIterator* ni = iterobj->getNativeIterator(); - MOZ_ASSERT(ni->objShape()); + MOZ_ASSERT(ni->shapeCount() > 0); obj->shape()->maybeCacheIterator(cx, iterobj); IteratorHashPolicy::Lookup lookup( - ni->objShape(), reinterpret_cast<Shape**>(ni->protoShapesBegin()), - ni->protoShapeCount(), ni->shapesHash()); + reinterpret_cast<Shape**>(ni->shapesBegin()), ni->shapeCount(), + ni->shapesHash()); ObjectRealm::IteratorCache& cache = ObjectRealm::get(obj).iteratorCache; bool ok; @@ -1139,7 +1142,7 @@ bool js::EnumerateProperties(JSContext* cx, HandleObject obj, #ifdef DEBUG static bool IndicesAreValid(NativeObject* obj, NativeIterator* ni) { - MOZ_ASSERT(ni->indicesAvailable()); + MOZ_ASSERT(ni->hasValidIndices()); size_t numDenseElements = obj->getDenseInitializedLength(); size_t numFixedSlots = obj->numFixedSlots(); const Value* elements = obj->getDenseElements(); @@ -1188,7 +1191,7 @@ static bool IndicesAreValid(NativeObject* obj, NativeIterator* ni) { } #endif -template <bool WantIndices, bool SkipRegistration> +template <bool WantIndices> static PropertyIteratorObject* GetIteratorImpl(JSContext* cx, HandleObject obj) { MOZ_ASSERT(!obj->is<PropertyIteratorObject>()); @@ -1196,16 +1199,15 @@ static PropertyIteratorObject* GetIteratorImpl(JSContext* cx, "We may end up allocating shapes in the wrong zone!"); uint32_t cacheableProtoChainLength = 0; - if (PropertyIteratorObject* iterobj = LookupInIteratorCache( - cx, obj, &cacheableProtoChainLength, !SkipRegistration)) { + if (PropertyIteratorObject* iterobj = + LookupInIteratorCache(cx, obj, &cacheableProtoChainLength)) { NativeIterator* ni = iterobj->getNativeIterator(); - bool recreateWithIndices = WantIndices && ni->indicesSupported(); + bool recreateWithIndices = WantIndices && ni->indicesAvailableOnRequest(); if (!recreateWithIndices) { - MOZ_ASSERT_IF(WantIndices && ni->indicesAvailable(), + MOZ_ASSERT_IF(WantIndices && ni->hasValidIndices(), IndicesAreValid(&obj->as<NativeObject>(), ni)); - if (!SkipRegistration) { - RegisterEnumerator(cx, ni, obj); - } + ni->initObjectBeingIterated(*obj); + RegisterEnumerator(cx, ni); return iterobj; } } @@ -1213,14 +1215,10 @@ static PropertyIteratorObject* GetIteratorImpl(JSContext* cx, if (cacheableProtoChainLength > 0 && !CanStoreInIteratorCache(obj)) { cacheableProtoChainLength = 0; } - if (cacheableProtoChainLength > NativeIterator::ShapeCountLimit) { - cacheableProtoChainLength = 0; - } RootedIdVector keys(cx); PropertyIndexVector indices(cx); bool supportsIndices = false; - uint32_t ownPropertyCount = 0; if (MOZ_UNLIKELY(obj->is<ProxyObject>())) { if (!Proxy::enumerate(cx, obj, &keys)) { @@ -1233,7 +1231,6 @@ static PropertyIteratorObject* GetIteratorImpl(JSContext* cx, return nullptr; } supportsIndices = enumerator.supportsIndices(); - ownPropertyCount = enumerator.ownPropertyCount(); MOZ_ASSERT_IF(WantIndices && supportsIndices, keys.length() == indices.length()); } @@ -1248,24 +1245,19 @@ static PropertyIteratorObject* GetIteratorImpl(JSContext* cx, // // In debug builds, AssertDenseElementsNotIterated is used to check the flag // is set correctly. - if (!SkipRegistration) { - if (obj->is<NativeObject>() && - obj->as<NativeObject>().getDenseInitializedLength() > 0) { - obj->as<NativeObject>().markDenseElementsMaybeInIteration(); - } + if (obj->is<NativeObject>() && + obj->as<NativeObject>().getDenseInitializedLength() > 0) { + obj->as<NativeObject>().markDenseElementsMaybeInIteration(); } PropertyIndexVector* indicesPtr = WantIndices && supportsIndices ? &indices : nullptr; - PropertyIteratorObject* iterobj = - CreatePropertyIterator(cx, obj, keys, supportsIndices, indicesPtr, - cacheableProtoChainLength, ownPropertyCount); + PropertyIteratorObject* iterobj = CreatePropertyIterator( + cx, obj, keys, supportsIndices, indicesPtr, cacheableProtoChainLength); if (!iterobj) { return nullptr; } - if (!SkipRegistration) { - RegisterEnumerator(cx, iterobj->getNativeIterator(), obj); - } + RegisterEnumerator(cx, iterobj->getNativeIterator()); cx->check(iterobj); MOZ_ASSERT_IF( @@ -1291,34 +1283,24 @@ static PropertyIteratorObject* GetIteratorImpl(JSContext* cx, } PropertyIteratorObject* js::GetIterator(JSContext* cx, HandleObject obj) { - return GetIteratorImpl<false, false>(cx, obj); + return GetIteratorImpl<false>(cx, obj); } PropertyIteratorObject* js::GetIteratorWithIndices(JSContext* cx, HandleObject obj) { - return GetIteratorImpl<true, false>(cx, obj); -} - -PropertyIteratorObject* js::GetIteratorUnregistered(JSContext* cx, - HandleObject obj) { - return GetIteratorImpl<false, true>(cx, obj); -} - -PropertyIteratorObject* js::GetIteratorWithIndicesUnregistered( - JSContext* cx, HandleObject obj) { - return GetIteratorImpl<true, true>(cx, obj); + return GetIteratorImpl<true>(cx, obj); } PropertyIteratorObject* js::LookupInIteratorCache(JSContext* cx, HandleObject obj) { uint32_t dummy = 0; - return LookupInIteratorCache(cx, obj, &dummy, true); + return LookupInIteratorCache(cx, obj, &dummy); } PropertyIteratorObject* js::LookupInShapeIteratorCache(JSContext* cx, HandleObject obj) { uint32_t dummy = 0; - return LookupInShapeIteratorCache(cx, obj, &dummy, true); + return LookupInShapeIteratorCache(cx, obj, &dummy); } // ES 2017 draft 7.4.7. @@ -1674,7 +1656,7 @@ PropertyIteratorObject* GlobalObject::getOrCreateEmptyIterator(JSContext* cx) { if (!cx->global()->data().emptyIterator) { RootedIdVector props(cx); // Empty PropertyIteratorObject* iter = - CreatePropertyIterator(cx, nullptr, props, false, nullptr, 0, 0); + CreatePropertyIterator(cx, nullptr, props, false, nullptr, 0); if (!iter) { return nullptr; } diff --git a/js/src/vm/Iteration.h b/js/src/vm/Iteration.h @@ -193,6 +193,27 @@ class NativeIteratorListIter { } }; +// If an object only has own data properties, we can store a list of +// PropertyIndex that can be used in Ion to more efficiently access those +// properties in cases like `for (var key in obj) { ...obj[key]... }`. +enum class NativeIteratorIndices : uint32_t { + // The object being iterated does not support indices. + Unavailable = 0, + + // The object being iterated supports indices, but none have been + // allocated, because it has not yet been iterated by Ion code that + // can use indices-based access. + AvailableOnRequest = 1, + + // The object being iterated had indices allocated, but they were + // disabled due to a deleted property. + Disabled = 2, + + // The object being iterated had indices allocated, and they are + // still valid. + Valid = 3 +}; + class IteratorProperty { uintptr_t raw_ = 0; @@ -230,13 +251,31 @@ struct NativeIterator : public NativeIteratorListNode { // Internal iterator object. const GCPtr<JSObject*> iterObj_ = {}; - const GCPtr<Shape*> objShape_ = {}; - uint32_t propertyCount_ = 0; - uint32_t propertyCursor_; // initialized by constructor - uint32_t ownPropertyCount_; // initialized by constructor - HashNumber shapesHash_; // initialized by constructor - uint16_t protoShapeCount_ = 0; - uint8_t flags_ = 0; + + // The end of GCPtr<Shape*>s that appear directly after |this|, as part of an + // overall allocation that stores |*this|, shapes, iterated strings, and maybe + // indices. Once this has been fully initialized, it also equals the start of + // iterated strings. + GCPtr<Shape*>* shapesEnd_; // initialized by constructor + + // The next property, pointing into an array of strings directly after any + // GCPtr<Shape*>s that appear directly after |*this|, as part of an overall + // allocation that stores |*this|, shapes, iterated strings, and maybe + // indices. Strings are stored as a JSLinearString* with a low-bit tag + // indicating whether they were deleted while iterating this object, in which + // case they should be skipped. The post barrier for writing to this is + // handled in NativeIterator::NativeIterator by adding iterObj_ to the + // whole cell buffer, and no pre barrier is required because we never modify + // these after initialization. + IteratorProperty* propertyCursor_; // initialized by constructor + + // The limit/end of properties to iterate. Once |this| has been fully + // initialized, it also equals the start of indices, if indices are present, + // or the end of the full allocation storing |*this|, shapes, and strings, if + // indices are not present. + IteratorProperty* propertiesEnd_; // initialized by constructor + + HashNumber shapesHash_; // initialized by constructor public: // For cacheable native iterators, whether the iterator is currently @@ -269,21 +308,6 @@ struct NativeIterator : public NativeIteratorListNode { // null/undefined. static constexpr uint32_t IsEmptyIteratorSingleton = 0x8; - // NOTE: the three flags below pertain to iterator indices optimizations. - // If an object only has own data properties, we can store a list of - // PropertyIndex that can be used in Ion to more efficiently access those - // properties in cases like `for (var key in obj) { ...obj[key]... }`. - - // Whether the object supports indices, in the event that they are - // requested. Note that this is exclusive with IndicesAvailable - static constexpr uint32_t IndicesSupported = 0x10; - - // Whether space was initially reserved for indices for this iterator. - static constexpr uint32_t IndicesAllocated = 0x20; - - // Whether indices are actually valid in the reserved area - static constexpr uint32_t IndicesAvailable = 0x40; - // If any of these bits are set on a |NativeIterator|, it isn't // currently reusable. (An active |NativeIterator| can't be stolen // *right now*; a |NativeIterator| that's had its properties mutated @@ -292,16 +316,27 @@ struct NativeIterator : public NativeIteratorListNode { Active | HasUnvisitedPropertyDeletion; }; - // We have a full u32 for this, but due to the way we compute the address - // of indices in the MacroAssembler, we want to have a few extra bits of - // wiggle room for shifting - static constexpr uint32_t PropCountLimit = 1 << 30; + private: + static constexpr uint32_t FlagsBits = 4; + static constexpr uint32_t IndicesBits = 2; + + static constexpr uint32_t FlagsMask = (1 << FlagsBits) - 1; - // If it's really important we can increase the size of protoShapeCount_, - // but increasing it to 32 bits would add another word. - static constexpr uint32_t ShapeCountLimit = 1 << 16; + static constexpr uint32_t PropCountShift = IndicesBits + FlagsBits; + static constexpr uint32_t PropCountBits = 32 - PropCountShift; + + public: + static constexpr uint32_t IndicesShift = FlagsBits; + static constexpr uint32_t IndicesMask = ((1 << IndicesBits) - 1) + << IndicesShift; + + static constexpr uint32_t PropCountLimit = 1 << PropCountBits; private: + // Stores Flags bits and indices state in the lower bits and the initial + // property count above them. + uint32_t flagsAndCount_ = 0; + #ifdef DEBUG // If true, this iterator may contain indexed properties that came from // objects on the prototype chain. This is used by certain debug assertions. @@ -312,8 +347,9 @@ struct NativeIterator : public NativeIteratorListNode { // No further fields appear after here *in NativeIterator*, but this class is // always allocated with space tacked on immediately after |this| to store - // propertyCount_ IteratorProperty values, optionally propertyCount_ - // PropertyIndex values, and protoShapeCount_ GCPtr<Shape*> values. + // shapes p to |shapesEnd_|, iterated property names after that up to + // |propertiesEnd_|, and maybe PropertyIndex values up to |indices_end()|. + public: /** * Initialize a NativeIterator properly allocated for |props.length()| @@ -328,7 +364,7 @@ struct NativeIterator : public NativeIteratorListNode { NativeIterator(JSContext* cx, Handle<PropertyIteratorObject*> propIter, Handle<JSObject*> objBeingIterated, HandleIdVector props, bool supportsIndices, PropertyIndexVector* indices, - uint32_t numShapes, uint32_t ownPropertyCount, bool* hadError); + uint32_t numShapes, bool* hadError); JSObject* objectBeingIterated() const { return objectBeingIterated_; } @@ -341,30 +377,22 @@ struct NativeIterator : public NativeIteratorListNode { objectBeingIterated_ = nullptr; } - const GCPtr<Shape*>& objShape() const { return objShape_; } - - GCPtr<Shape*>* protoShapesBegin(size_t numProperties) const { - uintptr_t raw = reinterpret_cast<uintptr_t>(this); - uintptr_t propertiesStart = raw + offsetOfFirstProperty(); - uintptr_t propertiesEnd = - propertiesStart + numProperties * sizeof(IteratorProperty); - uintptr_t result = propertiesEnd; - if (flags_ & Flags::IndicesAllocated) { - result += numProperties * sizeof(PropertyIndex); - } - return reinterpret_cast<GCPtr<Shape*>*>(result); + GCPtr<Shape*>* shapesBegin() const { + static_assert( + alignof(GCPtr<Shape*>) <= alignof(NativeIterator), + "NativeIterator must be aligned to begin storing " + "GCPtr<Shape*>s immediately after it with no required padding"); + const NativeIterator* immediatelyAfter = this + 1; + auto* afterNonConst = const_cast<NativeIterator*>(immediatelyAfter); + return reinterpret_cast<GCPtr<Shape*>*>(afterNonConst); } - GCPtr<Shape*>* protoShapesBegin() const { - return protoShapesBegin(allocatedPropertyCount()); - } + GCPtr<Shape*>* shapesEnd() const { return shapesEnd_; } - GCPtr<Shape*>* protoShapesEnd() const { - return protoShapesBegin() + protoShapeCount_; + uint32_t shapeCount() const { + return mozilla::PointerRangeSize(shapesBegin(), shapesEnd()); } - uint32_t protoShapeCount() const { return protoShapeCount_; } - IteratorProperty* propertiesBegin() const { static_assert( alignof(GCPtr<Shape*>) >= alignof(IteratorProperty), @@ -378,32 +406,37 @@ struct NativeIterator : public NativeIteratorListNode { "present, with no padding space required for correct " "alignment"); - return reinterpret_cast<IteratorProperty*>(uintptr_t(this) + sizeof(*this)); - } + // We *could* just check the assertion below if we wanted, but the + // incompletely-initialized NativeIterator case matters for so little + // code that we prefer not imposing the condition-check on every single + // user. + MOZ_ASSERT(isInitialized(), + "NativeIterator must be initialized, or else |shapesEnd_| " + "isn't necessarily the start of properties and instead " + "|propertyCursor_| is"); - IteratorProperty* propertiesEnd() const { - return propertiesBegin() + propertyCount_; + return reinterpret_cast<IteratorProperty*>(shapesEnd_); } - IteratorProperty* nextProperty() const { - return propertiesBegin() + propertyCursor_; - } + IteratorProperty* propertiesEnd() const { return propertiesEnd_; } + + IteratorProperty* nextProperty() const { return propertyCursor_; } PropertyIndex* indicesBegin() const { // PropertyIndex must be able to be appear directly after the properties // array, with no padding required for correct alignment. static_assert(alignof(IteratorProperty) >= alignof(PropertyIndex)); - return reinterpret_cast<PropertyIndex*>(propertiesEnd()); + return reinterpret_cast<PropertyIndex*>(propertiesEnd_); } PropertyIndex* indicesEnd() const { - MOZ_ASSERT(flags_ & Flags::IndicesAllocated); - return indicesBegin() + propertyCount_ * sizeof(PropertyIndex); + MOZ_ASSERT(indicesState() == NativeIteratorIndices::Valid); + return indicesBegin() + numKeys() * sizeof(PropertyIndex); } MOZ_ALWAYS_INLINE JS::Value nextIteratedValueAndAdvance() { - while (propertyCursor_ < propertyCount_) { - IteratorProperty& prop = *nextProperty(); + while (propertyCursor_ < propertiesEnd_) { + IteratorProperty& prop = *propertyCursor_; incCursor(); if (prop.deleted()) { continue; @@ -411,6 +444,7 @@ struct NativeIterator : public NativeIteratorListNode { return JS::StringValue(prop.asString()); } + MOZ_ASSERT(propertyCursor_ == propertiesEnd_); return JS::MagicValue(JS_NO_ITER_VALUE); } @@ -433,27 +467,17 @@ struct NativeIterator : public NativeIteratorListNode { // Note: JIT code inlines |propertyCursor_| resetting when an iterator // ends: see |MacroAssembler::iteratorClose|. - propertyCursor_ = 0; + propertyCursor_ = propertiesBegin(); } bool previousPropertyWas(JS::Handle<JSLinearString*> str) { MOZ_ASSERT(isInitialized()); - return propertyCursor_ > 0 && - propertiesBegin()[propertyCursor_ - 1].asString() == str; + return propertyCursor_ > propertiesBegin() && + propertyCursor_[-1].asString() == str; } - size_t numKeys() const { return propertyCount_; } - - size_t ownPropertyCount() const { return ownPropertyCount_; } - - size_t allocatedPropertyCount() const { - // propertyCursor_ holds the number of allocated properties until - // the iterator is initialized. This is so we can know the proper layout - // of the trailing bytes if we trigger a GC inside the constructor. - if (!isInitialized()) { - return propertyCursor_; - } - return propertyCount_; + size_t numKeys() const { + return mozilla::PointerRangeSize(propertiesBegin(), propertiesEnd()); } JSObject* iterObj() const { return iterObj_; } @@ -465,7 +489,7 @@ struct NativeIterator : public NativeIteratorListNode { HashNumber shapesHash() const { return shapesHash_; } - bool isInitialized() const { return flags_ & Flags::Initialized; } + bool isInitialized() const { return flags() & Flags::Initialized; } size_t allocationSize() const; @@ -479,30 +503,60 @@ struct NativeIterator : public NativeIteratorListNode { #endif private: - bool indicesAllocated() const { return flags_ & Flags::IndicesAllocated; } + uint32_t flags() const { return flagsAndCount_ & FlagsMask; } - bool isUnlinked() const { return !prev_ && !next_; } + NativeIteratorIndices indicesState() const { + return NativeIteratorIndices((flagsAndCount_ & IndicesMask) >> + IndicesShift); + } - public: - bool indicesAvailable() const { return flags_ & Flags::IndicesAvailable; } + uint32_t initialPropertyCount() const { + return flagsAndCount_ >> PropCountShift; + } + + static uint32_t initialFlagsAndCount(uint32_t count) { + // No flags are initially set. + MOZ_ASSERT(count < PropCountLimit); + return count << PropCountShift; + } - bool indicesSupported() const { return flags_ & Flags::IndicesSupported; } + void setFlags(uint32_t flags) { + MOZ_ASSERT((flags & ~FlagsMask) == 0); + flagsAndCount_ = (flagsAndCount_ & ~FlagsMask) | flags; + } + + void setIndicesState(NativeIteratorIndices indices) { + uint32_t indicesBits = uint32_t(indices) << IndicesShift; + flagsAndCount_ = (flagsAndCount_ & ~IndicesMask) | indicesBits; + } + + bool indicesAllocated() const { + return indicesState() >= NativeIteratorIndices::Disabled; + } + + void markInitialized() { + MOZ_ASSERT(flags() == 0); + setFlags(Flags::Initialized); + } + + bool isUnlinked() const { return !prev_ && !next_; } + public: // Whether this is the shared empty iterator object used for iterating over // null/undefined. bool isEmptyIteratorSingleton() const { // Note: equivalent code is inlined in MacroAssembler::iteratorClose. - bool res = flags_ & Flags::IsEmptyIteratorSingleton; + bool res = flags() & Flags::IsEmptyIteratorSingleton; MOZ_ASSERT_IF( - res, flags_ == (Flags::Initialized | Flags::IsEmptyIteratorSingleton)); + res, flags() == (Flags::Initialized | Flags::IsEmptyIteratorSingleton)); MOZ_ASSERT_IF(res, !objectBeingIterated_); - MOZ_ASSERT_IF(res, propertyCount_ == 0); - MOZ_ASSERT_IF(res, protoShapeCount_ == 0); + MOZ_ASSERT_IF(res, initialPropertyCount() == 0); + MOZ_ASSERT_IF(res, shapeCount() == 0); MOZ_ASSERT_IF(res, isUnlinked()); return res; } void markEmptyIteratorSingleton() { - flags_ |= Flags::IsEmptyIteratorSingleton; + flagsAndCount_ |= Flags::IsEmptyIteratorSingleton; // isEmptyIteratorSingleton() has various debug assertions. MOZ_ASSERT(isEmptyIteratorSingleton()); @@ -511,40 +565,39 @@ struct NativeIterator : public NativeIteratorListNode { bool isActive() const { MOZ_ASSERT(isInitialized()); - return flags_ & Flags::Active; + return flags() & Flags::Active; } void markActive() { MOZ_ASSERT(isInitialized()); MOZ_ASSERT(!isEmptyIteratorSingleton()); - flags_ |= Flags::Active; + flagsAndCount_ |= Flags::Active; } void markInactive() { MOZ_ASSERT(isInitialized()); MOZ_ASSERT(!isEmptyIteratorSingleton()); - flags_ &= ~Flags::Active; + flagsAndCount_ &= ~Flags::Active; } bool isReusable() const { MOZ_ASSERT(isInitialized()); - if (!(flags_ & Flags::Initialized)) { - return false; - } - if (flags_ & Flags::Active) { - return false; - } - return true; + // Cached NativeIterators are reusable if they're not currently active + // and their properties array hasn't been mutated, i.e. if only + // |Flags::Initialized| is set. Using |Flags::NotReusable| to test + // would also work, but this formulation is safer against memory + // corruption. + return flags() == Flags::Initialized; } void markHasUnvisitedPropertyDeletion() { MOZ_ASSERT(isInitialized()); MOZ_ASSERT(!isEmptyIteratorSingleton()); - flags_ |= Flags::HasUnvisitedPropertyDeletion; + flagsAndCount_ |= Flags::HasUnvisitedPropertyDeletion; } void unmarkHasUnvisitedPropertyDeletion() { @@ -552,13 +605,21 @@ struct NativeIterator : public NativeIteratorListNode { MOZ_ASSERT(!isEmptyIteratorSingleton()); MOZ_ASSERT(hasUnvisitedPropertyDeletion()); - flags_ &= ~Flags::HasUnvisitedPropertyDeletion; + flagsAndCount_ &= ~Flags::HasUnvisitedPropertyDeletion; } bool hasUnvisitedPropertyDeletion() const { MOZ_ASSERT(isInitialized()); - return flags_ & Flags::HasUnvisitedPropertyDeletion; + return flags() & Flags::HasUnvisitedPropertyDeletion; + } + + bool hasValidIndices() const { + return indicesState() == NativeIteratorIndices::Valid; + } + + bool indicesAvailableOnRequest() const { + return indicesState() == NativeIteratorIndices::AvailableOnRequest; } // Indicates the native iterator may walk prototype properties. @@ -566,16 +627,16 @@ struct NativeIterator : public NativeIteratorListNode { // If we can use indices for this iterator, we know it doesn't have // prototype properties, and so we use this as a check for prototype // properties. - return !indicesAvailable() && !indicesSupported(); + return !hasValidIndices() && !indicesAvailableOnRequest(); } void disableIndices() { - // Clear the IndicesAvailable flag so we won't use the indices on this - // iterator, and ensure IndicesSupported is cleared as well, so we don't - // re-request an iterator with indices. However, we leave the - // IndicesAllocated flag because we need to free them later, and skip them - // when looking for shapes. - flags_ &= ~(Flags::IndicesAvailable | Flags::IndicesSupported); + // If we have allocated indices, set the state to Disabled. + // This will ensure that we don't use them, but we still + // free them correctly. + if (indicesState() == NativeIteratorIndices::Valid) { + setIndicesState(NativeIteratorIndices::Disabled); + } } void link(NativeIteratorListNode* other) { @@ -610,32 +671,24 @@ struct NativeIterator : public NativeIteratorListNode { return offsetof(NativeIterator, objectBeingIterated_); } - static constexpr size_t offsetOfProtoShapeCount() { - return offsetof(NativeIterator, protoShapeCount_); + static constexpr size_t offsetOfShapesEnd() { + return offsetof(NativeIterator, shapesEnd_); } static constexpr size_t offsetOfPropertyCursor() { return offsetof(NativeIterator, propertyCursor_); } - static constexpr size_t offsetOfPropertyCount() { - return offsetof(NativeIterator, propertyCount_); - } - - static constexpr size_t offsetOfOwnPropertyCount() { - return offsetof(NativeIterator, ownPropertyCount_); + static constexpr size_t offsetOfPropertiesEnd() { + return offsetof(NativeIterator, propertiesEnd_); } - static constexpr size_t offsetOfFlags() { - return offsetof(NativeIterator, flags_); + static constexpr size_t offsetOfFlagsAndCount() { + return offsetof(NativeIterator, flagsAndCount_); } - static constexpr size_t offsetOfObjectShape() { - return offsetof(NativeIterator, objShape_); - } - - static constexpr size_t offsetOfFirstProperty() { - // Properties are stored directly after |this|. + static constexpr size_t offsetOfFirstShape() { + // Shapes are stored directly after |this|. return sizeof(NativeIterator); } }; @@ -709,11 +762,6 @@ PropertyIteratorObject* LookupInShapeIteratorCache(JSContext* cx, PropertyIteratorObject* GetIterator(JSContext* cx, HandleObject obj); PropertyIteratorObject* GetIteratorWithIndices(JSContext* cx, HandleObject obj); -PropertyIteratorObject* GetIteratorUnregistered(JSContext* cx, - HandleObject obj); -PropertyIteratorObject* GetIteratorWithIndicesUnregistered(JSContext* cx, - HandleObject obj); - PropertyIteratorObject* ValueToIterator(JSContext* cx, HandleValue vp); void CloseIterator(JSObject* obj); diff --git a/js/src/vm/Realm.h b/js/src/vm/Realm.h @@ -215,18 +215,13 @@ class PropertyIteratorObject; struct IteratorHashPolicy { struct Lookup { - Shape* objShape; - Shape** protoShapes; - size_t numProtoShapes; + Shape** shapes; + size_t numShapes; HashNumber shapesHash; - Lookup(Shape* objShape, Shape** protoShapes, size_t numProtoShapes, - HashNumber shapesHash) - : objShape(objShape), - protoShapes(protoShapes), - numProtoShapes(numProtoShapes), - shapesHash(shapesHash) { - MOZ_ASSERT(objShape); + Lookup(Shape** shapes, size_t numShapes, HashNumber shapesHash) + : shapes(shapes), numShapes(numShapes), shapesHash(shapesHash) { + MOZ_ASSERT(numShapes > 0); } }; static HashNumber hash(const Lookup& lookup) { return lookup.shapesHash; }