diff --git a/js/src/jit/CacheIRCompiler.cpp b/js/src/jit/CacheIRCompiler.cpp index 538da8b0fa65..21b851a71e03 100644 --- a/js/src/jit/CacheIRCompiler.cpp +++ b/js/src/jit/CacheIRCompiler.cpp @@ -2206,8 +2206,6 @@ bool CacheIRCompiler::emitDoubleModResult() { bool CacheIRCompiler::emitInt32AddResult() { JitSpew(JitSpew_Codegen, __FUNCTION__); AutoOutputRegister output(*this); - AutoScratchRegisterMaybeOutput scratch(allocator, masm, output); - Register lhs = allocator.useRegister(masm, reader.int32OperandId()); Register rhs = allocator.useRegister(masm, reader.int32OperandId()); @@ -2216,16 +2214,14 @@ bool CacheIRCompiler::emitInt32AddResult() { return false; } - masm.mov(rhs, scratch); - masm.branchAdd32(Assembler::Overflow, lhs, scratch, failure->label()); - EmitStoreResult(masm, scratch, JSVAL_TYPE_INT32, output); + masm.branchAdd32(Assembler::Overflow, lhs, rhs, failure->label()); + EmitStoreResult(masm, rhs, JSVAL_TYPE_INT32, output); return true; } bool CacheIRCompiler::emitInt32SubResult() { JitSpew(JitSpew_Codegen, __FUNCTION__); AutoOutputRegister output(*this); - AutoScratchRegisterMaybeOutput scratch(allocator, masm, output); Register lhs = allocator.useRegister(masm, reader.int32OperandId()); Register rhs = allocator.useRegister(masm, reader.int32OperandId()); @@ -2234,9 +2230,8 @@ bool CacheIRCompiler::emitInt32SubResult() { return false; } - masm.mov(lhs, scratch); - masm.branchSub32(Assembler::Overflow, rhs, scratch, failure->label()); - EmitStoreResult(masm, scratch, JSVAL_TYPE_INT32, output); + masm.branchSub32(Assembler::Overflow, rhs, lhs, failure->label()); + EmitStoreResult(masm, lhs, JSVAL_TYPE_INT32, output); return true; } @@ -2246,8 +2241,7 @@ bool CacheIRCompiler::emitInt32MulResult() { AutoOutputRegister output(*this); Register lhs = allocator.useRegister(masm, reader.int32OperandId()); Register rhs = allocator.useRegister(masm, reader.int32OperandId()); - AutoScratchRegister scratch(allocator, masm); - AutoScratchRegisterMaybeOutput scratch2(allocator, masm, output); + AutoScratchRegisterMaybeOutput scratch(allocator, masm, output); FailurePath* failure; if (!addFailurePath(&failure)) { @@ -2256,18 +2250,17 @@ bool CacheIRCompiler::emitInt32MulResult() { Label maybeNegZero, done; masm.mov(lhs, scratch); - masm.branchMul32(Assembler::Overflow, rhs, scratch, failure->label()); - masm.branchTest32(Assembler::Zero, scratch, scratch, &maybeNegZero); + masm.branchMul32(Assembler::Overflow, rhs, lhs, failure->label()); + masm.branchTest32(Assembler::Zero, lhs, lhs, &maybeNegZero); masm.jump(&done); masm.bind(&maybeNegZero); - masm.mov(lhs, scratch2); // Result is -0 if exactly one of lhs or rhs is negative. - masm.or32(rhs, scratch2); - masm.branchTest32(Assembler::Signed, scratch2, scratch2, failure->label()); + masm.or32(rhs, scratch); + masm.branchTest32(Assembler::Signed, scratch, scratch, failure->label()); masm.bind(&done); - EmitStoreResult(masm, scratch, JSVAL_TYPE_INT32, output); + EmitStoreResult(masm, lhs, JSVAL_TYPE_INT32, output); return true; } @@ -2277,7 +2270,6 @@ bool CacheIRCompiler::emitInt32DivResult() { Register lhs = allocator.useRegister(masm, reader.int32OperandId()); Register rhs = allocator.useRegister(masm, reader.int32OperandId()); AutoScratchRegister rem(allocator, masm); - AutoScratchRegisterMaybeOutput scratch(allocator, masm, output); FailurePath* failure; if (!addFailurePath(&failure)) { @@ -2295,14 +2287,13 @@ bool CacheIRCompiler::emitInt32DivResult() { masm.branchTest32(Assembler::Signed, rhs, rhs, failure->label()); masm.bind(¬Zero); - masm.mov(lhs, scratch); LiveRegisterSet volatileRegs(GeneralRegisterSet::Volatile(), liveVolatileFloatRegs()); - masm.flexibleDivMod32(rhs, scratch, rem, false, volatileRegs); + masm.flexibleDivMod32(rhs, lhs, rem, false, volatileRegs); // A remainder implies a double result. masm.branchTest32(Assembler::NonZero, rem, rem, failure->label()); - EmitStoreResult(masm, scratch, JSVAL_TYPE_INT32, output); + EmitStoreResult(masm, lhs, JSVAL_TYPE_INT32, output); return true; } @@ -2311,7 +2302,6 @@ bool CacheIRCompiler::emitInt32ModResult() { AutoOutputRegister output(*this); Register lhs = allocator.useRegister(masm, reader.int32OperandId()); Register rhs = allocator.useRegister(masm, reader.int32OperandId()); - AutoScratchRegisterMaybeOutput scratch(allocator, masm, output); FailurePath* failure; if (!addFailurePath(&failure)) { @@ -2330,12 +2320,11 @@ bool CacheIRCompiler::emitInt32ModResult() { // Prevent negative 0 and -2147483648 / -1. masm.branch32(Assembler::Equal, lhs, Imm32(INT32_MIN), failure->label()); - masm.mov(lhs, scratch); LiveRegisterSet volatileRegs(GeneralRegisterSet::Volatile(), liveVolatileFloatRegs()); - masm.flexibleRemainder32(rhs, scratch, false, volatileRegs); + masm.flexibleRemainder32(rhs, lhs, false, volatileRegs); - EmitStoreResult(masm, scratch, JSVAL_TYPE_INT32, output); + EmitStoreResult(masm, lhs, JSVAL_TYPE_INT32, output); return true; } @@ -2343,42 +2332,36 @@ bool CacheIRCompiler::emitInt32ModResult() { bool CacheIRCompiler::emitInt32BitOrResult() { JitSpew(JitSpew_Codegen, __FUNCTION__); AutoOutputRegister output(*this); - AutoScratchRegisterMaybeOutput scratch(allocator, masm, output); Register lhs = allocator.useRegister(masm, reader.int32OperandId()); Register rhs = allocator.useRegister(masm, reader.int32OperandId()); - masm.mov(rhs, scratch); - masm.or32(lhs, scratch); - EmitStoreResult(masm, scratch, JSVAL_TYPE_INT32, output); + masm.or32(lhs, rhs); + EmitStoreResult(masm, rhs, JSVAL_TYPE_INT32, output); return true; } bool CacheIRCompiler::emitInt32BitXorResult() { JitSpew(JitSpew_Codegen, __FUNCTION__); AutoOutputRegister output(*this); - AutoScratchRegisterMaybeOutput scratch(allocator, masm, output); Register lhs = allocator.useRegister(masm, reader.int32OperandId()); Register rhs = allocator.useRegister(masm, reader.int32OperandId()); - masm.mov(rhs, scratch); - masm.xor32(lhs, scratch); - EmitStoreResult(masm, scratch, JSVAL_TYPE_INT32, output); + masm.xor32(lhs, rhs); + EmitStoreResult(masm, rhs, JSVAL_TYPE_INT32, output); return true; } bool CacheIRCompiler::emitInt32BitAndResult() { JitSpew(JitSpew_Codegen, __FUNCTION__); AutoOutputRegister output(*this); - AutoScratchRegisterMaybeOutput scratch(allocator, masm, output); Register lhs = allocator.useRegister(masm, reader.int32OperandId()); Register rhs = allocator.useRegister(masm, reader.int32OperandId()); - masm.mov(rhs, scratch); - masm.and32(lhs, scratch); - EmitStoreResult(masm, scratch, JSVAL_TYPE_INT32, output); + masm.and32(lhs, rhs); + EmitStoreResult(masm, rhs, JSVAL_TYPE_INT32, output); return true; } @@ -2387,13 +2370,11 @@ bool CacheIRCompiler::emitInt32LeftShiftResult() { AutoOutputRegister output(*this); Register lhs = allocator.useRegister(masm, reader.int32OperandId()); Register rhs = allocator.useRegister(masm, reader.int32OperandId()); - AutoScratchRegisterMaybeOutput scratch(allocator, masm, output); - masm.mov(lhs, scratch); // Mask shift amount as specified by 12.9.3.1 Step 7 masm.and32(Imm32(0x1F), rhs); - masm.flexibleLshift32(rhs, scratch); - EmitStoreResult(masm, scratch, JSVAL_TYPE_INT32, output); + masm.flexibleLshift32(rhs, lhs); + EmitStoreResult(masm, lhs, JSVAL_TYPE_INT32, output); return true; } @@ -2403,13 +2384,11 @@ bool CacheIRCompiler::emitInt32RightShiftResult() { AutoOutputRegister output(*this); Register lhs = allocator.useRegister(masm, reader.int32OperandId()); Register rhs = allocator.useRegister(masm, reader.int32OperandId()); - AutoScratchRegisterMaybeOutput scratch(allocator, masm, output); - masm.mov(lhs, scratch); // Mask shift amount as specified by 12.9.4.1 Step 7 masm.and32(Imm32(0x1F), rhs); - masm.flexibleRshift32Arithmetic(rhs, scratch); - EmitStoreResult(masm, scratch, JSVAL_TYPE_INT32, output); + masm.flexibleRshift32Arithmetic(rhs, lhs); + EmitStoreResult(masm, lhs, JSVAL_TYPE_INT32, output); return true; } @@ -2421,33 +2400,31 @@ bool CacheIRCompiler::emitInt32URightShiftResult() { Register lhs = allocator.useRegister(masm, reader.int32OperandId()); Register rhs = allocator.useRegister(masm, reader.int32OperandId()); bool allowDouble = reader.readBool(); - AutoScratchRegisterMaybeOutput scratch(allocator, masm, output); FailurePath* failure; if (!addFailurePath(&failure)) { return false; } - masm.mov(lhs, scratch); // Mask shift amount as specified by 12.9.4.1 Step 7 masm.and32(Imm32(0x1F), rhs); - masm.flexibleRshift32(rhs, scratch); + masm.flexibleRshift32(rhs, lhs); Label intDone, floatDone; if (allowDouble) { Label toUint; - masm.branchTest32(Assembler::Signed, scratch, scratch, &toUint); + masm.branchTest32(Assembler::Signed, lhs, lhs, &toUint); masm.jump(&intDone); masm.bind(&toUint); ScratchDoubleScope fpscratch(masm); - masm.convertUInt32ToDouble(scratch, fpscratch); + masm.convertUInt32ToDouble(lhs, fpscratch); masm.boxDouble(fpscratch, output.valueReg(), fpscratch); masm.jump(&floatDone); } else { - masm.branchTest32(Assembler::Signed, scratch, scratch, failure->label()); + masm.branchTest32(Assembler::Signed, lhs, lhs, failure->label()); } masm.bind(&intDone); - EmitStoreResult(masm, scratch, JSVAL_TYPE_INT32, output); + EmitStoreResult(masm, lhs, JSVAL_TYPE_INT32, output); masm.bind(&floatDone); return true; } @@ -2456,7 +2433,6 @@ bool CacheIRCompiler::emitInt32NegationResult() { JitSpew(JitSpew_Codegen, __FUNCTION__); AutoOutputRegister output(*this); Register val = allocator.useRegister(masm, reader.int32OperandId()); - AutoScratchRegisterMaybeOutput scratch(allocator, masm, output); FailurePath* failure; if (!addFailurePath(&failure)) { @@ -2466,9 +2442,8 @@ bool CacheIRCompiler::emitInt32NegationResult() { // Guard against 0 and MIN_INT by checking if low 31-bits are all zero. // Both of these result in a double. masm.branchTest32(Assembler::Zero, val, Imm32(0x7fffffff), failure->label()); - masm.mov(val, scratch); - masm.neg32(scratch); - masm.tagValue(JSVAL_TYPE_INT32, scratch, output.valueReg()); + masm.neg32(val); + masm.tagValue(JSVAL_TYPE_INT32, val, output.valueReg()); return true; } @@ -2510,11 +2485,8 @@ bool CacheIRCompiler::emitInt32NotResult() { JitSpew(JitSpew_Codegen, __FUNCTION__); AutoOutputRegister output(*this); Register val = allocator.useRegister(masm, reader.int32OperandId()); - AutoScratchRegisterMaybeOutput scratch(allocator, masm, output); - - masm.mov(val, scratch); - masm.not32(scratch); - masm.tagValue(JSVAL_TYPE_INT32, scratch, output.valueReg()); + masm.not32(val); + masm.tagValue(JSVAL_TYPE_INT32, val, output.valueReg()); return true; } diff --git a/js/src/jit/CacheIRCompiler.h b/js/src/jit/CacheIRCompiler.h index 309c51383d5c..f7bdf25caf59 100644 --- a/js/src/jit/CacheIRCompiler.h +++ b/js/src/jit/CacheIRCompiler.h @@ -151,9 +151,6 @@ namespace jit { // for access to a particular OperandId, and the register allocator will // generate the required code to fill that request. // -// Input OperandIds should be considered as immutable, and should not be mutated -// during the execution of a stub. -// // There are also a number of RAII classes that interact with the register // allocator, in order to provide access to more registers than just those // provided for by the OperandIds. diff --git a/js/src/jit/x86-shared/MacroAssembler-x86-shared-inl.h b/js/src/jit/x86-shared/MacroAssembler-x86-shared-inl.h index 489dc0f2231c..51df81f1f787 100644 --- a/js/src/jit/x86-shared/MacroAssembler-x86-shared-inl.h +++ b/js/src/jit/x86-shared/MacroAssembler-x86-shared-inl.h @@ -308,58 +308,28 @@ void MacroAssembler::lshift32(Register shift, Register srcDest) { shll_cl(srcDest); } -// All the shift instructions have the same requirement; the shift amount -// must be in ecx. In order to handle arbitrary input registers, we divide this -// operation into phases: -// -// [PUSH] Preserve any registers which may be clobbered -// [MOVE] Move the shift to ecx and the amount to be shifted to an -// arbitrarily chosen preserved register that is not ecx. -// [SHIFT] Do the shift operation -// [MOVE] Move the result back to the destination -// [POP] Restore the registers which were preserved. -inline void FlexibleShift32(MacroAssembler& masm, Register shift, - Register srcDest, bool left, - bool arithmetic = false) { - // Choose an arbitrary register that's not ecx - Register internalSrcDest = (srcDest != ecx) ? srcDest : ebx; - MOZ_ASSERT(internalSrcDest != ecx); - - // Add registers we may clobber and want to ensure are restored as live, and - // remove what we definitely clobber (the destination) - LiveRegisterSet preserve; - preserve.add(ecx); - preserve.add(internalSrcDest); - - preserve.takeUnchecked(srcDest); - - // [PUSH] - masm.PushRegsInMask(preserve); - - // [MOVE] - masm.moveRegPair(shift, srcDest, ecx, internalSrcDest); - if (masm.oom()) { - return; +inline void FlexibleShift32(MacroAssembler& masm, Register shift, Register dest, + bool left, bool arithmetic = false) { + if (shift != ecx) { + if (dest != ecx) { + masm.push(ecx); + } + masm.mov(shift, ecx); } - // [SHIFT] if (left) { - masm.lshift32(ecx, internalSrcDest); + masm.lshift32(ecx, dest); } else { if (arithmetic) { - masm.rshift32Arithmetic(ecx, internalSrcDest); + masm.rshift32Arithmetic(ecx, dest); } else { - masm.rshift32(ecx, internalSrcDest); + masm.rshift32(ecx, dest); } } - // [MOVE] - if (internalSrcDest != srcDest) { - masm.mov(internalSrcDest, srcDest); + if (shift != ecx && dest != ecx) { + masm.pop(ecx); } - - // [POP] - masm.PopRegsInMask(preserve); } void MacroAssembler::flexibleLshift32(Register shift, Register srcDest) { diff --git a/js/src/jsapi-tests/testJitMacroAssembler.cpp b/js/src/jsapi-tests/testJitMacroAssembler.cpp index 6a3e1589bf17..da28ac699895 100644 --- a/js/src/jsapi-tests/testJitMacroAssembler.cpp +++ b/js/src/jsapi-tests/testJitMacroAssembler.cpp @@ -215,190 +215,6 @@ BEGIN_TEST(testJitMacroAssembler_flexibleQuotient) { } END_TEST(testJitMacroAssembler_flexibleQuotient) -// To make sure ecx isn't being clobbered; globally scoped to ensure it has the -// right lifetime. -const uintptr_t guardEcx = 0xfeedbad; - -void shiftTest(StackMacroAssembler& masm, const char* name, - void (*operation)(StackMacroAssembler& masm, Register, Register), - const uintptr_t* lhsInput, const uintptr_t* rhsInput, - const uintptr_t* result) { - AllocatableGeneralRegisterSet leftOutputHandSides(GeneralRegisterSet::All()); - - while (!leftOutputHandSides.empty()) { - Register lhsOutput = leftOutputHandSides.takeAny(); - - AllocatableGeneralRegisterSet rightHandSides(GeneralRegisterSet::All()); - while (!rightHandSides.empty()) { - Register rhs = rightHandSides.takeAny(); - - // You can only use shift as the same reg if the values are the same - if (lhsOutput == rhs && lhsInput != rhsInput) { - continue; - } - - Label next, outputFail, clobberRhs, clobberEcx, dump; - masm.mov(ImmWord(guardEcx), ecx); - masm.mov(ImmWord(*lhsInput), lhsOutput); - masm.mov(ImmWord(*rhsInput), rhs); - - operation(masm, rhs, lhsOutput); - - // Ensure Result is correct - masm.branch32(Assembler::NotEqual, AbsoluteAddress(result), lhsOutput, - &outputFail); - - // Ensure RHS was not clobbered - masm.branch32(Assembler::NotEqual, AbsoluteAddress(rhsInput), rhs, - &clobberRhs); - - if (lhsOutput != ecx && rhs != ecx) { - // If neither lhsOutput nor rhs is ecx, make sure ecx has been - // preserved, otherwise it's expected to be covered by the RHS clobber - // check above, or intentionally clobbered as the output. - masm.branch32(Assembler::NotEqual, AbsoluteAddress(&guardEcx), ecx, - &clobberEcx); - } - - masm.jump(&next); - - masm.bind(&outputFail); - masm.printf("Incorrect output (got %d) ", lhsOutput); - masm.jump(&dump); - - masm.bind(&clobberRhs); - masm.printf("rhs clobbered %d", rhs); - masm.jump(&dump); - - masm.bind(&clobberEcx); - masm.printf("ecx clobbered"); - masm.jump(&dump); - - masm.bind(&dump); - masm.mov(ImmPtr(lhsOutput.name()), lhsOutput); - masm.printf("(lhsOutput/srcDest) %s ", lhsOutput); - masm.mov(ImmPtr(name), lhsOutput); - masm.printf("%s ", lhsOutput); - masm.mov(ImmPtr(rhs.name()), lhsOutput); - masm.printf("(shift/rhs) %s \n", lhsOutput); - // Breakpoint to force test failure. - masm.breakpoint(); - masm.bind(&next); - } - } -} - -BEGIN_TEST(testJitMacroAssembler_flexibleRshift) { - StackMacroAssembler masm(cx); - - if (!Prepare(masm)) { - return false; - } - - { - // Test case 16 >> 2 == 4; - const uintptr_t lhsInput = 16; - const uintptr_t rhsInput = 2; - const uintptr_t result = 4; - - shiftTest(masm, "flexibleRshift32", - [](StackMacroAssembler& masm, Register rhs, Register lhsOutput) { - masm.flexibleRshift32(rhs, lhsOutput); - }, - &lhsInput, &rhsInput, &result); - } - - { - // Test case 16 >> 16 == 0 -- this helps cover the case where the same - // register can be passed for source and dest. - const uintptr_t lhsInput = 16; - const uintptr_t rhsInput = 16; - const uintptr_t result = 0; - - shiftTest(masm, "flexibleRshift32", - [](StackMacroAssembler& masm, Register rhs, Register lhsOutput) { - masm.flexibleRshift32(rhs, lhsOutput); - }, - &lhsInput, &rhsInput, &result); - } - - return Execute(cx, masm); -} -END_TEST(testJitMacroAssembler_flexibleRshift) - -BEGIN_TEST(testJitMacroAssembler_flexibleRshiftArithmetic) { - StackMacroAssembler masm(cx); - - if (!Prepare(masm)) { - return false; - } - - { - // Test case 4294967295 >> 2 == 4294967295; - const uintptr_t lhsInput = 4294967295; - const uintptr_t rhsInput = 2; - const uintptr_t result = 4294967295; - shiftTest(masm, "flexibleRshift32Arithmetic", - [](StackMacroAssembler& masm, Register rhs, Register lhsOutput) { - masm.flexibleRshift32Arithmetic(rhs, lhsOutput); - }, - &lhsInput, &rhsInput, &result); - } - - { - // Test case 16 >> 16 == 0 -- this helps cover the case where the same - // register can be passed for source and dest. - const uintptr_t lhsInput = 16; - const uintptr_t rhsInput = 16; - const uintptr_t result = 0; - - shiftTest(masm, "flexibleRshift32Arithmetic", - [](StackMacroAssembler& masm, Register rhs, Register lhsOutput) { - masm.flexibleRshift32Arithmetic(rhs, lhsOutput); - }, - &lhsInput, &rhsInput, &result); - } - - return Execute(cx, masm); -} -END_TEST(testJitMacroAssembler_flexibleRshiftArithmetic) - -BEGIN_TEST(testJitMacroAssembler_flexibleLshift) { - StackMacroAssembler masm(cx); - - if (!Prepare(masm)) { - return false; - } - { - // Test case 16 << 2 == 64; - const uintptr_t lhsInput = 16; - const uintptr_t rhsInput = 2; - const uintptr_t result = 64; - - shiftTest(masm, "flexibleLshift32", - [](StackMacroAssembler& masm, Register rhs, Register lhsOutput) { - masm.flexibleLshift32(rhs, lhsOutput); - }, - &lhsInput, &rhsInput, &result); - } - - { - // Test case 4 << 4 == 64; duplicated input case - const uintptr_t lhsInput = 4; - const uintptr_t rhsInput = 4; - const uintptr_t result = 64; - - shiftTest(masm, "flexibleLshift32", - [](StackMacroAssembler& masm, Register rhs, Register lhsOutput) { - masm.flexibleLshift32(rhs, lhsOutput); - }, - &lhsInput, &rhsInput, &result); - } - - return Execute(cx, masm); -} -END_TEST(testJitMacroAssembler_flexibleLshift) - BEGIN_TEST(testJitMacroAssembler_truncateDoubleToInt64) { StackMacroAssembler masm(cx);