Bug 1657830 part 1 - Remove some never-used data from jump relocation table on ARM64 and x64. r=tcampbell
authorJan de Mooij <jdemooij@mozilla.com>
Fri, 07 Aug 2020 17:50:24 +0000
changeset 544365 40177251aedfccc2a0e0d776d1b8cb2c45102707
parent 544364 13c2bda8cb38a6e5d8004483427c582859ceaf0d
child 544366 b05b003aadab616dfcc586f2d3a1559ef971e0e8
push id37693
push usercsabou@mozilla.com
push dateWed, 12 Aug 2020 15:55:27 +0000
treeherdermozilla-central@fb8518702f1b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstcampbell
bugs1657830
milestone81.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 1657830 part 1 - Remove some never-used data from jump relocation table on ARM64 and x64. r=tcampbell The jump instruction itself is sufficient to get the address from the extended jump table. This has been 'dead' code on x64 since the code landed in 2011. ARM64 copied it. Differential Revision: https://phabricator.services.mozilla.com/D86368
js/src/jit/arm64/Assembler-arm64.cpp
js/src/jit/x64/Assembler-x64.cpp
js/src/jit/x64/Assembler-x64.h
--- a/js/src/jit/arm64/Assembler-arm64.cpp
+++ b/js/src/jit/arm64/Assembler-arm64.cpp
@@ -72,26 +72,16 @@ namespace js {
 namespace jit {
 
 void Assembler::finish() {
   armbuffer_.flushPool();
 
   // The extended jump table is part of the code buffer.
   ExtendedJumpTable_ = emitExtendedJumpTable();
   Assembler::FinalizeCode();
-
-  // The jump relocation table starts with a fixed-width integer pointing
-  // to the start of the extended jump table.
-  // Space for this integer is allocated by Assembler::addJumpRelocation()
-  // before writing the first entry.
-  // Don't touch memory if we saw an OOM error.
-  if (jumpRelocations_.length() && !oom()) {
-    MOZ_ASSERT(jumpRelocations_.length() >= sizeof(uint32_t));
-    *(uint32_t*)jumpRelocations_.buffer() = ExtendedJumpTable_.getOffset();
-  }
 }
 
 bool Assembler::appendRawCode(const uint8_t* code, size_t numBytes) {
   flush();
   return armbuffer_.appendRawCode(code, numBytes);
 }
 
 bool Assembler::reserve(size_t size) {
@@ -285,27 +275,17 @@ void Assembler::bind(Label* label, Buffe
   // Bind the label, so that future uses may encode the offset immediately.
   label->bind(targetOffset.getOffset());
 }
 
 void Assembler::addJumpRelocation(BufferOffset src, RelocationKind reloc) {
   // Only JITCODE relocations are patchable at runtime.
   MOZ_ASSERT(reloc == RelocationKind::JITCODE);
 
-  // The jump relocation table starts with a fixed-width integer pointing
-  // to the start of the extended jump table. But, we don't know the
-  // actual extended jump table offset yet, so write a 0 which we'll
-  // patch later in Assembler::finish().
-  if (!jumpRelocations_.length()) {
-    jumpRelocations_.writeFixedUint32_t(0);
-  }
-
-  // Each entry in the table is an (offset, extendedTableIndex) pair.
   jumpRelocations_.writeUnsigned(src.getOffset());
-  jumpRelocations_.writeUnsigned(pendingJumps_.length());
 }
 
 void Assembler::addPendingJump(BufferOffset src, ImmPtr target,
                                RelocationKind reloc) {
   MOZ_ASSERT(target.value != nullptr);
 
   if (reloc == RelocationKind::JITCODE) {
     addJumpRelocation(src, reloc);
@@ -446,37 +426,30 @@ void Assembler::ToggleCall(CodeLocationL
 void Assembler::UpdateLoad64Value(Instruction* inst0, uint64_t value) {
   MOZ_ASSERT(inst0->IsLDR());
   uint64_t* literal = inst0->LiteralAddress<uint64_t*>();
   *literal = value;
 }
 
 class RelocationIterator {
   CompactBufferReader reader_;
-  uint32_t tableStart_;
-  uint32_t offset_;
-  uint32_t extOffset_;
+  uint32_t offset_ = 0;
 
  public:
-  explicit RelocationIterator(CompactBufferReader& reader) : reader_(reader) {
-    // The first uint32_t stores the extended table offset.
-    tableStart_ = reader_.readFixedUint32_t();
-  }
+  explicit RelocationIterator(CompactBufferReader& reader) : reader_(reader) {}
 
   bool read() {
     if (!reader_.more()) {
       return false;
     }
     offset_ = reader_.readUnsigned();
-    extOffset_ = reader_.readUnsigned();
     return true;
   }
 
   uint32_t offset() const { return offset_; }
-  uint32_t extendedOffset() const { return extOffset_; }
 };
 
 static JitCode* CodeFromJump(JitCode* code, uint8_t* jump) {
   const Instruction* inst = (const Instruction*)jump;
   uint8_t* target;
 
   // We're expecting a call created by MacroAssembler::call(JitCode*).
   // It looks like:
--- a/js/src/jit/x64/Assembler-x64.cpp
+++ b/js/src/jit/x64/Assembler-x64.cpp
@@ -106,26 +106,18 @@ ABIArg ABIArgGenerator::next(MIRType typ
     default:
       MOZ_CRASH("Unexpected argument type");
   }
   return current_;
 #endif
 }
 
 void Assembler::writeRelocation(JmpSrc src, RelocationKind reloc) {
-  if (!jumpRelocations_.length()) {
-    // The jump relocation table starts with a fixed-width integer pointing
-    // to the start of the extended jump table. But, we don't know the
-    // actual extended jump table offset yet, so write a 0 which we'll
-    // patch later.
-    jumpRelocations_.writeFixedUint32_t(0);
-  }
   if (reloc == RelocationKind::JITCODE) {
     jumpRelocations_.writeUnsigned(src.offset());
-    jumpRelocations_.writeUnsigned(jumps_.length());
   }
 }
 
 void Assembler::addPendingJump(JmpSrc src, ImmPtr target,
                                RelocationKind reloc) {
   MOZ_ASSERT(target.value != nullptr);
 
   // Emit reloc before modifying the jump table, since it computes a 0-based
@@ -159,25 +151,16 @@ void Assembler::finish() {
     masm.ud2();
     return;
   }
 
   // Emit the jump table.
   masm.haltingAlign(SizeOfJumpTableEntry);
   extendedJumpTable_ = masm.size();
 
-  // Now that we know the offset to the jump table, squirrel it into the
-  // jump relocation buffer if any JitCode references exist and must be
-  // tracked for GC.
-  MOZ_ASSERT_IF(jumpRelocations_.length(),
-                jumpRelocations_.length() >= sizeof(uint32_t));
-  if (jumpRelocations_.length()) {
-    *(uint32_t*)jumpRelocations_.buffer() = extendedJumpTable_;
-  }
-
   // Zero the extended jumps table.
   for (size_t i = 0; i < jumps_.length(); i++) {
 #ifdef DEBUG
     size_t oldSize = masm.size();
 #endif
     masm.jmp_rip(2);
     MOZ_ASSERT_IF(!masm.oom(), masm.size() - oldSize == 6);
     // Following an indirect branch with ud2 hints to the hardware that
@@ -219,37 +202,30 @@ void Assembler::executableCopy(uint8_t* 
       // *after* the extended jump, i.e. after the 64-bit immedate.
       X86Encoding::SetPointer(entry + SizeOfExtendedJump, rp.target);
     }
   }
 }
 
 class RelocationIterator {
   CompactBufferReader reader_;
-  uint32_t tableStart_;
-  uint32_t offset_;
-  uint32_t extOffset_;
+  uint32_t offset_ = 0;
 
  public:
-  explicit RelocationIterator(CompactBufferReader& reader)
-      : reader_(reader), offset_(0), extOffset_(0) {
-    tableStart_ = reader_.readFixedUint32_t();
-  }
+  explicit RelocationIterator(CompactBufferReader& reader) : reader_(reader) {}
 
   bool read() {
     if (!reader_.more()) {
       return false;
     }
     offset_ = reader_.readUnsigned();
-    extOffset_ = reader_.readUnsigned();
     return true;
   }
 
   uint32_t offset() const { return offset_; }
-  uint32_t extendedOffset() const { return extOffset_; }
 };
 
 JitCode* Assembler::CodeFromJump(JitCode* code, uint8_t* jump) {
   uint8_t* target = (uint8_t*)X86Encoding::GetRel32Target(jump);
   if (target >= code->raw() &&
       target < code->raw() + code->instructionsSize()) {
     // This jump is within the code buffer, so it has been redirected to
     // the extended jump table.
--- a/js/src/jit/x64/Assembler-x64.h
+++ b/js/src/jit/x64/Assembler-x64.h
@@ -287,25 +287,17 @@ namespace jit {
 static constexpr ValueOperand JSReturnOperand = ValueOperand(JSReturnReg);
 
 class Assembler : public AssemblerX86Shared {
   // x64 jumps may need extra bits of relocation, because a jump may extend
   // beyond the signed 32-bit range. To account for this we add an extended
   // jump table at the bottom of the instruction stream, and if a jump
   // overflows its range, it will redirect here.
   //
-  // In our relocation table, we store two offsets instead of one: the offset
-  // to the original jump, and an offset to the extended jump if we will need
-  // to use it instead. The offsets are stored as:
-  //    [unsigned] Unsigned offset to short jump, from the start of the code.
-  //    [unsigned] Unsigned offset to the extended jump, from the start of
-  //               the jump table, in units of SizeOfJumpTableEntry.
-  //
-  // The start of the relocation table contains the offset from the code
-  // buffer to the start of the extended jump table.
+  // The relocation table stores the offset to the short jump.
   //
   // Each entry in this table is a jmp [rip], followed by a ud2 to hint to the
   // hardware branch predictor that there is no fallthrough, followed by the
   // eight bytes containing an immediate address. This comes out to 16 bytes.
   //    +1 byte for opcode
   //    +1 byte for mod r/m
   //    +4 bytes for rip-relative offset (2)
   //    +2 bytes for ud2 instruction