From ff044caf809a9bc31c5c2b761343caf91a11bfb6 Mon Sep 17 00:00:00 2001 From: Jan de Mooij Date: Tue, 21 Jul 2020 14:12:54 +0000 Subject: [PATCH] Bug 1653246 part 5 - Optimize DataView set* methods in CacheIR. r=anba Pretty similar to the previous patch. To deal with register pressure on 32-bit x86, the patch adds copyToScratchRegister to load an operand into a scratch register, as alternative for useRegister. Differential Revision: https://phabricator.services.mozilla.com/D84128 --- js/src/jit/CacheIR.cpp | 105 ++++++++++- js/src/jit/CacheIR.h | 5 +- js/src/jit/CacheIRCompiler.cpp | 307 +++++++++++++++++++++++++++++++-- js/src/jit/CacheIRCompiler.h | 14 +- js/src/jit/CacheIROps.yaml | 11 ++ 5 files changed, 416 insertions(+), 26 deletions(-) diff --git a/js/src/jit/CacheIR.cpp b/js/src/jit/CacheIR.cpp index 63c025d4577f..075181f2b605 100644 --- a/js/src/jit/CacheIR.cpp +++ b/js/src/jit/CacheIR.cpp @@ -3589,8 +3589,7 @@ AttachDecision SetPropIRGenerator::tryAttachNativeSetSlot(HandleObject obj, return AttachDecision::Attach; } -OperandId SetPropIRGenerator::emitNumericGuard(ValOperandId valId, - Scalar::Type type) { +OperandId IRGenerator::emitNumericGuard(ValOperandId valId, Scalar::Type type) { switch (type) { case Scalar::Int8: case Scalar::Uint8: @@ -5198,6 +5197,88 @@ AttachDecision CallIRGenerator::tryAttachDataViewGet(HandleFunction callee, return AttachDecision::Attach; } +AttachDecision CallIRGenerator::tryAttachDataViewSet(HandleFunction callee, + Scalar::Type type) { + // Ensure |this| is a DataViewObject. + if (!thisval_.isObject() || !thisval_.toObject().is()) { + return AttachDecision::NoAction; + } + + // Expected arguments: offset (number), value, optional littleEndian (boolean) + if (argc_ < 2 || argc_ > 3) { + return AttachDecision::NoAction; + } + if (!args_[0].isNumber()) { + return AttachDecision::NoAction; + } + if (Scalar::isBigIntType(type)) { + if (!args_[1].isBigInt()) { + return AttachDecision::NoAction; + } + } else { + if (!args_[1].isNumber()) { + return AttachDecision::NoAction; + } + } + if (argc_ > 2 && !args_[2].isBoolean()) { + return AttachDecision::NoAction; + } + + DataViewObject* dv = &thisval_.toObject().as(); + + // Bounds check the offset. + int32_t offsetInt32; + if (!mozilla::NumberEqualsInt32(args_[0].toNumber(), &offsetInt32)) { + return AttachDecision::NoAction; + } + if (offsetInt32 < 0 || + !dv->offsetIsInBounds(Scalar::byteSize(type), offsetInt32)) { + return AttachDecision::NoAction; + } + + // Initialize the input operand. + Int32OperandId argcId(writer.setInputOperandId(0)); + + // Guard callee is this DataView native function. + emitNativeCalleeGuard(callee); + + // Guard |this| is a DataViewObject. + ValOperandId thisValId = + writer.loadArgumentFixedSlot(ArgumentKind::This, argc_); + ObjOperandId objId = writer.guardToObject(thisValId); + writer.guardClass(objId, GuardClassKind::DataView); + + // Convert offset to int32. + ValOperandId offsetId = + writer.loadArgumentFixedSlot(ArgumentKind::Arg0, argc_); + Int32OperandId int32OffsetId = writer.guardToInt32Index(offsetId); + + // Convert value to number or BigInt. + ValOperandId valueId = + writer.loadArgumentFixedSlot(ArgumentKind::Arg1, argc_); + OperandId numericValueId = emitNumericGuard(valueId, type); + + // TODO(Warp): add BooleanOperandId (bug 1647602). + Int32OperandId boolLittleEndianId; + if (argc_ > 2) { + ValOperandId littleEndianId = + writer.loadArgumentFixedSlot(ArgumentKind::Arg2, argc_); + boolLittleEndianId = writer.guardToBoolean(littleEndianId); + } else { + boolLittleEndianId = writer.loadInt32Constant(0); + } + + writer.storeDataViewValueResult(objId, int32OffsetId, numericValueId, + boolLittleEndianId, type); + + // This stub doesn't need type monitoring, it always returns |undefined|. + writer.returnFromIC(); + cacheIRStubKind_ = BaselineCacheIRStubKind::Regular; + + trackAttached("DataViewSet"); + return AttachDecision::Attach; +} + AttachDecision CallIRGenerator::tryAttachUnsafeGetReservedSlot( HandleFunction callee, InlinableNative native) { // Need an object and int32 slot argument. @@ -6471,6 +6552,26 @@ AttachDecision CallIRGenerator::tryAttachInlinableNative( return tryAttachDataViewGet(callee, Scalar::BigInt64); case InlinableNative::DataViewGetBigUint64: return tryAttachDataViewGet(callee, Scalar::BigUint64); + case InlinableNative::DataViewSetInt8: + return tryAttachDataViewSet(callee, Scalar::Int8); + case InlinableNative::DataViewSetUint8: + return tryAttachDataViewSet(callee, Scalar::Uint8); + case InlinableNative::DataViewSetInt16: + return tryAttachDataViewSet(callee, Scalar::Int16); + case InlinableNative::DataViewSetUint16: + return tryAttachDataViewSet(callee, Scalar::Uint16); + case InlinableNative::DataViewSetInt32: + return tryAttachDataViewSet(callee, Scalar::Int32); + case InlinableNative::DataViewSetUint32: + return tryAttachDataViewSet(callee, Scalar::Uint32); + case InlinableNative::DataViewSetFloat32: + return tryAttachDataViewSet(callee, Scalar::Float32); + case InlinableNative::DataViewSetFloat64: + return tryAttachDataViewSet(callee, Scalar::Float64); + case InlinableNative::DataViewSetBigInt64: + return tryAttachDataViewSet(callee, Scalar::BigInt64); + case InlinableNative::DataViewSetBigUint64: + return tryAttachDataViewSet(callee, Scalar::BigUint64); // Intl natives. case InlinableNative::IntlGuardToCollator: diff --git a/js/src/jit/CacheIR.h b/js/src/jit/CacheIR.h index dc7e971c9558..9cfff7d20fdd 100644 --- a/js/src/jit/CacheIR.h +++ b/js/src/jit/CacheIR.h @@ -1085,6 +1085,8 @@ class MOZ_RAII IRGenerator { void emitIdGuard(ValOperandId valId, jsid id); + OperandId emitNumericGuard(ValOperandId valId, Scalar::Type type); + friend class CacheIRSpewer; public: @@ -1370,8 +1372,6 @@ class MOZ_RAII SetPropIRGenerator : public IRGenerator { // matches |id|. void maybeEmitIdGuard(jsid id); - OperandId emitNumericGuard(ValOperandId valId, Scalar::Type type); - AttachDecision tryAttachNativeSetSlot(HandleObject obj, ObjOperandId objId, HandleId id, ValOperandId rhsId); AttachDecision tryAttachUnboxedExpandoSetSlot(HandleObject obj, @@ -1573,6 +1573,7 @@ class MOZ_RAII CallIRGenerator : public IRGenerator { AttachDecision tryAttachArrayJoin(HandleFunction callee); AttachDecision tryAttachArrayIsArray(HandleFunction callee); AttachDecision tryAttachDataViewGet(HandleFunction callee, Scalar::Type type); + AttachDecision tryAttachDataViewSet(HandleFunction callee, Scalar::Type type); AttachDecision tryAttachUnsafeGetReservedSlot(HandleFunction callee, InlinableNative native); AttachDecision tryAttachIsSuspendedGenerator(HandleFunction callee); diff --git a/js/src/jit/CacheIRCompiler.cpp b/js/src/jit/CacheIRCompiler.cpp index 19e479a05475..02fabc514ae5 100644 --- a/js/src/jit/CacheIRCompiler.cpp +++ b/js/src/jit/CacheIRCompiler.cpp @@ -8,6 +8,7 @@ #include "mozilla/ArrayUtils.h" #include "mozilla/FunctionTypeTraits.h" +#include "mozilla/MaybeOneOf.h" #include "mozilla/ScopeExit.h" #include @@ -114,12 +115,12 @@ ValueOperand CacheRegisterAllocator::useValueRegister(MacroAssembler& masm, // guarded isNumber on the provided val. void CacheRegisterAllocator::ensureDoubleRegister(MacroAssembler& masm, NumberOperandId op, - FloatRegister dest) { + FloatRegister dest) const { // If AutoScratchFloatRegister is active, we have to add sizeof(double) to // any stack slot offsets below. int32_t stackOffset = hasAutoScratchFloatRegisterSpill() ? sizeof(double) : 0; - OperandLocation& loc = operandLocations_[op.id()]; + const OperandLocation& loc = operandLocations_[op.id()]; Label failure, done; switch (loc.kind()) { @@ -184,6 +185,53 @@ void CacheRegisterAllocator::ensureDoubleRegister(MacroAssembler& masm, masm.bind(&done); } +void CacheRegisterAllocator::copyToScratchRegister(MacroAssembler& masm, + TypedOperandId typedId, + Register dest) const { + // If AutoScratchFloatRegister is active, we have to add sizeof(double) to + // any stack slot offsets below. + int32_t stackOffset = hasAutoScratchFloatRegisterSpill() ? sizeof(double) : 0; + + const OperandLocation& loc = operandLocations_[typedId.id()]; + + Label failure, done; + switch (loc.kind()) { + case OperandLocation::ValueReg: { + masm.unboxNonDouble(loc.valueReg(), dest, typedId.type()); + break; + } + case OperandLocation::ValueStack: { + Address addr = valueAddress(masm, &loc); + addr.offset += stackOffset; + masm.unboxNonDouble(addr, dest, typedId.type()); + break; + } + case OperandLocation::BaselineFrame: { + Address addr = addressOf(masm, loc.baselineFrameSlot()); + addr.offset += stackOffset; + masm.unboxNonDouble(addr, dest, typedId.type()); + break; + } + case OperandLocation::PayloadReg: { + MOZ_ASSERT(loc.payloadType() == typedId.type()); + masm.mov(loc.payloadReg(), dest); + return; + } + case OperandLocation::PayloadStack: { + MOZ_ASSERT(loc.payloadType() == typedId.type()); + MOZ_ASSERT(loc.payloadStack() <= stackPushed_); + Address addr(masm.getStackPointer(), stackPushed_ - loc.payloadStack()); + addr.offset += stackOffset; + masm.loadPtr(addr, dest); + return; + } + case OperandLocation::DoubleReg: + case OperandLocation::Constant: + case OperandLocation::Uninitialized: + MOZ_CRASH("Unhandled operand location"); + } +} + ValueOperand CacheRegisterAllocator::useFixedValueRegister(MacroAssembler& masm, ValOperandId valId, ValueOperand reg) { @@ -779,7 +827,7 @@ void CacheRegisterAllocator::popPayload(MacroAssembler& masm, } Address CacheRegisterAllocator::valueAddress(MacroAssembler& masm, - OperandLocation* loc) { + const OperandLocation* loc) const { MOZ_ASSERT(loc >= operandLocations_.begin() && loc < operandLocations_.end()); return Address(masm.getStackPointer(), stackPushed_ - loc->valueStack()); } @@ -4476,6 +4524,24 @@ bool CacheIRCompiler::emitLoadTypedObjectElementResult( /* allowDoubleForUint32 = */ true); } +static void EmitDataViewBoundsCheck(MacroAssembler& masm, size_t byteSize, + Register obj, Register offset, + Register scratch, Label* fail) { + // Ensure both offset < length and offset + (byteSize - 1) < length. + static_assert(ArrayBufferObject::MaxBufferByteLength <= INT32_MAX, + "Code assumes DataView length fits in int32"); + masm.unboxInt32(Address(obj, DataViewObject::lengthOffset()), scratch); + if (byteSize == 1) { + masm.spectreBoundsCheck32(offset, scratch, InvalidReg, fail); + } else { + // temp := length - (byteSize - 1) + // if temp < 0: fail + // if offset >= temp: fail + masm.branchSub32(Assembler::Signed, Imm32(byteSize - 1), scratch, fail); + masm.spectreBoundsCheck32(offset, scratch, InvalidReg, fail); + } +} + bool CacheIRCompiler::emitLoadDataViewValueResult(ObjOperandId objId, Int32OperandId offsetId, Int32OperandId littleEndianId, @@ -4500,22 +4566,8 @@ bool CacheIRCompiler::emitLoadDataViewValueResult(ObjOperandId objId, const size_t byteSize = Scalar::byteSize(elementType); - // Ensure both offset < length and offset + (byteSize - 1) < length. - static_assert(ArrayBufferObject::MaxBufferByteLength <= INT32_MAX, - "Code assumes DataView length fits in int32"); - masm.unboxInt32(Address(obj, DataViewObject::lengthOffset()), outputScratch); - if (byteSize == 1) { - masm.spectreBoundsCheck32(offset, outputScratch, InvalidReg, - failure->label()); - } else { - // temp := length - (byteSize - 1) - // if temp < 0: fail - // if offset >= temp: fail - masm.branchSub32(Assembler::Signed, Imm32(byteSize - 1), outputScratch, - failure->label()); - masm.spectreBoundsCheck32(offset, outputScratch, InvalidReg, - failure->label()); - } + EmitDataViewBoundsCheck(masm, byteSize, obj, offset, outputScratch, + failure->label()); masm.loadPtr(Address(obj, DataViewObject::dataOffset()), outputScratch); @@ -4645,6 +4697,223 @@ bool CacheIRCompiler::emitLoadDataViewValueResult(ObjOperandId objId, return true; } +bool CacheIRCompiler::emitStoreDataViewValueResult( + ObjOperandId objId, Int32OperandId offsetId, uint32_t valueId, + Int32OperandId littleEndianId, Scalar::Type elementType) { + JitSpew(JitSpew_Codegen, "%s", __FUNCTION__); + + AutoOutputRegister output(*this); +#ifdef JS_CODEGEN_X86 + // Use a scratch register to avoid running out of the registers. + Register obj = output.valueReg().typeReg(); + allocator.copyToScratchRegister(masm, objId, obj); +#else + Register obj = allocator.useRegister(masm, objId); +#endif + Register offset = allocator.useRegister(masm, offsetId); + Register littleEndian = allocator.useRegister(masm, littleEndianId); + + AutoAvailableFloatRegister floatScratch0(*this, FloatReg0); + Maybe valInt32; + Maybe valBigInt; + switch (elementType) { + case Scalar::Int8: + case Scalar::Uint8: + case Scalar::Int16: + case Scalar::Uint16: + case Scalar::Int32: + case Scalar::Uint32: + case Scalar::Uint8Clamped: + valInt32.emplace(allocator.useRegister(masm, Int32OperandId(valueId))); + break; + + case Scalar::Float32: + case Scalar::Float64: + allocator.ensureDoubleRegister(masm, NumberOperandId(valueId), + floatScratch0); + break; + + case Scalar::BigInt64: + case Scalar::BigUint64: + valBigInt.emplace(allocator.useRegister(masm, BigIntOperandId(valueId))); + break; + + case Scalar::MaxTypedArrayViewType: + case Scalar::Int64: + case Scalar::Simd128: + MOZ_CRASH("Unsupported type"); + } + + Register scratch1 = output.valueReg().scratchReg(); + MOZ_ASSERT(scratch1 != obj, "scratchReg must not be typeReg"); + + // On platforms with enough registers, |scratch2| is an extra scratch register + // (pair) used for byte-swapping the value. +#ifndef JS_CODEGEN_X86 + mozilla::MaybeOneOf scratch2; + switch (elementType) { + case Scalar::Int8: + case Scalar::Uint8: + break; + case Scalar::Int16: + case Scalar::Uint16: + case Scalar::Int32: + case Scalar::Uint32: + case Scalar::Float32: + scratch2.construct(allocator, masm); + break; + case Scalar::Float64: + case Scalar::BigInt64: + case Scalar::BigUint64: + scratch2.construct(allocator, masm); + break; + case Scalar::Uint8Clamped: + default: + MOZ_CRASH("Invalid type"); + } +#endif + + FailurePath* failure; + if (!addFailurePath(&failure)) { + return false; + } + + const size_t byteSize = Scalar::byteSize(elementType); + + EmitDataViewBoundsCheck(masm, byteSize, obj, offset, scratch1, + failure->label()); + + masm.loadPtr(Address(obj, DataViewObject::dataOffset()), scratch1); + BaseIndex dest(scratch1, offset, TimesOne); + + if (byteSize == 1) { + // Byte swapping has no effect, so just do the byte store. + masm.store8(*valInt32, dest); + masm.moveValue(UndefinedValue(), output.valueReg()); + return true; + } + + // On 32-bit x86, |obj| is already a scratch register so use that. If we need + // a Register64 we also use the littleEndian register and use the stack + // location for the check below. + bool pushedLittleEndian = false; +#ifdef JS_CODEGEN_X86 + if (byteSize == 8) { + masm.push(littleEndian); + pushedLittleEndian = true; + } + auto valScratch32 = [&]() -> Register { return obj; }; + auto valScratch64 = [&]() -> Register64 { + return Register64(obj, littleEndian); + }; +#else + auto valScratch32 = [&]() -> Register { + return scratch2.ref(); + }; + auto valScratch64 = [&]() -> Register64 { + return scratch2.ref(); + }; +#endif + + // Load the value into a gpr register. + switch (elementType) { + case Scalar::Int16: + case Scalar::Uint16: + case Scalar::Int32: + case Scalar::Uint32: + masm.move32(*valInt32, valScratch32()); + break; + case Scalar::Float32: { + FloatRegister scratchFloat32 = floatScratch0.get().asSingle(); + masm.convertDoubleToFloat32(floatScratch0, scratchFloat32); + masm.canonicalizeFloatIfDeterministic(scratchFloat32); + masm.moveFloat32ToGPR(scratchFloat32, valScratch32()); + break; + } + case Scalar::Float64: { + masm.canonicalizeDoubleIfDeterministic(floatScratch0); + masm.moveDoubleToGPR64(floatScratch0, valScratch64()); + break; + } + case Scalar::BigInt64: + case Scalar::BigUint64: + masm.loadBigInt64(*valBigInt, valScratch64()); + break; + case Scalar::Int8: + case Scalar::Uint8: + case Scalar::Uint8Clamped: + default: + MOZ_CRASH("Invalid type"); + } + + // Swap the bytes in the loaded value. + Label skip; + if (pushedLittleEndian) { + masm.branch32(MOZ_LITTLE_ENDIAN() ? Assembler::NotEqual : Assembler::Equal, + Address(masm.getStackPointer(), 0), Imm32(0), &skip); + } else { + masm.branch32(MOZ_LITTLE_ENDIAN() ? Assembler::NotEqual : Assembler::Equal, + littleEndian, Imm32(0), &skip); + } + switch (elementType) { + case Scalar::Int16: + masm.byteSwap16SignExtend(valScratch32()); + break; + case Scalar::Uint16: + masm.byteSwap16ZeroExtend(valScratch32()); + break; + case Scalar::Int32: + case Scalar::Uint32: + case Scalar::Float32: + masm.byteSwap32(valScratch32()); + break; + case Scalar::Float64: + case Scalar::BigInt64: + case Scalar::BigUint64: + masm.byteSwap64(valScratch64()); + break; + case Scalar::Int8: + case Scalar::Uint8: + case Scalar::Uint8Clamped: + default: + MOZ_CRASH("Invalid type"); + } + masm.bind(&skip); + + // Store the value. + switch (elementType) { + case Scalar::Int16: + case Scalar::Uint16: + masm.store16Unaligned(valScratch32(), dest); + break; + case Scalar::Int32: + case Scalar::Uint32: + case Scalar::Float32: + masm.store32Unaligned(valScratch32(), dest); + break; + case Scalar::Float64: + case Scalar::BigInt64: + case Scalar::BigUint64: + masm.store64Unaligned(valScratch64(), dest); + break; + case Scalar::Int8: + case Scalar::Uint8: + case Scalar::Uint8Clamped: + default: + MOZ_CRASH("Invalid typed array type"); + } + +#ifdef JS_CODEGEN_X86 + // Restore registers. + if (pushedLittleEndian) { + masm.pop(littleEndian); + } +#endif + + masm.moveValue(UndefinedValue(), output.valueReg()); + return true; +} + bool CacheIRCompiler::emitStoreTypedObjectScalarProperty( ObjOperandId objId, uint32_t offsetOffset, TypedThingLayout layout, Scalar::Type type, uint32_t rhsId) { diff --git a/js/src/jit/CacheIRCompiler.h b/js/src/jit/CacheIRCompiler.h index 62751defaa6b..d56a76fe2914 100644 --- a/js/src/jit/CacheIRCompiler.h +++ b/js/src/jit/CacheIRCompiler.h @@ -347,7 +347,7 @@ class MOZ_RAII CacheRegisterAllocator { void popPayload(MacroAssembler& masm, OperandLocation* loc, Register dest); void popValue(MacroAssembler& masm, OperandLocation* loc, ValueOperand dest); - Address valueAddress(MacroAssembler& masm, OperandLocation* loc); + Address valueAddress(MacroAssembler& masm, const OperandLocation* loc) const; #ifdef DEBUG void assertValidState() const; @@ -496,9 +496,17 @@ class MOZ_RAII CacheRegisterAllocator { // Loads (potentially coercing) and unboxes a value into a float register // This is infallible, as there should have been a previous guard - // to ensure the ValOperandId is already a number. + // to ensure the value is already a number. + // Does not change the allocator's state. void ensureDoubleRegister(MacroAssembler& masm, NumberOperandId op, - FloatRegister dest); + FloatRegister dest) const; + + // Loads an unboxed value into a scratch register. This can be useful + // especially on 32-bit x86 when there are not enough registers for + // useRegister. + // Does not change the allocator's state. + void copyToScratchRegister(MacroAssembler& masm, TypedOperandId typedId, + Register dest) const; // Returns |val|'s JSValueType or JSVAL_TYPE_UNKNOWN. JSValueType knownType(ValOperandId val) const; diff --git a/js/src/jit/CacheIROps.yaml b/js/src/jit/CacheIROps.yaml index c835fe47b24d..636a7a7fc3f5 100644 --- a/js/src/jit/CacheIROps.yaml +++ b/js/src/jit/CacheIROps.yaml @@ -1294,6 +1294,17 @@ elementType: ScalarTypeImm allowDoubleForUint32: BoolImm +- name: StoreDataViewValueResult + shared: true + transpile: false + cost_estimate: 4 + args: + obj: ObjId + offset: Int32Id + value: RawId + littleEndian: Int32Id + elementType: ScalarTypeImm + - name: LoadInt32ArrayLengthResult shared: true transpile: true