Bug 1519077 - Don't mutate inputs in CacheIR operations r=jandem
☠☠ backed out by c332d5df8ce7 ☠ ☠
authorMatthew Gaudet <mgaudet@mozilla.com>
Thu, 07 Feb 2019 22:32:00 +0000
changeset 519129 8a75915b2541f5ad01212ec087ea067bf50a1ce0
parent 519128 87514c3aaa3a9bd26879cc7fdf5e3c96de948854
child 519130 cabd5a85dc0fe6a4a40fb93025801b34a01e0262
push id10862
push userffxbld-merge
push dateMon, 11 Mar 2019 13:01:11 +0000
treeherdermozilla-beta@a2e7f5c935da [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjandem
bugs1519077
milestone67.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1519077 - Don't mutate inputs in CacheIR operations r=jandem Differential Revision: https://phabricator.services.mozilla.com/D18708
js/src/jit/CacheIRCompiler.cpp
js/src/jit/CacheIRCompiler.h
--- a/js/src/jit/CacheIRCompiler.cpp
+++ b/js/src/jit/CacheIRCompiler.cpp
@@ -2201,80 +2201,88 @@ bool CacheIRCompiler::emitDoubleModResul
   masm.boxDouble(FloatReg0, output.valueReg(), FloatReg0);
 
   return true;
 }
 
 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());
 
   FailurePath* failure;
   if (!addFailurePath(&failure)) {
     return false;
   }
 
-  masm.branchAdd32(Assembler::Overflow, lhs, rhs, failure->label());
-  EmitStoreResult(masm, rhs, JSVAL_TYPE_INT32, output);
+  masm.mov(rhs, scratch);
+  masm.branchAdd32(Assembler::Overflow, lhs, scratch, failure->label());
+  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());
 
   FailurePath* failure;
   if (!addFailurePath(&failure)) {
     return false;
   }
 
-  masm.branchSub32(Assembler::Overflow, rhs, lhs, failure->label());
-  EmitStoreResult(masm, lhs, JSVAL_TYPE_INT32, output);
+  masm.mov(lhs, scratch);
+  masm.branchSub32(Assembler::Overflow, rhs, scratch, failure->label());
+  EmitStoreResult(masm, scratch, JSVAL_TYPE_INT32, output);
 
   return true;
 }
 
 bool CacheIRCompiler::emitInt32MulResult() {
   JitSpew(JitSpew_Codegen, __FUNCTION__);
   AutoOutputRegister output(*this);
   Register lhs = allocator.useRegister(masm, reader.int32OperandId());
   Register rhs = allocator.useRegister(masm, reader.int32OperandId());
-  AutoScratchRegisterMaybeOutput scratch(allocator, masm, output);
+  AutoScratchRegister scratch(allocator, masm);
+  AutoScratchRegisterMaybeOutput scratch2(allocator, masm, output);
 
   FailurePath* failure;
   if (!addFailurePath(&failure)) {
     return false;
   }
 
   Label maybeNegZero, done;
   masm.mov(lhs, scratch);
-  masm.branchMul32(Assembler::Overflow, rhs, lhs, failure->label());
-  masm.branchTest32(Assembler::Zero, lhs, lhs, &maybeNegZero);
+  masm.branchMul32(Assembler::Overflow, rhs, scratch, failure->label());
+  masm.branchTest32(Assembler::Zero, scratch, scratch, &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, scratch);
-  masm.branchTest32(Assembler::Signed, scratch, scratch, failure->label());
+  masm.or32(rhs, scratch2);
+  masm.branchTest32(Assembler::Signed, scratch2, scratch2, failure->label());
 
   masm.bind(&done);
-  EmitStoreResult(masm, lhs, JSVAL_TYPE_INT32, output);
+  EmitStoreResult(masm, scratch, JSVAL_TYPE_INT32, output);
   return true;
 }
 
 bool CacheIRCompiler::emitInt32DivResult() {
   JitSpew(JitSpew_Codegen, __FUNCTION__);
   AutoOutputRegister output(*this);
   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)) {
     return false;
   }
 
   // Prevent division by 0.
   masm.branchTest32(Assembler::Zero, rhs, rhs, failure->label());
@@ -2282,31 +2290,33 @@ bool CacheIRCompiler::emitInt32DivResult
   // Prevent negative 0 and -2147483648 / -1.
   masm.branch32(Assembler::Equal, lhs, Imm32(INT32_MIN), failure->label());
 
   Label notZero;
   masm.branch32(Assembler::NotEqual, lhs, Imm32(0), &notZero);
   masm.branchTest32(Assembler::Signed, rhs, rhs, failure->label());
   masm.bind(&notZero);
 
+  masm.mov(lhs, scratch);
   LiveRegisterSet volatileRegs(GeneralRegisterSet::Volatile(),
                                liveVolatileFloatRegs());
-  masm.flexibleDivMod32(rhs, lhs, rem, false, volatileRegs);
+  masm.flexibleDivMod32(rhs, scratch, rem, false, volatileRegs);
 
   // A remainder implies a double result.
   masm.branchTest32(Assembler::NonZero, rem, rem, failure->label());
-  EmitStoreResult(masm, lhs, JSVAL_TYPE_INT32, output);
+  EmitStoreResult(masm, scratch, JSVAL_TYPE_INT32, output);
   return true;
 }
 
 bool CacheIRCompiler::emitInt32ModResult() {
   JitSpew(JitSpew_Codegen, __FUNCTION__);
   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)) {
     return false;
   }
 
   // Modulo takes the sign of the dividend; don't handle negative dividends
   // here.
@@ -2315,140 +2325,155 @@ bool CacheIRCompiler::emitInt32ModResult
   // Negative divisor (could be fixed with abs)
   masm.branchTest32(Assembler::Signed, rhs, rhs, failure->label());
 
   // x % 0 results in NaN
   masm.branchTest32(Assembler::Zero, rhs, rhs, failure->label());
 
   // 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, lhs, false, volatileRegs);
-
-  EmitStoreResult(masm, lhs, JSVAL_TYPE_INT32, output);
+  masm.flexibleRemainder32(rhs, scratch, false, volatileRegs);
+
+  EmitStoreResult(masm, scratch, JSVAL_TYPE_INT32, output);
 
   return true;
 }
 
 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.or32(lhs, rhs);
-  EmitStoreResult(masm, rhs, JSVAL_TYPE_INT32, output);
+  masm.mov(rhs, scratch);
+  masm.or32(lhs, scratch);
+  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.xor32(lhs, rhs);
-  EmitStoreResult(masm, rhs, JSVAL_TYPE_INT32, output);
+  masm.mov(rhs, scratch);
+  masm.xor32(lhs, scratch);
+  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.and32(lhs, rhs);
-  EmitStoreResult(masm, rhs, JSVAL_TYPE_INT32, output);
+  masm.mov(rhs, scratch);
+  masm.and32(lhs, scratch);
+  EmitStoreResult(masm, scratch, JSVAL_TYPE_INT32, output);
 
   return true;
 }
 bool CacheIRCompiler::emitInt32LeftShiftResult() {
   JitSpew(JitSpew_Codegen, __FUNCTION__);
   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, lhs);
-  EmitStoreResult(masm, lhs, JSVAL_TYPE_INT32, output);
+  masm.flexibleLshift32(rhs, scratch);
+  EmitStoreResult(masm, scratch, JSVAL_TYPE_INT32, output);
 
   return true;
 }
 
 bool CacheIRCompiler::emitInt32RightShiftResult() {
   JitSpew(JitSpew_Codegen, __FUNCTION__);
   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, lhs);
-  EmitStoreResult(masm, lhs, JSVAL_TYPE_INT32, output);
+  masm.flexibleRshift32Arithmetic(rhs, scratch);
+  EmitStoreResult(masm, scratch, JSVAL_TYPE_INT32, output);
 
   return true;
 }
 
 bool CacheIRCompiler::emitInt32URightShiftResult() {
   JitSpew(JitSpew_Codegen, __FUNCTION__);
   AutoOutputRegister output(*this);
 
   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, lhs);
+  masm.flexibleRshift32(rhs, scratch);
   Label intDone, floatDone;
   if (allowDouble) {
     Label toUint;
-    masm.branchTest32(Assembler::Signed, lhs, lhs, &toUint);
+    masm.branchTest32(Assembler::Signed, scratch, scratch, &toUint);
     masm.jump(&intDone);
 
     masm.bind(&toUint);
     ScratchDoubleScope fpscratch(masm);
-    masm.convertUInt32ToDouble(lhs, fpscratch);
+    masm.convertUInt32ToDouble(scratch, fpscratch);
     masm.boxDouble(fpscratch, output.valueReg(), fpscratch);
     masm.jump(&floatDone);
   } else {
-    masm.branchTest32(Assembler::Signed, lhs, lhs, failure->label());
+    masm.branchTest32(Assembler::Signed, scratch, scratch, failure->label());
   }
   masm.bind(&intDone);
-  EmitStoreResult(masm, lhs, JSVAL_TYPE_INT32, output);
+  EmitStoreResult(masm, scratch, JSVAL_TYPE_INT32, output);
   masm.bind(&floatDone);
   return true;
 }
 
 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)) {
     return false;
   }
 
   // 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.neg32(val);
-  masm.tagValue(JSVAL_TYPE_INT32, val, output.valueReg());
+  masm.mov(val, scratch);
+  masm.neg32(scratch);
+  masm.tagValue(JSVAL_TYPE_INT32, scratch, output.valueReg());
   return true;
 }
 
 bool CacheIRCompiler::emitInt32IncResult() {
   AutoOutputRegister output(*this);
   Register input = allocator.useRegister(masm, reader.int32OperandId());
   AutoScratchRegisterMaybeOutput scratch(allocator, masm, output);
 
@@ -2480,18 +2505,21 @@ bool CacheIRCompiler::emitInt32DecResult
 
   return true;
 }
 
 bool CacheIRCompiler::emitInt32NotResult() {
   JitSpew(JitSpew_Codegen, __FUNCTION__);
   AutoOutputRegister output(*this);
   Register val = allocator.useRegister(masm, reader.int32OperandId());
-  masm.not32(val);
-  masm.tagValue(JSVAL_TYPE_INT32, val, output.valueReg());
+  AutoScratchRegisterMaybeOutput scratch(allocator, masm, output);
+
+  masm.mov(val, scratch);
+  masm.not32(scratch);
+  masm.tagValue(JSVAL_TYPE_INT32, scratch, output.valueReg());
   return true;
 }
 
 bool CacheIRCompiler::emitDoubleNegationResult() {
   JitSpew(JitSpew_Codegen, __FUNCTION__);
   AutoOutputRegister output(*this);
   ValueOperand val = allocator.useValueRegister(masm, reader.valOperandId());
 
--- a/js/src/jit/CacheIRCompiler.h
+++ b/js/src/jit/CacheIRCompiler.h
@@ -146,16 +146,19 @@ namespace jit {
 // materialization.
 //
 // Intra-op Register allocation:
 //
 // During the emission of a CacheIR op, code can ask the CacheRegisterAllocator
 // 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.
 //
 // - AutoOutputReg: The register which will hold the output value of the stub.
 // - AutoScratchReg: By default, an arbitrary scratch register, however a
 //   specific register can be requested.
 // - AutoScratchRegMaybeOutput: Any arbitrary scratch register, but the output