From ac8df94b51f7df0c95903ceb17249a521f24055e Mon Sep 17 00:00:00 2001 From: Iain Ireland Date: Fri, 14 Mar 2025 20:33:19 +0000 Subject: [PATCH] Bug 1804104: Share more StoreDenseElementHole code r=jandem The two code paths were already almost identical. The only differences were that the Ion code used setupAlignedABICall and had a small optimization to reuse the condition flags an earlier test (on architectures with condition flags). Differential Revision: https://phabricator.services.mozilla.com/D241531 --- js/src/jit/CacheIRCompiler.cpp | 43 +----------- js/src/jit/CodeGenerator.cpp | 118 ++++++--------------------------- js/src/jit/CodeGenerator.h | 1 - js/src/jit/MacroAssembler.cpp | 51 ++++++++++++++ js/src/jit/MacroAssembler.h | 4 ++ 5 files changed, 76 insertions(+), 141 deletions(-) diff --git a/js/src/jit/CacheIRCompiler.cpp b/js/src/jit/CacheIRCompiler.cpp index e8d58e0f0b1f..9051c2801d82 100644 --- a/js/src/jit/CacheIRCompiler.cpp +++ b/js/src/jit/CacheIRCompiler.cpp @@ -6984,48 +6984,9 @@ bool CacheIRCompiler::emitStoreDenseElementHole(ObjOperandId objId, masm.spectreBoundsCheck32(index, initLength, spectreTemp, &outOfBounds); masm.jump(&inBounds); - // If we're out-of-bounds, only handle the index == initLength case. masm.bind(&outOfBounds); - masm.branch32(Assembler::NotEqual, initLength, index, failure->label()); - - // If index < capacity, we can add a dense element inline. If not we - // need to allocate more elements. - Label allocElement, addNewElement; - Address capacity(scratch, ObjectElements::offsetOfCapacity()); - masm.spectreBoundsCheck32(index, capacity, spectreTemp, &allocElement); - masm.jump(&addNewElement); - - masm.bind(&allocElement); - - LiveRegisterSet save = liveVolatileRegs(); - save.takeUnchecked(scratch); - masm.PushRegsInMask(save); - - using Fn = bool (*)(JSContext* cx, NativeObject* obj); - masm.setupUnalignedABICall(scratch); - masm.loadJSContext(scratch); - masm.passABIArg(scratch); - masm.passABIArg(obj); - masm.callWithABI(); - masm.storeCallPointerResult(scratch); - - masm.PopRegsInMask(save); - masm.branchIfFalseBool(scratch, failure->label()); - - // Load the reallocated elements pointer. - masm.loadPtr(Address(obj, NativeObject::offsetOfElements()), scratch); - - masm.bind(&addNewElement); - - // Increment initLength. - masm.add32(Imm32(1), initLength); - - // If length is now <= index, increment length too. - Label skipIncrementLength; - Address length(scratch, ObjectElements::offsetOfLength()); - masm.branch32(Assembler::Above, length, index, &skipIncrementLength); - masm.add32(Imm32(1), length); - masm.bind(&skipIncrementLength); + masm.prepareOOBStoreElement(obj, index, scratch, spectreTemp, + failure->label(), liveVolatileRegs()); // Skip EmitPreBarrier as the memory is uninitialized. masm.jump(&storeSkipPreBarrier); diff --git a/js/src/jit/CodeGenerator.cpp b/js/src/jit/CodeGenerator.cpp index d4d93ae66b1e..e110148547f9 100644 --- a/js/src/jit/CodeGenerator.cpp +++ b/js/src/jit/CodeGenerator.cpp @@ -14956,95 +14956,17 @@ void CodeGenerator::visitStoreHoleValueElement(LStoreHoleValueElement* lir) { masm.storeValue(MagicValue(JS_ELEMENTS_HOLE), element); } -void CodeGenerator::emitStoreElementHoleOOL(LInstruction* lir) { - Register object, elements, index; - mozilla::Maybe value; - Register temp; - - if (lir->isStoreElementHoleV()) { - LStoreElementHoleV* store = lir->toStoreElementHoleV(); - object = ToRegister(store->object()); - elements = ToRegister(store->elements()); - index = ToRegister(store->index()); - value.emplace(TypedOrValueRegister(ToValue(store->value()))); - temp = ToRegister(store->temp0()); - } else { - LStoreElementHoleT* store = lir->toStoreElementHoleT(); - object = ToRegister(store->object()); - elements = ToRegister(store->elements()); - index = ToRegister(store->index()); - if (store->value()->isConstant()) { - value.emplace( - ConstantOrRegister(store->value()->toConstant()->toJSValue())); - } else { - MIRType valueType = store->mir()->value()->type(); - value.emplace( - TypedOrValueRegister(valueType, ToAnyRegister(store->value()))); - } - temp = ToRegister(store->temp0()); - } - - Address initLength(elements, ObjectElements::offsetOfInitializedLength()); - - // We're out-of-bounds. We only handle the index == initlength case. - // If index > initializedLength, bail out. Note that this relies on the - // condition flags sticking from the incoming branch. - // Also note: this branch does not need Spectre mitigations, doing that for - // the capacity check below is sufficient. - Label allocElement, addNewElement; -#if defined(JS_CODEGEN_MIPS64) || defined(JS_CODEGEN_LOONG64) || \ - defined(JS_CODEGEN_RISCV64) - // Had to reimplement for MIPS because there are no flags. - bailoutCmp32(Assembler::NotEqual, initLength, index, lir->snapshot()); -#else - bailoutIf(Assembler::NotEqual, lir->snapshot()); -#endif - - // If index < capacity, we can add a dense element inline. If not, we need - // to allocate more elements first. - masm.spectreBoundsCheck32( - index, Address(elements, ObjectElements::offsetOfCapacity()), temp, - &allocElement); - masm.jump(&addNewElement); - - masm.bind(&allocElement); - - // Save all live volatile registers, except |temp|. - LiveRegisterSet liveRegs = liveVolatileRegs(lir); - liveRegs.takeUnchecked(temp); - masm.PushRegsInMask(liveRegs); - - masm.setupAlignedABICall(); - masm.loadJSContext(temp); - masm.passABIArg(temp); - masm.passABIArg(object); - - using Fn = bool (*)(JSContext*, NativeObject*); - masm.callWithABI(); - masm.storeCallPointerResult(temp); - - masm.PopRegsInMask(liveRegs); - bailoutIfFalseBool(temp, lir->snapshot()); - - // Load the reallocated elements pointer. - masm.loadPtr(Address(object, NativeObject::offsetOfElements()), elements); - - masm.bind(&addNewElement); - - // Increment initLength - masm.add32(Imm32(1), initLength); - - // If length is now <= index, increment length too. - Label skipIncrementLength; - Address length(elements, ObjectElements::offsetOfLength()); - masm.branch32(Assembler::Above, length, index, &skipIncrementLength); - masm.add32(Imm32(1), length); - masm.bind(&skipIncrementLength); -} - void CodeGenerator::visitStoreElementHoleT(LStoreElementHoleT* lir) { + Register obj = ToRegister(lir->object()); + Register elements = ToRegister(lir->elements()); + Register index = ToRegister(lir->index()); + Register temp = ToRegister(lir->temp0()); + auto* ool = new (alloc()) LambdaOutOfLineCode([=](OutOfLineCode& ool) { - emitStoreElementHoleOOL(lir); + Label bail; + masm.prepareOOBStoreElement(obj, index, elements, temp, &bail, + liveVolatileRegs(lir)); + bailoutFrom(&bail, lir->snapshot()); // Jump to the inline path where we will store the value. // We rejoin after the prebarrier, because the memory is uninitialized. @@ -15052,11 +14974,6 @@ void CodeGenerator::visitStoreElementHoleT(LStoreElementHoleT* lir) { }); addOutOfLineCode(ool, lir->mir()); - Register obj = ToRegister(lir->object()); - Register elements = ToRegister(lir->elements()); - Register index = ToRegister(lir->index()); - Register temp = ToRegister(lir->temp0()); - Address initLength(elements, ObjectElements::offsetOfInitializedLength()); masm.spectreBoundsCheck32(index, initLength, temp, ool->entry()); @@ -15075,8 +14992,17 @@ void CodeGenerator::visitStoreElementHoleT(LStoreElementHoleT* lir) { } void CodeGenerator::visitStoreElementHoleV(LStoreElementHoleV* lir) { + Register obj = ToRegister(lir->object()); + Register elements = ToRegister(lir->elements()); + Register index = ToRegister(lir->index()); + ValueOperand value = ToValue(lir->value()); + Register temp = ToRegister(lir->temp0()); + auto* ool = new (alloc()) LambdaOutOfLineCode([=](OutOfLineCode& ool) { - emitStoreElementHoleOOL(lir); + Label bail; + masm.prepareOOBStoreElement(obj, index, elements, temp, &bail, + liveVolatileRegs(lir)); + bailoutFrom(&bail, lir->snapshot()); // Jump to the inline path where we will store the value. // We rejoin after the prebarrier, because the memory is uninitialized. @@ -15084,12 +15010,6 @@ void CodeGenerator::visitStoreElementHoleV(LStoreElementHoleV* lir) { }); addOutOfLineCode(ool, lir->mir()); - Register obj = ToRegister(lir->object()); - Register elements = ToRegister(lir->elements()); - Register index = ToRegister(lir->index()); - ValueOperand value = ToValue(lir->value()); - Register temp = ToRegister(lir->temp0()); - Address initLength(elements, ObjectElements::offsetOfInitializedLength()); masm.spectreBoundsCheck32(index, initLength, temp, ool->entry()); diff --git a/js/src/jit/CodeGenerator.h b/js/src/jit/CodeGenerator.h index e8c54b1f439d..ddf64dbf0091 100644 --- a/js/src/jit/CodeGenerator.h +++ b/js/src/jit/CodeGenerator.h @@ -117,7 +117,6 @@ class CodeGenerator final : public CodeGeneratorSpecific { void visitOutOfLineCallVM( OutOfLineCallVM* ool); - void emitStoreElementHoleOOL(LInstruction* lir); void emitIsCallableOOL(Register object, Register output); void emitResumableWasmTrapOOL(LInstruction* lir, size_t framePushed, diff --git a/js/src/jit/MacroAssembler.cpp b/js/src/jit/MacroAssembler.cpp index 36fc1e55f889..251d77d065f8 100644 --- a/js/src/jit/MacroAssembler.cpp +++ b/js/src/jit/MacroAssembler.cpp @@ -9300,6 +9300,57 @@ void MacroAssembler::registerIterator(Register enumeratorsList, Register iter, storePtr(iter, Address(enumeratorsList, NativeIterator::offsetOfPrev())); } +void MacroAssembler::prepareOOBStoreElement(Register object, Register index, + Register elements, + Register spectreTemp, + Label* failure, + LiveRegisterSet volatileLiveRegs) { + Address length(elements, ObjectElements::offsetOfLength()); + Address initLength(elements, ObjectElements::offsetOfInitializedLength()); + Address capacity(elements, ObjectElements::offsetOfCapacity()); + + // We only handle the index == initLength case. + branch32(Assembler::NotEqual, initLength, index, failure); + + // If index < capacity, we can add a dense element inline. If not, we + // need to allocate more elements. + Label allocElement, addNewElement; + spectreBoundsCheck32(index, capacity, spectreTemp, &allocElement); + jump(&addNewElement); + + bind(&allocElement); + + volatileLiveRegs.takeUnchecked(elements); + volatileLiveRegs.takeUnchecked(spectreTemp); + PushRegsInMask(volatileLiveRegs); + + // Use `elements` as a scratch register because we're about to reallocate it. + using Fn = bool (*)(JSContext* cx, NativeObject* obj); + setupUnalignedABICall(elements); + loadJSContext(elements); + passABIArg(elements); + passABIArg(object); + callWithABI(); + storeCallPointerResult(elements); + + PopRegsInMask(volatileLiveRegs); + branchIfFalseBool(elements, failure); + + // Load the reallocated elements pointer. + loadPtr(Address(object, NativeObject::offsetOfElements()), elements); + + bind(&addNewElement); + + // Increment initLength. + add32(Imm32(1), initLength); + + // If length is now <= index, increment length too. + Label skipIncrementLength; + branch32(Assembler::Above, length, index, &skipIncrementLength); + add32(Imm32(1), length); + bind(&skipIncrementLength); +} + void MacroAssembler::toHashableNonGCThing(ValueOperand value, ValueOperand result, FloatRegister tempFloat) { diff --git a/js/src/jit/MacroAssembler.h b/js/src/jit/MacroAssembler.h index a62a337eeaef..853ba9ecc116 100644 --- a/js/src/jit/MacroAssembler.h +++ b/js/src/jit/MacroAssembler.h @@ -5404,6 +5404,10 @@ class MacroAssembler : public MacroAssemblerSpecific { Register temp3); void registerIterator(Register enumeratorsList, Register iter, Register temp); + void prepareOOBStoreElement(Register object, Register index, + Register elements, Register spectreTemp, + Label* failure, LiveRegisterSet volatileLiveRegs); + void toHashableNonGCThing(ValueOperand value, ValueOperand result, FloatRegister tempFloat);