Backed out 2 changesets (bug 1519077, bug 1526024) for jsapi-tests failures on a CLOSED TREE

Backed out changeset 8a75915b2541 (bug 1519077)
Backed out changeset 87514c3aaa3a (bug 1526024)

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, rhs, failure->label());
-  masm.branchAdd32(Assembler::Overflow, lhs, scratch, failure->label());
+  EmitStoreResult(masm, rhs, JSVAL_TYPE_INT32, output);
-  EmitStoreResult(masm, scratch, 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, lhs, failure->label());
-  masm.branchSub32(Assembler::Overflow, rhs, scratch, failure->label());
+  EmitStoreResult(masm, lhs, JSVAL_TYPE_INT32, output);
-  EmitStoreResult(masm, scratch, 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 scratch(allocator, masm, output);
-  AutoScratchRegisterMaybeOutput scratch2(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.branchMul32(Assembler::Overflow, rhs, lhs, failure->label());
-  masm.branchTest32(Assembler::Zero, scratch, scratch, &maybeNegZero);
+  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.or32(rhs, scratch);
-  masm.branchTest32(Assembler::Signed, scratch2, scratch2, failure->label());
+  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(&notZero);
 
-  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, rhs);
-  masm.or32(lhs, scratch);
+  EmitStoreResult(masm, rhs, JSVAL_TYPE_INT32, output);
-  EmitStoreResult(masm, scratch, 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, rhs);
-  masm.xor32(lhs, scratch);
+  EmitStoreResult(masm, rhs, JSVAL_TYPE_INT32, output);
-  EmitStoreResult(masm, scratch, 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, rhs);
-  masm.and32(lhs, scratch);
+  EmitStoreResult(masm, rhs, JSVAL_TYPE_INT32, output);
-  EmitStoreResult(masm, scratch, 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);
+  masm.flexibleLshift32(rhs, lhs);
-  EmitStoreResult(masm, scratch, JSVAL_TYPE_INT32, output);
+  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);
+  masm.flexibleRshift32Arithmetic(rhs, lhs);
-  EmitStoreResult(masm, scratch, JSVAL_TYPE_INT32, output);
+  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(val);
-  masm.neg32(scratch);
+  masm.tagValue(JSVAL_TYPE_INT32, val, output.valueReg());
-  masm.tagValue(JSVAL_TYPE_INT32, scratch, 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.not32(val);
-
+  masm.tagValue(JSVAL_TYPE_INT32, val, output.valueReg());
-  masm.mov(val, scratch);
-  masm.not32(scratch);
-  masm.tagValue(JSVAL_TYPE_INT32, scratch, output.valueReg());
   return true;
 }
 

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.

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
+inline void FlexibleShift32(MacroAssembler& masm, Register shift, Register dest,
-// must be in ecx. In order to handle arbitrary input registers, we divide this
+                            bool left, bool arithmetic = false) {
-// operation into phases:
+  if (shift != ecx) {
-//
+    if (dest != ecx) {
-// [PUSH]     Preserve any registers which may be clobbered
+      masm.push(ecx);
-// [MOVE]     Move the shift to ecx and the amount to be shifted to an
+    }
-//            arbitrarily chosen preserved register that is not ecx.
+    masm.mov(shift, 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;
   }
 
-  // [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 (shift != ecx && dest != ecx) {
-  if (internalSrcDest != srcDest) {
+    masm.pop(ecx);
-    masm.mov(internalSrcDest, srcDest);
   }
-
-  // [POP]
-  masm.PopRegsInMask(preserve);
 }
 
 void MacroAssembler::flexibleLshift32(Register shift, Register srcDest) {

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);