Bug 1541404 part 28 - Fix ARM issues. r=tcampbell
☠☠ backed out by 9afa1dcd247b ☠ ☠
authorJan de Mooij <jdemooij@mozilla.com>
Tue, 14 May 2019 10:36:17 +0000
changeset 532576 760cc10c63b0a19b2553560c69d32a91f0472074
parent 532575 2742b0a517fb324dca5d5968a8d4556db2209f34
child 532577 c4b0bd61050bb7f3117c2a46ed740fc5d61d0700
push id11270
push userrgurzau@mozilla.com
push dateWed, 15 May 2019 15:07:19 +0000
treeherdermozilla-beta@571bc76da583 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstcampbell
bugs1541404
milestone68.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 1541404 part 28 - Fix ARM issues. r=tcampbell 1. We can't use loadValue for JSOP_DOUBLE because on ARM that might use LDRD or LDM and these are not guaranteed to support unaligned loads. Fix is to add loadUnalignedValue that always uses plain 32-bit loads. 2. DebugTrapHandler's fast path for the interpreter used "lr" as second scratch register, clobbering the return address. The setSecondScratchRegister mechanism prevents this. Differential Revision: https://phabricator.services.mozilla.com/D30879
js/src/jit/BaselineCompiler.cpp
js/src/jit/arm/MacroAssembler-arm.cpp
js/src/jit/arm/MacroAssembler-arm.h
js/src/jit/arm64/MacroAssembler-arm64.h
js/src/jit/mips32/MacroAssembler-mips32.h
js/src/jit/mips64/MacroAssembler-mips64.h
js/src/jit/none/MacroAssembler-none.h
js/src/jit/x64/MacroAssembler-x64.h
js/src/jit/x86/MacroAssembler-x86.h
--- a/js/src/jit/BaselineCompiler.cpp
+++ b/js/src/jit/BaselineCompiler.cpp
@@ -411,17 +411,17 @@ static void LoadUint24Operand(MacroAssem
   masm.rshift32(Imm32(8), dest);
 }
 
 static void LoadInlineValueOperand(MacroAssembler& masm, Register pc,
                                    ValueOperand dest) {
   // Note: the Value might be unaligned but as above we rely on all our
   // platforms having appropriate support for unaligned accesses (except for
   // floating point instructions on ARM).
-  masm.loadValue(Address(pc, sizeof(jsbytecode)), dest);
+  masm.loadUnalignedValue(Address(pc, sizeof(jsbytecode)), dest);
 }
 
 template <>
 void BaselineCompilerCodeGen::loadScript(Register dest) {
   masm.movePtr(ImmGCPtr(handler.script()), dest);
 }
 
 template <>
@@ -6682,16 +6682,20 @@ bool BaselineInterpreterGenerator::gener
 JitCode* JitRuntime::generateDebugTrapHandler(JSContext* cx,
                                               DebugTrapHandlerKind kind) {
   StackMacroAssembler masm;
 
   AllocatableGeneralRegisterSet regs(GeneralRegisterSet::All());
   regs.takeUnchecked(BaselineFrameReg);
   regs.takeUnchecked(ICStubReg);
   regs.takeUnchecked(PCRegAtStart);
+#ifdef JS_CODEGEN_ARM
+  regs.takeUnchecked(BaselineSecondScratchReg);
+  masm.setSecondScratchReg(BaselineSecondScratchReg);
+#endif
   Register scratch1 = regs.takeAny();
   Register scratch2 = regs.takeAny();
   Register scratch3 = regs.takeAny();
 
   if (kind == DebugTrapHandlerKind::Interpreter) {
     // The interpreter calls this for every script when debugging, so check if
     // the script has any breakpoints or is in step mode before calling into
     // C++.
--- a/js/src/jit/arm/MacroAssembler-arm.cpp
+++ b/js/src/jit/arm/MacroAssembler-arm.cpp
@@ -3045,19 +3045,16 @@ void MacroAssemblerARMCompat::loadValue(
     }
   } else {
     ma_alu(addr.base, lsl(addr.index, addr.scale), scratch, OpAdd);
     loadValue(Address(scratch, addr.offset), val);
   }
 }
 
 void MacroAssemblerARMCompat::loadValue(Address src, ValueOperand val) {
-  Address payload = ToPayload(src);
-  Address type = ToType(src);
-
   // TODO: copy this code into a generic function that acts on all sequences
   // of memory accesses
   if (isValueDTRDCandidate(val)) {
     // If the value we want is in two consecutive registers starting with an
     // even register, they can be combined as a single ldrd.
     int offset = src.offset;
     if (offset < 256 && offset > -256) {
       ma_ldrd(EDtrAddr(src.base, EDtrOffImm(src.offset)), val.payloadReg(),
@@ -3091,26 +3088,35 @@ void MacroAssemblerARMCompat::loadValue(
       }
       startDataTransferM(IsLoad, src.base, mode);
       transferReg(val.payloadReg());
       transferReg(val.typeReg());
       finishDataTransfer();
       return;
     }
   }
+
+  loadUnalignedValue(src, val);
+}
+
+void MacroAssemblerARMCompat::loadUnalignedValue(const Address& src,
+                                                 ValueOperand dest) {
+  Address payload = ToPayload(src);
+  Address type = ToType(src);
+
   // Ensure that loading the payload does not erase the pointer to the Value
   // in memory.
-  if (type.base != val.payloadReg()) {
+  if (type.base != dest.payloadReg()) {
     SecondScratchRegisterScope scratch2(asMasm());
-    ma_ldr(payload, val.payloadReg(), scratch2);
-    ma_ldr(type, val.typeReg(), scratch2);
+    ma_ldr(payload, dest.payloadReg(), scratch2);
+    ma_ldr(type, dest.typeReg(), scratch2);
   } else {
     SecondScratchRegisterScope scratch2(asMasm());
-    ma_ldr(type, val.typeReg(), scratch2);
-    ma_ldr(payload, val.payloadReg(), scratch2);
+    ma_ldr(type, dest.typeReg(), scratch2);
+    ma_ldr(payload, dest.payloadReg(), scratch2);
   }
 }
 
 void MacroAssemblerARMCompat::tagValue(JSValueType type, Register payload,
                                        ValueOperand dest) {
   MOZ_ASSERT(dest.typeReg() != dest.payloadReg());
   if (payload != dest.payloadReg()) {
     ma_mov(payload, dest.payloadReg());
--- a/js/src/jit/arm/MacroAssembler-arm.h
+++ b/js/src/jit/arm/MacroAssembler-arm.h
@@ -1055,16 +1055,21 @@ class MacroAssemblerARMCompat : public M
     store32(temp, ToPayload(dest));
   }
 
   void loadValue(Address src, ValueOperand val);
   void loadValue(Operand dest, ValueOperand val) {
     loadValue(dest.toAddress(), val);
   }
   void loadValue(const BaseIndex& addr, ValueOperand val);
+
+  // Like loadValue but guaranteed to not use LDRD or LDM instructions (these
+  // don't support unaligned accesses).
+  void loadUnalignedValue(const Address& src, ValueOperand dest);
+
   void tagValue(JSValueType type, Register payload, ValueOperand dest);
 
   void pushValue(ValueOperand val);
   void popValue(ValueOperand val);
   void pushValue(const Value& val) {
     push(Imm32(val.toNunboxTag()));
     if (val.isGCThing()) {
       push(ImmGCPtr(val.toGCThing()));
--- a/js/src/jit/arm64/MacroAssembler-arm64.h
+++ b/js/src/jit/arm64/MacroAssembler-arm64.h
@@ -292,16 +292,19 @@ class MacroAssemblerCompat : public vixl
     Ldr(ARMRegister(val, 64), MemOperand(src));
   }
   void loadValue(Address src, ValueOperand val) {
     Ldr(ARMRegister(val.valueReg(), 64), MemOperand(src));
   }
   void loadValue(const BaseIndex& src, ValueOperand val) {
     doBaseIndex(ARMRegister(val.valueReg(), 64), src, vixl::LDR_x);
   }
+  void loadUnalignedValue(const Address& src, ValueOperand dest) {
+    loadValue(src, dest);
+  }
   void tagValue(JSValueType type, Register payload, ValueOperand dest) {
     // This could be cleverer, but the first attempt had bugs.
     Orr(ARMRegister(dest.valueReg(), 64), ARMRegister(payload, 64),
         Operand(ImmShiftedTag(type).value));
   }
   void pushValue(ValueOperand val) {
     vixl::MacroAssembler::Push(ARMRegister(val.valueReg(), 64));
   }
--- a/js/src/jit/mips32/MacroAssembler-mips32.h
+++ b/js/src/jit/mips32/MacroAssembler-mips32.h
@@ -513,16 +513,21 @@ class MacroAssemblerMIPSCompat : public 
     store32(temp, ToPayload(dest));
   }
 
   void loadValue(Address src, ValueOperand val);
   void loadValue(Operand dest, ValueOperand val) {
     loadValue(dest.toAddress(), val);
   }
   void loadValue(const BaseIndex& addr, ValueOperand val);
+
+  void loadUnalignedValue(const Address& src, ValueOperand dest) {
+    loadValue(src, dest);
+  }
+
   void tagValue(JSValueType type, Register payload, ValueOperand dest);
 
   void pushValue(ValueOperand val);
   void popValue(ValueOperand val);
 #if MOZ_LITTLE_ENDIAN
   void pushValue(const Value& val) {
     push(Imm32(val.toNunboxTag()));
     if (val.isGCThing()) {
--- a/js/src/jit/mips64/MacroAssembler-mips64.h
+++ b/js/src/jit/mips64/MacroAssembler-mips64.h
@@ -555,16 +555,21 @@ class MacroAssemblerMIPS64Compat : publi
     storePtr(temp, dest);
   }
 
   void loadValue(Address src, ValueOperand val);
   void loadValue(Operand dest, ValueOperand val) {
     loadValue(dest.toAddress(), val);
   }
   void loadValue(const BaseIndex& addr, ValueOperand val);
+
+  void loadUnalignedValue(const Address& src, ValueOperand dest) {
+    loadValue(src, dest);
+  }
+
   void tagValue(JSValueType type, Register payload, ValueOperand dest);
 
   void pushValue(ValueOperand val);
   void popValue(ValueOperand val);
   void pushValue(const Value& val) {
     if (val.isGCThing()) {
       writeDataRelocation(val);
       movWithPatch(ImmWord(val.asRawBits()), ScratchRegister);
--- a/js/src/jit/none/MacroAssembler-none.h
+++ b/js/src/jit/none/MacroAssembler-none.h
@@ -284,16 +284,20 @@ class MacroAssemblerNone : public Assemb
   template <typename T, typename S, typename U>
   void storeValue(T, S, U) {
     MOZ_CRASH();
   }
   template <typename T, typename S>
   void loadValue(T, S) {
     MOZ_CRASH();
   }
+  template <typename T, typename S>
+  void loadUnalignedValue(T, S) {
+    MOZ_CRASH();
+  }
   template <typename T>
   void pushValue(const T&) {
     MOZ_CRASH();
   }
   template <typename T, typename S>
   void pushValue(T, S) {
     MOZ_CRASH();
   }
--- a/js/src/jit/x64/MacroAssembler-x64.h
+++ b/js/src/jit/x64/MacroAssembler-x64.h
@@ -191,16 +191,19 @@ class MacroAssemblerX64 : public MacroAs
   }
   void loadValue(Operand src, ValueOperand val) { movq(src, val.valueReg()); }
   void loadValue(Address src, ValueOperand val) {
     loadValue(Operand(src), val);
   }
   void loadValue(const BaseIndex& src, ValueOperand val) {
     loadValue(Operand(src), val);
   }
+  void loadUnalignedValue(const Address& src, ValueOperand dest) {
+    loadValue(src, dest);
+  }
   void tagValue(JSValueType type, Register payload, ValueOperand dest) {
     ScratchRegisterScope scratch(asMasm());
     MOZ_ASSERT(dest.valueReg() != scratch);
     if (payload != dest.valueReg()) {
       movq(payload, dest.valueReg());
     }
     mov(ImmShiftedTag(type), scratch);
     orq(scratch, dest.valueReg());
--- a/js/src/jit/x86/MacroAssembler-x86.h
+++ b/js/src/jit/x86/MacroAssembler-x86.h
@@ -213,16 +213,19 @@ class MacroAssemblerX86 : public MacroAs
     }
   }
   void loadValue(Address src, ValueOperand val) {
     loadValue(Operand(src), val);
   }
   void loadValue(const BaseIndex& src, ValueOperand val) {
     loadValue(Operand(src), val);
   }
+  void loadUnalignedValue(const Address& src, ValueOperand dest) {
+    loadValue(src, dest);
+  }
   void tagValue(JSValueType type, Register payload, ValueOperand dest) {
     MOZ_ASSERT(dest.typeReg() != dest.payloadReg());
     if (payload != dest.payloadReg()) {
       movl(payload, dest.payloadReg());
     }
     movl(ImmType(type), dest.typeReg());
   }
   void pushValue(ValueOperand val) {