From c258dcd9fb6de2a34a33b5307936dafaac55f5ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Bargull?= Date: Wed, 12 Aug 2020 18:18:54 +0000 Subject: [PATCH] Bug 1658268: Optimise Boolean in CacheIR and Warp. r=jandem The CacheIR implementation uses the new `LoadValueTruthyResult` op instead of the existing `LoadThingTruthyResult` ops to mirror how Warp implements boolean coercion: Warp doesn't use ICs but instead uses MNot resp. MTest with Value typed inputs. Mirroring this approach avoids to have (from the user POV) strange performance differences where for example using `if (Boolean(v))` can be faster than just `if (v)`, when `v` is Value-typed, but monomorphic. Differential Revision: https://phabricator.services.mozilla.com/D86524 --- js/src/jit-test/tests/cacheir/boolean-call.js | 67 +++++++++ js/src/jit/CacheIR.cpp | 33 +++++ js/src/jit/CacheIR.h | 1 + js/src/jit/CacheIRCompiler.cpp | 139 +++++++++++++++++- js/src/jit/CacheIROps.yaml | 7 + js/src/jit/WarpCacheIRTranspiler.cpp | 13 ++ 6 files changed, 254 insertions(+), 6 deletions(-) create mode 100644 js/src/jit-test/tests/cacheir/boolean-call.js diff --git a/js/src/jit-test/tests/cacheir/boolean-call.js b/js/src/jit-test/tests/cacheir/boolean-call.js new file mode 100644 index 000000000000..7c6d94f3d421 --- /dev/null +++ b/js/src/jit-test/tests/cacheir/boolean-call.js @@ -0,0 +1,67 @@ +function wrapper(values) { + function test(values) { + var expected = values.map(v => !!v); + + for (var i = 0; i < 100; ++i) { + var ix = i % values.length; + var val = values[ix]; + var actual = Boolean(val); + assertEq(actual, expected[ix]); + } + } + + for (var i = 0; i < 2; ++i) { + test(values); + } +} + +function makeTest() { + // Create a copy to avoid type pollution. + return Function(`return ${wrapper}`)(); +} + +// Use a new compartment to create a wrapper. +var g = newGlobal({newCompartment: true}); + +var testValues = { + boolean: [true, false], + int32: [-2147483648, -1, 0, 1, 2147483647], + double: [-Infinity, -1.5, -1, -0.5, -0, +0, +0.5, +1, +1.5, Infinity, NaN], + string: ["", "true", "false", "0", "1", "hello"], + symbol: [Symbol(), Symbol("desc"), Symbol.iterator], + bigint: [ + -(2n ** 1000n), + -18446744073709551616n, // -(2n**64n) + -9223372036854775808n, // -(2n**63n) + -4294967296n, // -(2**32) + -2147483648n, // -(2**31) + -1n, 0n, 1n, + 2147483648n, // 2**31 + 4294967296n, // 2**32 + 9223372036854775808n, // 2n**63n + 18446744073709551616n, // 2n**64n + 2n ** 1000n, + ], + object: [{}, [], function(){}, new Proxy({}, {}), createIsHTMLDDA(), g.eval("createIsHTMLDDA()")], + null: [null], + undefined: [undefined], +}; + +for (var values of Object.values(testValues)) { + makeTest()(values); +} + +// boolean and int32 +makeTest()([].concat(testValues.boolean, testValues.int32)); + +// int32 and double +makeTest()([].concat(testValues.int32, testValues.double)); + +// null and undefined +makeTest()([].concat(testValues.null, testValues.undefined)); + +// null, undefined, and object +makeTest()([].concat(testValues.null, testValues.undefined, testValues.object)); + +// all values +makeTest()(Object.values(testValues).flat()); diff --git a/js/src/jit/CacheIR.cpp b/js/src/jit/CacheIR.cpp index e32535a93389..e9b56a15551b 100644 --- a/js/src/jit/CacheIR.cpp +++ b/js/src/jit/CacheIR.cpp @@ -7392,6 +7392,35 @@ AttachDecision CallIRGenerator::tryAttachAtomicsIsLockFree( return AttachDecision::Attach; } +AttachDecision CallIRGenerator::tryAttachBoolean(HandleFunction callee) { + // Need zero or one argument. + if (argc_ > 1) { + return AttachDecision::NoAction; + } + + // Initialize the input operand. + Int32OperandId argcId(writer.setInputOperandId(0)); + + // Guard callee is the 'Boolean' native function. + emitNativeCalleeGuard(callee); + + if (argc_ == 0) { + writer.loadBooleanResult(false); + } else { + ValOperandId valId = + writer.loadArgumentFixedSlot(ArgumentKind::Arg0, argc_); + + writer.loadValueTruthyResult(valId); + } + + // This stub doesn't need to be monitored, because it always returns a bool. + writer.returnFromIC(); + cacheIRStubKind_ = BaselineCacheIRStubKind::Regular; + + trackAttached("Boolean"); + return AttachDecision::Attach; +} + AttachDecision CallIRGenerator::tryAttachFunCall(HandleFunction callee) { MOZ_ASSERT(callee->isNativeWithoutJitEntry()); if (callee->native() != fun_call) { @@ -8494,6 +8523,10 @@ AttachDecision CallIRGenerator::tryAttachInlinableNative( case InlinableNative::AtomicsIsLockFree: return tryAttachAtomicsIsLockFree(callee); + // Boolean natives. + case InlinableNative::Boolean: + return tryAttachBoolean(callee); + default: return AttachDecision::NoAction; } diff --git a/js/src/jit/CacheIR.h b/js/src/jit/CacheIR.h index 0333bc195466..335102085c3e 100644 --- a/js/src/jit/CacheIR.h +++ b/js/src/jit/CacheIR.h @@ -1722,6 +1722,7 @@ class MOZ_RAII CallIRGenerator : public IRGenerator { AttachDecision tryAttachAtomicsLoad(HandleFunction callee); AttachDecision tryAttachAtomicsStore(HandleFunction callee); AttachDecision tryAttachAtomicsIsLockFree(HandleFunction callee); + AttachDecision tryAttachBoolean(HandleFunction callee); AttachDecision tryAttachFunCall(HandleFunction calleeFunc); AttachDecision tryAttachFunApply(HandleFunction calleeFunc); diff --git a/js/src/jit/CacheIRCompiler.cpp b/js/src/jit/CacheIRCompiler.cpp index 4ba8a94ffcb4..725240d83d57 100644 --- a/js/src/jit/CacheIRCompiler.cpp +++ b/js/src/jit/CacheIRCompiler.cpp @@ -5756,12 +5756,23 @@ bool CacheIRCompiler::emitLoadObjectTruthyResult(ObjOperandId objId) { masm.jump(&done); masm.bind(&slowPath); - masm.setupUnalignedABICall(scratch); - masm.passABIArg(obj); - masm.callWithABI(JS_FUNC_TO_DATA_PTR(void*, js::EmulatesUndefined)); - masm.convertBoolToInt32(ReturnReg, ReturnReg); - masm.xor32(Imm32(1), ReturnReg); - masm.tagValue(JSVAL_TYPE_BOOLEAN, ReturnReg, output.valueReg()); + { + LiveRegisterSet volatileRegs(GeneralRegisterSet::Volatile(), + liveVolatileFloatRegs()); + volatileRegs.takeUnchecked(scratch); + volatileRegs.takeUnchecked(output); + masm.PushRegsInMask(volatileRegs); + + masm.setupUnalignedABICall(scratch); + masm.passABIArg(obj); + masm.callWithABI(JS_FUNC_TO_DATA_PTR(void*, js::EmulatesUndefined)); + masm.convertBoolToInt32(ReturnReg, scratch); + masm.xor32(Imm32(1), scratch); + + masm.PopRegsInMask(volatileRegs); + + masm.tagValue(JSVAL_TYPE_BOOLEAN, scratch, output.valueReg()); + } masm.bind(&done); return true; @@ -5786,6 +5797,122 @@ bool CacheIRCompiler::emitLoadBigIntTruthyResult(BigIntOperandId bigIntId) { return true; } +bool CacheIRCompiler::emitLoadValueTruthyResult(ValOperandId inputId) { + JitSpew(JitSpew_Codegen, "%s", __FUNCTION__); + + AutoOutputRegister output(*this); + ValueOperand value = allocator.useValueRegister(masm, inputId); + AutoScratchRegisterMaybeOutput scratch1(allocator, masm, output); + AutoScratchRegister scratch2(allocator, masm); + AutoScratchFloatRegister floatReg(this); + + Label ifFalse, ifTrue, done; + + { + ScratchTagScope tag(masm, value); + masm.splitTagForTest(value, tag); + + masm.branchTestUndefined(Assembler::Equal, tag, &ifFalse); + masm.branchTestNull(Assembler::Equal, tag, &ifFalse); + + Label notBoolean; + masm.branchTestBoolean(Assembler::NotEqual, tag, ¬Boolean); + { + ScratchTagScopeRelease _(&tag); + masm.branchTestBooleanTruthy(false, value, &ifFalse); + masm.jump(&ifTrue); + } + masm.bind(¬Boolean); + + Label notInt32; + masm.branchTestInt32(Assembler::NotEqual, tag, ¬Int32); + { + ScratchTagScopeRelease _(&tag); + masm.branchTestInt32Truthy(false, value, &ifFalse); + masm.jump(&ifTrue); + } + masm.bind(¬Int32); + + Label notObject; + masm.branchTestObject(Assembler::NotEqual, tag, ¬Object); + { + ScratchTagScopeRelease _(&tag); + + Register obj = masm.extractObject(value, scratch1); + + Label slowPath; + masm.branchIfObjectEmulatesUndefined(obj, scratch2, &slowPath, &ifFalse); + masm.jump(&ifTrue); + + masm.bind(&slowPath); + { + LiveRegisterSet volatileRegs(GeneralRegisterSet::Volatile(), + liveVolatileFloatRegs()); + volatileRegs.takeUnchecked(scratch1); + volatileRegs.takeUnchecked(scratch2); + volatileRegs.takeUnchecked(output); + masm.PushRegsInMask(volatileRegs); + + masm.setupUnalignedABICall(scratch2); + masm.passABIArg(obj); + masm.callWithABI(JS_FUNC_TO_DATA_PTR(void*, js::EmulatesUndefined)); + masm.storeCallBoolResult(scratch2); + + masm.PopRegsInMask(volatileRegs); + + masm.branchTest32(Assembler::NonZero, scratch2, scratch2, &ifFalse); + masm.jump(&ifTrue); + } + } + masm.bind(¬Object); + + Label notString; + masm.branchTestString(Assembler::NotEqual, tag, ¬String); + { + ScratchTagScopeRelease _(&tag); + masm.branchTestStringTruthy(false, value, &ifFalse); + masm.jump(&ifTrue); + } + masm.bind(¬String); + + Label notBigInt; + masm.branchTestBigInt(Assembler::NotEqual, tag, ¬BigInt); + { + ScratchTagScopeRelease _(&tag); + masm.branchTestBigIntTruthy(false, value, &ifFalse); + masm.jump(&ifTrue); + } + masm.bind(¬BigInt); + + masm.branchTestSymbol(Assembler::Equal, tag, &ifTrue); + +#ifdef DEBUG + Label isDouble; + masm.branchTestDouble(Assembler::Equal, tag, &isDouble); + masm.assumeUnreachable("Unexpected value type"); + masm.bind(&isDouble); +#endif + + { + ScratchTagScopeRelease _(&tag); + masm.unboxDouble(value, floatReg); + masm.branchTestDoubleTruthy(false, floatReg, &ifFalse); + } + + // Fall through to true case. + } + + masm.bind(&ifTrue); + masm.moveValue(BooleanValue(true), output.valueReg()); + masm.jump(&done); + + masm.bind(&ifFalse); + masm.moveValue(BooleanValue(false), output.valueReg()); + + masm.bind(&done); + return true; +} + bool CacheIRCompiler::emitLoadNewObjectFromTemplateResult( uint32_t templateObjectOffset, uint32_t, uint32_t) { JitSpew(JitSpew_Codegen, "%s", __FUNCTION__); diff --git a/js/src/jit/CacheIROps.yaml b/js/src/jit/CacheIROps.yaml index d9f34c5e36bc..df58bb0f86ab 100644 --- a/js/src/jit/CacheIROps.yaml +++ b/js/src/jit/CacheIROps.yaml @@ -2285,6 +2285,13 @@ args: bigInt: BigIntId +- name: LoadValueTruthyResult + shared: true + transpile: true + cost_estimate: 4 + args: + input: ValId + - name: LoadValueResult shared: false transpile: false diff --git a/js/src/jit/WarpCacheIRTranspiler.cpp b/js/src/jit/WarpCacheIRTranspiler.cpp index 6c3de923d456..4168ff872d01 100644 --- a/js/src/jit/WarpCacheIRTranspiler.cpp +++ b/js/src/jit/WarpCacheIRTranspiler.cpp @@ -2507,6 +2507,19 @@ bool WarpCacheIRTranspiler::emitAtomicsIsLockFreeResult( return true; } +bool WarpCacheIRTranspiler::emitLoadValueTruthyResult(ValOperandId inputId) { + MDefinition* input = getOperand(inputId); + + // Convert to bool with the '!!' idiom. + auto* resultInverted = MNot::New(alloc(), input); + add(resultInverted); + auto* result = MNot::New(alloc(), resultInverted); + add(result); + + pushResult(result); + return true; +} + bool WarpCacheIRTranspiler::emitLoadArgumentSlot(ValOperandId resultId, uint32_t slotIndex) { // Reverse of GetIndexOfArgument specialized to !hasArgumentArray.