Avoid redundant AArch64 load_addr instructions

Summary:
AArch64 `load_addr` always reserved an eight-byte relocation slot so it could expand to `adrp+add`. For page-aligned absolute targets, every relocation outcome needs only one instruction: `adr` for a nearby target, `adrp` for a target in page range, or a literal `ldr` for a farther target. The old layout therefore retained a redundant NOP or emitted `add #0`.

Emit a four-byte placeholder when the absolute target has a zero page offset. Update the Builder size estimate before branch relaxation, and make relocation patch only the first word for single-instruction `adr`, `adrp`, and literal `ldr` encodings. Non-page-aligned targets continue reserving two words and emitting `adrp+add` when necessary; label-based ADR relocation retains its fixed slot because final alignment is not known at emission time.

Extend the AArch64 patching coverage for the one-instruction `adr`, `adrp`, and literal `ldr` paths. Add a Builder boundary test proving the smaller `load_addr` size is included correctly when deciding whether a nearby `tbz` requires relaxation.

Reviewed By: alexmalyshev

Differential Revision: D114745354

fbshipit-source-id: 47398b94fdc51e203668040c85a9d50c505ceffa

Author: kddnewton

cinderx/ThirdParty/asmjit/src/asmjit/arm/a64assembler.cpp

@@ -1297,9 +1297,10 @@ Error Assembler::_emit(InstId instId, const Operand_& o0, const Operand_& o1, co
           re->_sourceOffset = codeOffset;
           re->_payload = targetAddress;
 
-          // Emit adr Rd, #0 (encodes Rd in bits [4:0]) + NOP placeholder.
+          // A page-aligned target never needs the second instruction.
           writer.emit32uLE(opcode.get());
-          writer.emit32uLE(0xD503201Fu); // NOP
+          if ((targetAddress & 0xFFFu) != 0)
+            writer.emit32uLE(0xD503201Fu); // NOP
           goto EmitDone;
         }
 

cinderx/ThirdParty/asmjit/src/asmjit/arm/a64builder.cpp

@@ -92,8 +92,14 @@ static uint32_t estimateNodeSize(BaseNode* node, uint32_t currentOffset) noexcep
     for (uint32_t i = 0; i < inst->opCount(); i++) {
       const Operand_& op = inst->op(i);
 
-      if (realId == Inst::kIdAdr && (op.isLabel() || op.isImm()))
-        return 8;
+      if (realId == Inst::kIdAdr) {
+        if (op.isLabel())
+          return 8;
+        if (op.isImm() &&
+            op.as<Imm>().predicate() == Predicate::kAbsoluteAddress &&
+            (op.as<Imm>().valueAs<uint64_t>() & 0xFFFu) != 0)
+          return 8;
+      }
       if (realId == Inst::kIdLdr && op.isMem() && op.as<Mem>().hasBaseLabel())
         return 8;
       if (realId == Inst::kIdMov && op.isImm() && op.as<Imm>().valueAs<uint64_t>() > 0xFFFF)

cinderx/ThirdParty/asmjit/src/asmjit/core/codeholder.cpp

@@ -1195,10 +1195,10 @@ size_t CodeHolder::codeSize() const noexcept {
   return size_t(offset);
 }
 
-// Tries to encode a PC-relative address load into the 8-byte slot at
-// `buffer + offset` using `adr Rd, target` (±1MB) or `adrp Rd, page;
-// add Rd, Rd, #off` (±4GB). Returns true on success, false if the
-// displacement is too large for either encoding.
+// Tries to encode a PC-relative address load at `buffer + offset` using
+// `adr Rd, target` (±1MB) or `adrp Rd, page` with an optional
+// `add Rd, Rd, #off` (±4GB). Returns true on success, false if the displacement
+// is too large for either encoding.
 static bool tryEncodeAdrOrAdrpAdd(uint8_t* buffer, size_t offset, uint64_t targetAddress, uint64_t pc, uint32_t rd) noexcept {
   int64_t displacement = int64_t(targetAddress - pc);
 
@@ -1207,7 +1207,6 @@ static bool tryEncodeAdrOrAdrpAdd(uint8_t* buffer, size_t offset, uint64_t targe
     uint32_t immHi = (uint32_t(displacement) >> 2) & 0x7FFFFu;
     uint32_t adrOpcode = 0x10000000u | (immLo << 29) | (immHi << 5) | rd;
     Support::writeU32uLE(buffer + offset, adrOpcode);
-    Support::writeU32uLE(buffer + offset + 4, 0xD503201Fu); // NOP
     return true;
   }
 
@@ -1217,9 +1216,11 @@ static bool tryEncodeAdrOrAdrpAdd(uint8_t* buffer, size_t offset, uint64_t targe
     uint32_t immLo = uint32_t(pageDelta) & 3u;
     uint32_t immHi = (uint32_t(pageDelta) >> 2) & 0x7FFFFu;
     uint32_t adrpOpcode = 0x90000000u | (immLo << 29) | (immHi << 5) | rd;
-    uint32_t addOpcode = 0x91000000u | (pageOffset << 10) | (rd << 5) | rd;
     Support::writeU32uLE(buffer + offset, adrpOpcode);
-    Support::writeU32uLE(buffer + offset + 4, addOpcode);
+    if (pageOffset != 0) {
+      uint32_t addOpcode = 0x91000000u | (pageOffset << 10) | (rd << 5) | rd;
+      Support::writeU32uLE(buffer + offset + 4, addOpcode);
+    }
     return true;
   }
 
@@ -1474,8 +1475,8 @@ Error CodeHolder::relocateToBase(uint64_t baseAddress) noexcept {
       }
 
       case RelocType::kA64AdrAbsEntry: {
-        // AArch64: absolute address payload. Relaxes to `adr` (±1MB),
-        // `adrp+add` (±4GB), or `ldr Rd, [pc+off]` from the address table.
+        // AArch64: absolute address payload. Page-aligned targets reserve one
+        // instruction; other targets reserve two in case `adrp+add` is needed.
         uint64_t targetAddress = re->payload();
         uint64_t pc = baseAddress + sectionOffset + sourceOffset;
         uint32_t rd = Support::readU32uLE(buffer + sourceOffset) & 0x1Fu;
@@ -1502,7 +1503,6 @@ Error CodeHolder::relocateToBase(uint64_t baseAddress) noexcept {
 
           uint32_t ldrOpcode = 0x58000000u | ((uint32_t(ldrImm19) & 0x7FFFFu) << 5) | rd;
           Support::writeU32uLE(buffer + sourceOffset, ldrOpcode);
-          Support::writeU32uLE(buffer + sourceOffset + 4, 0xD503201Fu); // NOP
 
           Support::writeU64uLE(addressTableEntryData + atEntryIndex, targetAddress);
         }

cinderx/ThirdParty/asmjit/test/asmjit_test_patching_a64.cpp

@@ -952,17 +952,15 @@ UNIT(a64_load_addr_reloc_ldr) {
   const Section* text = code.textSection();
   const uint8_t* buf = text->data();
   uint32_t instr0 = readU32LE(buf);
-  uint32_t instr1 = readU32LE(buf + 4);
 
   // Should be ldr x4, [pc+off]: opcode 0x58000000 | (imm19 << 5) | 4
-  // The address table is right after the text section. Text = 8 bytes,
-  // so address table is at offset 8. ldr displacement = 8 - 0 = 8.
+  // The address table is aligned to offset 8. ldr displacement = 8 - 0 = 8.
   // imm19 = 8 / 4 = 2
   // ldr x4, [pc+8]: 0x58000000 | (2 << 5) | 4 = 0x58000044
+  EXPECT_EQ(text->bufferSize(), 4u)
+    .message("Expected a single ldr instruction");
   EXPECT_EQ(instr0, 0x58000044u)
     .message("Expected ldr x4, [pc+off], got: 0x%08X", instr0);
-  EXPECT_EQ(instr1, 0xD503201Fu)
-    .message("Expected NOP, got: 0x%08X", instr1);
 }
 
 // Test load_addr with negative displacement (target address < PC).
@@ -983,15 +981,14 @@ UNIT(a64_load_addr_reloc_adr_negative) {
   const Section* text = code.textSection();
   const uint8_t* buf = text->data();
   uint32_t instr0 = readU32LE(buf);
-  uint32_t instr1 = readU32LE(buf + 4);
 
   // displacement = -128 = 0xFFFFFF80 (as uint32)
   // immLo = 0xFFFFFF80 & 3 = 0, immHi = (0xFFFFFF80 >> 2) & 0x7FFFF = 0x7FFE0
   // adr x5: 0x10000000 | (0 << 29) | (0x7FFE0 << 5) | 5 = 0x10FFFC05
+  EXPECT_EQ(text->bufferSize(), 4u)
+    .message("Expected a single adr instruction");
   EXPECT_EQ(instr0, 0x10FFFC05u)
     .message("Expected adr x5, #-128, got: 0x%08X", instr0);
-  EXPECT_EQ(instr1, 0xD503201Fu)
-    .message("Expected NOP, got: 0x%08X", instr1);
 }
 
 // Test load_addr with a non-zero base address that still fits in adr range.
@@ -1107,7 +1104,7 @@ UNIT(a64_load_addr_reloc_adr_max_negative) {
     .message("Expected adr x7 with displacement -0x100000, got: 0x%08X", instr0);
 }
 
-// Test load_addr just past adr range should use adrp+add.
+// Test load_addr just past adr range should use adrp without a zero add.
 UNIT(a64_load_addr_reloc_adr_to_adrp_boundary) {
   CodeHolder code;
   a64::Assembler as;
@@ -1126,14 +1123,39 @@ UNIT(a64_load_addr_reloc_adr_to_adrp_boundary) {
   const uint8_t* buf = text->data();
   uint32_t instr0 = readU32LE(buf);
 
-  uint32_t instr1 = readU32LE(buf + 4);
-
-  // Should be adrp+add, not adr, since 0x100000 = 2^20 exceeds 21-bit signed max.
+  // Should be adrp, not adr, since 0x100000 = 2^20 exceeds 21-bit signed max.
   // pageDelta = (0x100000 >> 12) - 0 = 0x100, pageOffset = 0
   // adrp x8: 0x90000000 | (0x40 << 5) | 8 = 0x90000808
-  // add x8, x8, #0: 0x91000000 | (0 << 10) | (8 << 5) | 8 = 0x91000108
+  EXPECT_EQ(text->bufferSize(), 4u)
+    .message("Expected a single adrp instruction");
   EXPECT_EQ(instr0, 0x90000808u)
     .message("Expected adrp x8 (just past adr range), got: 0x%08X", instr0);
-  EXPECT_EQ(instr1, 0x91000108u)
-    .message("Expected add x8, x8, #0, got: 0x%08X", instr1);
+}
+
+// Test that Builder accounts for a page-aligned load_addr as one instruction
+// when deciding whether a branch needs relaxation.
+UNIT(a64_load_addr_builder_single_instruction_size) {
+  CodeHolder code;
+  Environment env(Arch::kAArch64);
+  code.init(env, 0);
+
+  a64::Builder as(&code);
+  Label target = as.newLabel();
+  as.tbz(a64::x0, 0, target);
+  as.load_addr(a64::x1, uint64_t(0x200000));
+  for (uint32_t i = 0; i < 8189; i++)
+    as.nop();
+  as.bind(target);
+  as.nop();
+
+  EXPECT_EQ(as.finalize(), kErrorOk);
+  EXPECT_EQ(code.flatten(), kErrorOk);
+  EXPECT_EQ(code.relocateToBase(0), kErrorOk);
+
+  const Section* text = code.textSection();
+  const uint8_t* buf = text->data();
+  EXPECT_EQ(text->bufferSize(), 32768u)
+    .message("Expected tbz and load_addr to remain single instructions");
+  EXPECT_EQ(Support::readU32uLE(buf + 4), 0x90001001u)
+    .message("Expected page-aligned load_addr immediately after tbz");
 }