Skip to content

MC: Centralize X86 PC-relative fixup adjustment in MCAssembler #147113

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

MaskRay
Copy link
Member

@MaskRay MaskRay commented Jul 4, 2025

Move the X86 PC-relative fixup adjustment from
X86MCCodeEmitter::emitImmediate to MCAssembler, leveraging a generalized
evaluateFixup. This saves a MCBinaryExpr. For call foo, the fixup
expression is now foo instead of foo-4. There is no change in
generated relocations.

In bolt/lib/Target/X86/X86MCPlusBuilder.cpp, createRelocation needs to
decrease the addend.

Both max-rss and instructions:u show a minor decrease.
https://llvm-compile-time-tracker.com/compare.php?from=ea600576a6f94d6f28925c4b99962cc26b463c29&to=016e8fd4ddf851e5555f606c6394241d68f1a7bb&stat=max-rss&linkStats=on

Next: Update targets that use FKF_IsAlignedDownTo32Bits to define
evaluateFixup and remove FKF_IsAlignedDownTo32Bits from the generic
code.

Created using spr 1.3.5-bogner
@llvmbot
Copy link
Member

llvmbot commented Jul 4, 2025

@llvm/pr-subscribers-mc

@llvm/pr-subscribers-bolt

Author: Fangrui Song (MaskRay)

Changes

Move the X86 PC-relative fixup adjustment from
X86MCCodeEmitter::emitImmediate to MCAssembler, leveraging a generalized
evaluateFixup. This saves a MCBinaryExpr. For call foo, the fixup
expression is now foo instead of foo-4. Updated X86 MC tests confirm
no change in generated relocations.

In bolt/lib/Target/X86/X86MCPlusBuilder.cpp, createRelocation needs to
decrease the addend.

Both max-rss and instructions:u show a minor decrease.
https://llvm-compile-time-tracker.com/compare.php?from=ea600576a6f94d6f28925c4b99962cc26b463c29&to=016e8fd4ddf851e5555f606c6394241d68f1a7bb&stat=max-rss&linkStats=on

Next: Update targets that use FKF_IsAlignedDownTo32Bits to define
evaluateFixup and remove FKF_IsAlignedDownTo32Bits from the generic
code.


Patch is 22.41 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/147113.diff

15 Files Affected:

  • (modified) bolt/lib/Target/X86/X86MCPlusBuilder.cpp (+5-3)
  • (modified) llvm/include/llvm/MC/MCAsmBackend.h (+6-3)
  • (modified) llvm/include/llvm/MC/MCValue.h (+1)
  • (modified) llvm/lib/MC/MCAssembler.cpp (+7-7)
  • (modified) llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp (+7-8)
  • (modified) llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h (+2-2)
  • (modified) llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp (+36-1)
  • (modified) llvm/lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp (+1-15)
  • (modified) llvm/test/MC/ELF/mc-dump.s (+1-1)
  • (modified) llvm/test/MC/X86/avx-64-att.s (+1-1)
  • (modified) llvm/test/MC/X86/fma4-att.s (+12-12)
  • (modified) llvm/test/MC/X86/x86-16.s (+2-2)
  • (modified) llvm/test/MC/X86/x86-32.s (+4-4)
  • (modified) llvm/test/MC/X86/x86-64.s (+14-14)
  • (modified) llvm/test/MC/X86/x86_64-encoding.s (+1-1)
diff --git a/bolt/lib/Target/X86/X86MCPlusBuilder.cpp b/bolt/lib/Target/X86/X86MCPlusBuilder.cpp
index a91fd146fa367..21d7562d41572 100644
--- a/bolt/lib/Target/X86/X86MCPlusBuilder.cpp
+++ b/bolt/lib/Target/X86/X86MCPlusBuilder.cpp
@@ -2442,9 +2442,10 @@ class X86MCPlusBuilder : public MCPlusBuilder {
 
     assert(FKI.TargetOffset == 0 && "0-bit relocation offset expected");
     const uint64_t RelOffset = Fixup.getOffset();
+    auto [RelSymbol, RelAddend] = extractFixupExpr(Fixup);
 
     uint32_t RelType;
-    if (FKI.Flags & MCFixupKindInfo::FKF_IsPCRel) {
+    if (Fixup.isPCRel()) {
       switch (FKI.TargetSize) {
       default:
         return std::nullopt;
@@ -2453,6 +2454,9 @@ class X86MCPlusBuilder : public MCPlusBuilder {
       case 32: RelType = ELF::R_X86_64_PC32; break;
       case 64: RelType = ELF::R_X86_64_PC64; break;
       }
+      // Adjust PC-relative fixup offsets, which are calculated from the start
+      // of the next instruction.
+      RelAddend -= FKI.TargetSize / 8;
     } else {
       switch (FKI.TargetSize) {
       default:
@@ -2464,8 +2468,6 @@ class X86MCPlusBuilder : public MCPlusBuilder {
       }
     }
 
-    auto [RelSymbol, RelAddend] = extractFixupExpr(Fixup);
-
     return Relocation({RelOffset, RelSymbol, RelType, RelAddend, 0});
   }
 
diff --git a/llvm/include/llvm/MC/MCAsmBackend.h b/llvm/include/llvm/MC/MCAsmBackend.h
index 54e43b89b7b00..46547560b83ed 100644
--- a/llvm/include/llvm/MC/MCAsmBackend.h
+++ b/llvm/include/llvm/MC/MCAsmBackend.h
@@ -106,9 +106,12 @@ class LLVM_ABI MCAsmBackend {
     return false;
   }
 
-  virtual bool evaluateTargetFixup(const MCFixup &Fixup, const MCValue &Target,
-                                   uint64_t &Value) {
-    llvm_unreachable("Need to implement hook if target has custom fixups");
+  // Evaluate a fixup, returning std::nullopt to use default handling for
+  // `Value` and `IsResolved`. Otherwise, returns `IsResolved` with the
+  // expectation that the hook updates `Value`.
+  virtual std::optional<bool> evaluateFixup(MCFixup &Fixup, MCValue &Target,
+                                            uint64_t &Value) {
+    return {};
   }
 
   void maybeAddReloc(const MCFragment &, const MCFixup &, const MCValue &,
diff --git a/llvm/include/llvm/MC/MCValue.h b/llvm/include/llvm/MC/MCValue.h
index 417c4689d1b47..c9a680ec327a6 100644
--- a/llvm/include/llvm/MC/MCValue.h
+++ b/llvm/include/llvm/MC/MCValue.h
@@ -42,6 +42,7 @@ class MCValue {
   friend class MCExpr;
   MCValue() = default;
   int64_t getConstant() const { return Cst; }
+  void setConstant(int64_t C) { Cst = C; }
   uint32_t getSpecifier() const { return Specifier; }
   void setSpecifier(uint32_t S) { Specifier = S; }
 
diff --git a/llvm/lib/MC/MCAssembler.cpp b/llvm/lib/MC/MCAssembler.cpp
index 4b7845335d822..69192db2a20d3 100644
--- a/llvm/lib/MC/MCAssembler.cpp
+++ b/llvm/lib/MC/MCAssembler.cpp
@@ -161,11 +161,13 @@ bool MCAssembler::evaluateFixup(const MCFragment &F, MCFixup &Fixup,
     return true;
   }
 
-  bool IsResolved = false;
+  // TODO: Require targets to set PCRel at fixup creation time.
   unsigned FixupFlags = getBackend().getFixupKindInfo(Fixup.getKind()).Flags;
-  bool IsPCRel = FixupFlags & MCFixupKindInfo::FKF_IsPCRel;
-  if (FixupFlags & MCFixupKindInfo::FKF_IsTarget) {
-    IsResolved = getBackend().evaluateTargetFixup(Fixup, Target, Value);
+  if (FixupFlags & MCFixupKindInfo::FKF_IsPCRel)
+    Fixup.setPCRel();
+  bool IsResolved = false;
+  if (auto State = getBackend().evaluateFixup(Fixup, Target, Value)) {
+    IsResolved = *State;
   } else {
     const MCSymbol *Add = Target.getAddSym();
     const MCSymbol *Sub = Target.getSubSym();
@@ -177,7 +179,7 @@ bool MCAssembler::evaluateFixup(const MCFragment &F, MCFixup &Fixup,
 
     bool ShouldAlignPC =
         FixupFlags & MCFixupKindInfo::FKF_IsAlignedDownTo32Bits;
-    if (IsPCRel) {
+    if (Fixup.isPCRel()) {
       uint64_t Offset = getFragmentOffset(F) + Fixup.getOffset();
 
       // A number of ARM fixups in Thumb mode require that the effective PC
@@ -202,8 +204,6 @@ bool MCAssembler::evaluateFixup(const MCFragment &F, MCFixup &Fixup,
 
   if (IsResolved && mc::isRelocRelocation(Fixup.getKind()))
     IsResolved = false;
-  if (IsPCRel)
-    Fixup.setPCRel();
   getBackend().applyFixup(F, Fixup, Target, Contents, Value, IsResolved);
   return true;
 }
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
index d298ce769c1dc..79ada59e7a524 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
@@ -70,10 +70,8 @@ MCFixupKindInfo RISCVAsmBackend::getFixupKindInfo(MCFixupKind Kind) const {
       {"fixup_riscv_12_i", 20, 12, 0},
       {"fixup_riscv_lo12_s", 0, 32, 0},
       {"fixup_riscv_pcrel_hi20", 12, 20, MCFixupKindInfo::FKF_IsPCRel},
-      {"fixup_riscv_pcrel_lo12_i", 20, 12,
-       MCFixupKindInfo::FKF_IsPCRel | MCFixupKindInfo::FKF_IsTarget},
-      {"fixup_riscv_pcrel_lo12_s", 0, 32,
-       MCFixupKindInfo::FKF_IsPCRel | MCFixupKindInfo::FKF_IsTarget},
+      {"fixup_riscv_pcrel_lo12_i", 20, 12, MCFixupKindInfo::FKF_IsPCRel},
+      {"fixup_riscv_pcrel_lo12_s", 0, 32, MCFixupKindInfo::FKF_IsPCRel},
       {"fixup_riscv_jal", 12, 20, MCFixupKindInfo::FKF_IsPCRel},
       {"fixup_riscv_branch", 0, 32, MCFixupKindInfo::FKF_IsPCRel},
       {"fixup_riscv_rvc_jump", 2, 11, MCFixupKindInfo::FKF_IsPCRel},
@@ -657,15 +655,16 @@ static const MCFixup *getPCRelHiFixup(const MCSpecifierExpr &Expr,
   return nullptr;
 }
 
-bool RISCVAsmBackend::evaluateTargetFixup(const MCFixup &Fixup,
-                                          const MCValue &Target,
-                                          uint64_t &Value) {
+std::optional<bool> RISCVAsmBackend::evaluateFixup(MCFixup &Fixup,
+                                                   MCValue &Target,
+                                                   uint64_t &Value) {
   const MCFixup *AUIPCFixup;
   const MCFragment *AUIPCDF;
   MCValue AUIPCTarget;
   switch (Fixup.getTargetKind()) {
   default:
-    llvm_unreachable("Unexpected fixup kind!");
+    // Use default handling for `Value` and `IsResolved`.
+    return {};
   case RISCV::fixup_riscv_pcrel_lo12_i:
   case RISCV::fixup_riscv_pcrel_lo12_s: {
     AUIPCFixup =
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h
index d98361ea5857f..292cacdbd45af 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h
@@ -47,8 +47,8 @@ class RISCVAsmBackend : public MCAsmBackend {
   bool shouldInsertFixupForCodeAlign(MCAssembler &Asm,
                                      MCAlignFragment &AF) override;
 
-  bool evaluateTargetFixup(const MCFixup &Fixup, const MCValue &Target,
-                           uint64_t &Value) override;
+  std::optional<bool> evaluateFixup(MCFixup &Fixup, MCValue &Target,
+                                    uint64_t &Value) override;
 
   bool addReloc(const MCFragment &, const MCFixup &, const MCValue &,
                 uint64_t &FixedValue, bool IsResolved);
diff --git a/llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp b/llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp
index b56d8f2d2eb66..14842c4b93e2c 100644
--- a/llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp
+++ b/llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp
@@ -169,7 +169,8 @@ class X86AsmBackend : public MCAsmBackend {
 
   MCFixupKindInfo getFixupKindInfo(MCFixupKind Kind) const override;
 
-  bool shouldForceRelocation(const MCFixup &, const MCValue &);
+  std::optional<bool> evaluateFixup(MCFixup &Fixup, MCValue &Target,
+                                    uint64_t &Value) override;
 
   void applyFixup(const MCFragment &, const MCFixup &, const MCValue &Target,
                   MutableArrayRef<char> Data, uint64_t Value,
@@ -687,6 +688,40 @@ static unsigned getFixupKindSize(unsigned Kind) {
   }
 }
 
+// Adjust PC-relative fixup offsets, which are calculated from the start of the
+// next instruction.
+std::optional<bool>
+X86AsmBackend::evaluateFixup(MCFixup &Fixup, MCValue &Target, uint64_t &Value) {
+  switch (Fixup.getTargetKind()) {
+  case FK_PCRel_1:
+    Target.setConstant(Target.getConstant() - 1);
+    break;
+  case FK_PCRel_2:
+    Target.setConstant(Target.getConstant() - 2);
+    break;
+  case FK_PCRel_4:
+  case X86::reloc_riprel_4byte:
+  case X86::reloc_riprel_4byte_movq_load:
+  case X86::reloc_riprel_4byte_movq_load_rex2:
+  case X86::reloc_riprel_4byte_relax:
+  case X86::reloc_riprel_4byte_relax_rex:
+  case X86::reloc_riprel_4byte_relax_rex2:
+  case X86::reloc_branch_4byte_pcrel:
+  case X86::reloc_riprel_4byte_relax_evex: {
+    Target.setConstant(Target.getConstant() - 4);
+    auto *Add = Target.getAddSym();
+    // If this is a pc-relative load off _GLOBAL_OFFSET_TABLE_:
+    // leaq _GLOBAL_OFFSET_TABLE_(%rip), %r15
+    // this needs to be a GOTPC32 relocation.
+    if (Add && Add->getName() == "_GLOBAL_OFFSET_TABLE_")
+      Fixup = MCFixup::create(Fixup.getOffset(), Fixup.getValue(),
+                              X86::reloc_global_offset_table);
+  } break;
+  }
+  // Use default handling for `Value` and `IsResolved`.
+  return {};
+}
+
 void X86AsmBackend::applyFixup(const MCFragment &F, const MCFixup &Fixup,
                                const MCValue &Target,
                                MutableArrayRef<char> Data, uint64_t Value,
diff --git a/llvm/lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp b/llvm/lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp
index 1666518e3365c..5ef6f14ece9ac 100644
--- a/llvm/lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp
+++ b/llvm/lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp
@@ -576,18 +576,10 @@ void X86MCCodeEmitter::emitImmediate(const MCOperand &DispOp, SMLoc Loc,
     }
   }
 
-  // If the fixup is pc-relative, we need to bias the value to be relative to
-  // the start of the field, not the end of the field.
   bool PCRel = false;
   switch (FixupKind) {
   case FK_PCRel_1:
-    PCRel = true;
-    ImmOffset -= 1;
-    break;
   case FK_PCRel_2:
-    PCRel = true;
-    ImmOffset -= 2;
-    break;
   case FK_PCRel_4:
   case X86::reloc_riprel_4byte:
   case X86::reloc_riprel_4byte_movq_load:
@@ -598,12 +590,6 @@ void X86MCCodeEmitter::emitImmediate(const MCOperand &DispOp, SMLoc Loc,
   case X86::reloc_branch_4byte_pcrel:
   case X86::reloc_riprel_4byte_relax_evex:
     PCRel = true;
-    ImmOffset -= 4;
-    // If this is a pc-relative load off _GLOBAL_OFFSET_TABLE_:
-    // leaq _GLOBAL_OFFSET_TABLE_(%rip), %r15
-    // this needs to be a GOTPC32 relocation.
-    if (startsWithGlobalOffsetTable(Expr) != GOT_None)
-      FixupKind = X86::reloc_global_offset_table;
     break;
   }
 
@@ -611,7 +597,7 @@ void X86MCCodeEmitter::emitImmediate(const MCOperand &DispOp, SMLoc Loc,
     Expr = MCBinaryExpr::createAdd(Expr, MCConstantExpr::create(ImmOffset, Ctx),
                                    Ctx, Expr->getLoc());
 
-  // Emit a symbolic constant as a fixup and 4 zeros.
+  // Emit a symbolic constant as a fixup and a few zero bytes.
   Fixups.push_back(MCFixup::create(static_cast<uint32_t>(CB.size() - StartByte),
                                    Expr, FixupKind, PCRel));
   emitConstant(0, Size, CB);
diff --git a/llvm/test/MC/ELF/mc-dump.s b/llvm/test/MC/ELF/mc-dump.s
index 2286a0d2510c0..ac9506f18b4b5 100644
--- a/llvm/test/MC/ELF/mc-dump.s
+++ b/llvm/test/MC/ELF/mc-dump.s
@@ -13,7 +13,7 @@
 # CHECK-NEXT:  Symbol @0 _start
 # CHECK-NEXT:0 Org Offset:3 Value:0
 # CHECK-NEXT:3 Relaxable Size:2 <MCInst #1996 <MCOperand Expr:.Ltmp0>>
-# CHECK-NEXT:  Fixup @1 Value:.Ltmp0-1 Kind:4006
+# CHECK-NEXT:  Fixup @1 Value:.Ltmp0 Kind:4006
 # CHECK-NEXT:5 Data Size:16 [48,8b,04,25,00,00,00,00,48,8b,04,25,00,00,00,00]
 # CHECK-NEXT:  Fixup @4 Value:f0@<variant 11> Kind:4021
 # CHECK-NEXT:  Fixup @12 Value:_start@<variant 11> Kind:4021
diff --git a/llvm/test/MC/X86/avx-64-att.s b/llvm/test/MC/X86/avx-64-att.s
index 39ee048c3736d..c1e778f92bb96 100644
--- a/llvm/test/MC/X86/avx-64-att.s
+++ b/llvm/test/MC/X86/avx-64-att.s
@@ -4125,7 +4125,7 @@ _foo:
 
 // CHECK: vblendvps %ymm1, _foo2(%rip), %ymm0, %ymm0
 // CHECK: encoding: [0xc4,0xe3,0x7d,0x4a,0x05,A,A,A,A,0x10]
-// CHECK: fixup A - offset: 5, value: _foo2-5
+// CHECK: fixup A - offset: 5, value: _foo2-1
 _foo2:
   nop
   vblendvps %ymm1, _foo2(%rip), %ymm0, %ymm0
diff --git a/llvm/test/MC/X86/fma4-att.s b/llvm/test/MC/X86/fma4-att.s
index c9bd954e90496..55e5ddd09302e 100644
--- a/llvm/test/MC/X86/fma4-att.s
+++ b/llvm/test/MC/X86/fma4-att.s
@@ -80,62 +80,62 @@
 // PR15040
 // CHECK: vfmaddss  foo(%rip), %xmm1, %xmm0, %xmm0
 // CHECK: encoding: [0xc4,0xe3,0xf9,0x6a,0x05,A,A,A,A,0x10]
-// CHECK: fixup A - offset: 5, value: foo-5, kind: reloc_riprel_4byte
+// CHECK: fixup A - offset: 5, value: foo-1, kind: reloc_riprel_4byte
           vfmaddss  foo(%rip), %xmm1, %xmm0, %xmm0
 
 // CHECK: vfmaddss   %xmm1, foo(%rip), %xmm0, %xmm0
 // CHECK: encoding: [0xc4,0xe3,0x79,0x6a,0x05,A,A,A,A,0x10]
-// CHECK: fixup A - offset: 5, value: foo-5, kind: reloc_riprel_4byte
+// CHECK: fixup A - offset: 5, value: foo-1, kind: reloc_riprel_4byte
           vfmaddss   %xmm1, foo(%rip),%xmm0, %xmm0
 
 // CHECK: vfmaddsd  foo(%rip), %xmm1, %xmm0, %xmm0
 // CHECK: encoding: [0xc4,0xe3,0xf9,0x6b,0x05,A,A,A,A,0x10]
-// CHECK: fixup A - offset: 5, value: foo-5, kind: reloc_riprel_4byte
+// CHECK: fixup A - offset: 5, value: foo-1, kind: reloc_riprel_4byte
           vfmaddsd  foo(%rip), %xmm1, %xmm0, %xmm0
 
 // CHECK: vfmaddsd   %xmm1, foo(%rip), %xmm0, %xmm0
 // CHECK: encoding: [0xc4,0xe3,0x79,0x6b,0x05,A,A,A,A,0x10]
-// CHECK: fixup A - offset: 5, value: foo-5, kind: reloc_riprel_4byte
+// CHECK: fixup A - offset: 5, value: foo-1, kind: reloc_riprel_4byte
           vfmaddsd   %xmm1, foo(%rip),%xmm0, %xmm0
 
 // CHECK: vfmaddps  foo(%rip), %xmm1, %xmm0, %xmm0
 // CHECK: encoding: [0xc4,0xe3,0xf9,0x68,0x05,A,A,A,A,0x10]
-// CHECK: fixup A - offset: 5, value: foo-5, kind: reloc_riprel_4byte
+// CHECK: fixup A - offset: 5, value: foo-1, kind: reloc_riprel_4byte
           vfmaddps  foo(%rip), %xmm1, %xmm0, %xmm0
 
 // CHECK: vfmaddps   %xmm1, foo(%rip), %xmm0, %xmm0
 // CHECK: encoding: [0xc4,0xe3,0x79,0x68,0x05,A,A,A,A,0x10]
-// CHECK: fixup A - offset: 5, value: foo-5, kind: reloc_riprel_4byte
+// CHECK: fixup A - offset: 5, value: foo-1, kind: reloc_riprel_4byte
           vfmaddps   %xmm1, foo(%rip),%xmm0, %xmm0
 
 // CHECK: vfmaddpd  foo(%rip), %xmm1, %xmm0, %xmm0
 // CHECK: encoding: [0xc4,0xe3,0xf9,0x69,0x05,A,A,A,A,0x10]
-// CHECK: fixup A - offset: 5, value: foo-5, kind: reloc_riprel_4byte
+// CHECK: fixup A - offset: 5, value: foo-1, kind: reloc_riprel_4byte
           vfmaddpd  foo(%rip), %xmm1, %xmm0, %xmm0
 
 // CHECK: vfmaddpd   %xmm1, foo(%rip), %xmm0, %xmm0
 // CHECK: encoding: [0xc4,0xe3,0x79,0x69,0x05,A,A,A,A,0x10]
-// CHECK: fixup A - offset: 5, value: foo-5, kind: reloc_riprel_4byte
+// CHECK: fixup A - offset: 5, value: foo-1, kind: reloc_riprel_4byte
           vfmaddpd   %xmm1, foo(%rip),%xmm0, %xmm0
 
 // CHECK: vfmaddps  foo(%rip), %ymm1, %ymm0, %ymm0
 // CHECK: encoding: [0xc4,0xe3,0xfd,0x68,0x05,A,A,A,A,0x10]
-// CHECK: fixup A - offset: 5, value: foo-5, kind: reloc_riprel_4byte
+// CHECK: fixup A - offset: 5, value: foo-1, kind: reloc_riprel_4byte
           vfmaddps  foo(%rip), %ymm1, %ymm0, %ymm0
 
 // CHECK: vfmaddps   %ymm1, foo(%rip), %ymm0, %ymm0
 // CHECK: encoding: [0xc4,0xe3,0x7d,0x68,0x05,A,A,A,A,0x10]
-// CHECK: fixup A - offset: 5, value: foo-5, kind: reloc_riprel_4byte
+// CHECK: fixup A - offset: 5, value: foo-1, kind: reloc_riprel_4byte
           vfmaddps   %ymm1, foo(%rip),%ymm0, %ymm0
 
 // CHECK: vfmaddpd  foo(%rip), %ymm1, %ymm0, %ymm0
 // CHECK: encoding: [0xc4,0xe3,0xfd,0x69,0x05,A,A,A,A,0x10]
-// CHECK: fixup A - offset: 5, value: foo-5, kind: reloc_riprel_4byte
+// CHECK: fixup A - offset: 5, value: foo-1, kind: reloc_riprel_4byte
           vfmaddpd  foo(%rip), %ymm1, %ymm0, %ymm0
 
 // CHECK: vfmaddpd   %ymm1, foo(%rip), %ymm0, %ymm0
 // CHECK: encoding: [0xc4,0xe3,0x7d,0x69,0x05,A,A,A,A,0x10]
-// CHECK: fixup A - offset: 5, value: foo-5, kind: reloc_riprel_4byte
+// CHECK: fixup A - offset: 5, value: foo-1, kind: reloc_riprel_4byte
           vfmaddpd   %ymm1, foo(%rip),%ymm0, %ymm0
 
 // vfmsub
diff --git a/llvm/test/MC/X86/x86-16.s b/llvm/test/MC/X86/x86-16.s
index be0563e5b1d63..9a1963760c99a 100644
--- a/llvm/test/MC/X86/x86-16.s
+++ b/llvm/test/MC/X86/x86-16.s
@@ -1058,13 +1058,13 @@ xresldtrk
 
 // CHECK: jmp foo
 // CHECK:  encoding: [0xe9,A,A]
-// CHECK:  fixup A - offset: 1, value: foo-2, kind: FK_PCRel_2
+// CHECK:  fixup A - offset: 1, value: foo, kind: FK_PCRel_2
 {disp32} jmp foo
 foo:
 
 // CHECK: je foo
 // CHECK:  encoding: [0x0f,0x84,A,A]
-// CHECK:  fixup A - offset: 2, value: foo-2, kind: FK_PCRel_2
+// CHECK:  fixup A - offset: 2, value: foo, kind: FK_PCRel_2
 {disp32} je foo
 
 // CHECK: movl nearer, %ebx
diff --git a/llvm/test/MC/X86/x86-32.s b/llvm/test/MC/X86/x86-32.s
index e3d473a05e59d..2b9dc921844b1 100644
--- a/llvm/test/MC/X86/x86-32.s
+++ b/llvm/test/MC/X86/x86-32.s
@@ -1120,20 +1120,20 @@ ptwritel %eax
 
 // CHECK: jmp foo
 // CHECK:  encoding: [0xe9,A,A,A,A]
-// CHECK:  fixup A - offset: 1, value: foo-4, kind: FK_PCRel_4
+// CHECK:  fixup A - offset: 1, value: foo, kind: FK_PCRel_4
 // CHECK: jmp foo
 // CHECK:  encoding: [0xe9,A,A,A,A]
-// CHECK:  fixup A - offset: 1, value: foo-4, kind: FK_PCRel_4
+// CHECK:  fixup A - offset: 1, value: foo, kind: FK_PCRel_4
 {disp32} jmp foo
 jmp.d32 foo
 foo:
 
 // CHECK: je foo
 // CHECK:  encoding: [0x0f,0x84,A,A,A,A]
-// CHECK:  fixup A - offset: 2, value: foo-4, kind: FK_PCRel_4
+// CHECK:  fixup A - offset: 2, value: foo, kind: FK_PCRel_4
 // CHECK: je foo
 // CHECK:  encoding: [0x0f,0x84,A,A,A,A]
-// CHECK:  fixup A - offset: 2, value: foo-4, kind: FK_PCRel_4
+// CHECK:  fixup A - offset: 2, value: foo, kind: FK_PCRel_4
 {disp32} je foo
 je.d32 foo
 
diff --git a/llvm/test/MC/X86/x86-64.s b/llvm/test/MC/X86/x86-64.s
index 108d1220107e3..657792a610318 100644
--- a/llvm/test/MC/X86/x86-64.s
+++ b/llvm/test/MC/X86/x86-64.s
@@ -600,22 +600,22 @@ fdivp %st(1), %st // CHECK: encoding: [0xde,0xf1]
 movl	foo(%rip), %eax
 // CHECK: movl	foo(%rip), %eax
 // CHECK: encoding: [0x8b,0x05,A,A,A,A]
-// CHECK: fixup A - offset: 2, value: foo-4, kind: reloc_riprel_4byte
+// CHECK: fixup A - offset: 2, value: foo, kind: reloc_riprel_4byte
 
 movb	$12, foo(%rip)
 // CHECK: movb	$12, foo(%rip)
 // CHECK: encoding: [0xc6,0x05,A,A,A,A,0x0c]
-// CHECK:    fixup A - offset: 2, value: foo-5, kind: reloc_riprel_4byte
+// CHECK:    fixup A - offset: 2, value: foo-1, kind: reloc_riprel_4byte
 
 movw	$12, foo(%rip)
 // CHECK: movw	$12, foo(%rip)
 // CHECK: encoding: [0x66,0xc7,0x05,A,A,A,A,0x0c,0x00]
-// CHECK:    fixup A - offset: 3, value: foo-6, kind: reloc_riprel_4byte
+// CHECK:    fixup A - offset: 3, value: foo-2, kind: reloc_riprel_4byte
 
 movl	$12, foo(%rip)
 // CHECK: movl	$12, foo(%rip)
 // CHECK: encoding: [0xc7,0x05,A,A,A,A,0x0c,0x00,0x00,0x00]
-// CHECK:    fixup A - offset: 2, value: foo-8, kind: reloc_riprel_4byte
+// CHECK:    fixup A - offset: 2, value: foo-4, kind: reloc_riprel_4byte
 
 // rdar://37247000
 movl	$12, 1024(%rip)
@@ -625,32 +625,32 @@ movl	$12, 1024(%rip)
 movq	$12, foo(%rip)
 // CHECK:  movq	$12, foo(%rip)
 // CHECK: encoding: [0x48,0xc7,0x05,A,A,A,A,0x0c,0x00,0x00,0x00]
-// CHECK:    fixup A - offset: 3, value: foo-8, kind: reloc_riprel_4byte
+// CHECK:    fixup A - offset: 3, value: foo-4, kind: reloc_riprel_4byte
 
 movl	foo(%eip), %eax
 // CHECK: movl	foo(%eip), %eax
 // CHECK: encoding: [0x67,0x8b,0x05,A,A,A,A]
-// CHECK: fixup A - offset: 3, value: foo-4, kind: reloc_riprel_4byte
+// CHECK: fixup A - offset: 3, value: foo, kind: reloc_riprel_4byte
 
 movb	$12, foo(%eip)
 // CHECK: movb	$12, foo(%eip)
 // CHECK: encoding: [0x67,0xc6,0x05,A,A,A,A,0x0c]
-// CHECK:    fixup A - offset: 3, value: foo-5, kind: reloc_riprel_4byte
+// CHECK:    fixup A - offset: 3, value: foo-1, kind: reloc_riprel_4byte
 
 movw	$12, foo(%eip)
 // CHECK: movw	$12, foo(%eip)
 // CHECK: encoding: [0x67,0x66,0xc7,0x05,A,A,A,A,0x0c,0x00]
-// CHECK:    fixup A - offset: 4, value: foo-6, kind: reloc_riprel_4byte
+// CHECK:    fixup A - offset: 4, value: foo-2, kind: reloc_r...
[truncated]

@llvmbot
Copy link
Member

llvmbot commented Jul 4, 2025

@llvm/pr-subscribers-backend-x86

Author: Fangrui Song (MaskRay)

Changes

Move the X86 PC-relative fixup adjustment from
X86MCCodeEmitter::emitImmediate to MCAssembler, leveraging a generalized
evaluateFixup. This saves a MCBinaryExpr. For call foo, the fixup
expression is now foo instead of foo-4. Updated X86 MC tests confirm
no change in generated relocations.

In bolt/lib/Target/X86/X86MCPlusBuilder.cpp, createRelocation needs to
decrease the addend.

Both max-rss and instructions:u show a minor decrease.
https://llvm-compile-time-tracker.com/compare.php?from=ea600576a6f94d6f28925c4b99962cc26b463c29&amp;to=016e8fd4ddf851e5555f606c6394241d68f1a7bb&amp;stat=max-rss&amp;linkStats=on

Next: Update targets that use FKF_IsAlignedDownTo32Bits to define
evaluateFixup and remove FKF_IsAlignedDownTo32Bits from the generic
code.


Patch is 22.41 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/147113.diff

15 Files Affected:

  • (modified) bolt/lib/Target/X86/X86MCPlusBuilder.cpp (+5-3)
  • (modified) llvm/include/llvm/MC/MCAsmBackend.h (+6-3)
  • (modified) llvm/include/llvm/MC/MCValue.h (+1)
  • (modified) llvm/lib/MC/MCAssembler.cpp (+7-7)
  • (modified) llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp (+7-8)
  • (modified) llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h (+2-2)
  • (modified) llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp (+36-1)
  • (modified) llvm/lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp (+1-15)
  • (modified) llvm/test/MC/ELF/mc-dump.s (+1-1)
  • (modified) llvm/test/MC/X86/avx-64-att.s (+1-1)
  • (modified) llvm/test/MC/X86/fma4-att.s (+12-12)
  • (modified) llvm/test/MC/X86/x86-16.s (+2-2)
  • (modified) llvm/test/MC/X86/x86-32.s (+4-4)
  • (modified) llvm/test/MC/X86/x86-64.s (+14-14)
  • (modified) llvm/test/MC/X86/x86_64-encoding.s (+1-1)
diff --git a/bolt/lib/Target/X86/X86MCPlusBuilder.cpp b/bolt/lib/Target/X86/X86MCPlusBuilder.cpp
index a91fd146fa367..21d7562d41572 100644
--- a/bolt/lib/Target/X86/X86MCPlusBuilder.cpp
+++ b/bolt/lib/Target/X86/X86MCPlusBuilder.cpp
@@ -2442,9 +2442,10 @@ class X86MCPlusBuilder : public MCPlusBuilder {
 
     assert(FKI.TargetOffset == 0 && "0-bit relocation offset expected");
     const uint64_t RelOffset = Fixup.getOffset();
+    auto [RelSymbol, RelAddend] = extractFixupExpr(Fixup);
 
     uint32_t RelType;
-    if (FKI.Flags & MCFixupKindInfo::FKF_IsPCRel) {
+    if (Fixup.isPCRel()) {
       switch (FKI.TargetSize) {
       default:
         return std::nullopt;
@@ -2453,6 +2454,9 @@ class X86MCPlusBuilder : public MCPlusBuilder {
       case 32: RelType = ELF::R_X86_64_PC32; break;
       case 64: RelType = ELF::R_X86_64_PC64; break;
       }
+      // Adjust PC-relative fixup offsets, which are calculated from the start
+      // of the next instruction.
+      RelAddend -= FKI.TargetSize / 8;
     } else {
       switch (FKI.TargetSize) {
       default:
@@ -2464,8 +2468,6 @@ class X86MCPlusBuilder : public MCPlusBuilder {
       }
     }
 
-    auto [RelSymbol, RelAddend] = extractFixupExpr(Fixup);
-
     return Relocation({RelOffset, RelSymbol, RelType, RelAddend, 0});
   }
 
diff --git a/llvm/include/llvm/MC/MCAsmBackend.h b/llvm/include/llvm/MC/MCAsmBackend.h
index 54e43b89b7b00..46547560b83ed 100644
--- a/llvm/include/llvm/MC/MCAsmBackend.h
+++ b/llvm/include/llvm/MC/MCAsmBackend.h
@@ -106,9 +106,12 @@ class LLVM_ABI MCAsmBackend {
     return false;
   }
 
-  virtual bool evaluateTargetFixup(const MCFixup &Fixup, const MCValue &Target,
-                                   uint64_t &Value) {
-    llvm_unreachable("Need to implement hook if target has custom fixups");
+  // Evaluate a fixup, returning std::nullopt to use default handling for
+  // `Value` and `IsResolved`. Otherwise, returns `IsResolved` with the
+  // expectation that the hook updates `Value`.
+  virtual std::optional<bool> evaluateFixup(MCFixup &Fixup, MCValue &Target,
+                                            uint64_t &Value) {
+    return {};
   }
 
   void maybeAddReloc(const MCFragment &, const MCFixup &, const MCValue &,
diff --git a/llvm/include/llvm/MC/MCValue.h b/llvm/include/llvm/MC/MCValue.h
index 417c4689d1b47..c9a680ec327a6 100644
--- a/llvm/include/llvm/MC/MCValue.h
+++ b/llvm/include/llvm/MC/MCValue.h
@@ -42,6 +42,7 @@ class MCValue {
   friend class MCExpr;
   MCValue() = default;
   int64_t getConstant() const { return Cst; }
+  void setConstant(int64_t C) { Cst = C; }
   uint32_t getSpecifier() const { return Specifier; }
   void setSpecifier(uint32_t S) { Specifier = S; }
 
diff --git a/llvm/lib/MC/MCAssembler.cpp b/llvm/lib/MC/MCAssembler.cpp
index 4b7845335d822..69192db2a20d3 100644
--- a/llvm/lib/MC/MCAssembler.cpp
+++ b/llvm/lib/MC/MCAssembler.cpp
@@ -161,11 +161,13 @@ bool MCAssembler::evaluateFixup(const MCFragment &F, MCFixup &Fixup,
     return true;
   }
 
-  bool IsResolved = false;
+  // TODO: Require targets to set PCRel at fixup creation time.
   unsigned FixupFlags = getBackend().getFixupKindInfo(Fixup.getKind()).Flags;
-  bool IsPCRel = FixupFlags & MCFixupKindInfo::FKF_IsPCRel;
-  if (FixupFlags & MCFixupKindInfo::FKF_IsTarget) {
-    IsResolved = getBackend().evaluateTargetFixup(Fixup, Target, Value);
+  if (FixupFlags & MCFixupKindInfo::FKF_IsPCRel)
+    Fixup.setPCRel();
+  bool IsResolved = false;
+  if (auto State = getBackend().evaluateFixup(Fixup, Target, Value)) {
+    IsResolved = *State;
   } else {
     const MCSymbol *Add = Target.getAddSym();
     const MCSymbol *Sub = Target.getSubSym();
@@ -177,7 +179,7 @@ bool MCAssembler::evaluateFixup(const MCFragment &F, MCFixup &Fixup,
 
     bool ShouldAlignPC =
         FixupFlags & MCFixupKindInfo::FKF_IsAlignedDownTo32Bits;
-    if (IsPCRel) {
+    if (Fixup.isPCRel()) {
       uint64_t Offset = getFragmentOffset(F) + Fixup.getOffset();
 
       // A number of ARM fixups in Thumb mode require that the effective PC
@@ -202,8 +204,6 @@ bool MCAssembler::evaluateFixup(const MCFragment &F, MCFixup &Fixup,
 
   if (IsResolved && mc::isRelocRelocation(Fixup.getKind()))
     IsResolved = false;
-  if (IsPCRel)
-    Fixup.setPCRel();
   getBackend().applyFixup(F, Fixup, Target, Contents, Value, IsResolved);
   return true;
 }
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
index d298ce769c1dc..79ada59e7a524 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
@@ -70,10 +70,8 @@ MCFixupKindInfo RISCVAsmBackend::getFixupKindInfo(MCFixupKind Kind) const {
       {"fixup_riscv_12_i", 20, 12, 0},
       {"fixup_riscv_lo12_s", 0, 32, 0},
       {"fixup_riscv_pcrel_hi20", 12, 20, MCFixupKindInfo::FKF_IsPCRel},
-      {"fixup_riscv_pcrel_lo12_i", 20, 12,
-       MCFixupKindInfo::FKF_IsPCRel | MCFixupKindInfo::FKF_IsTarget},
-      {"fixup_riscv_pcrel_lo12_s", 0, 32,
-       MCFixupKindInfo::FKF_IsPCRel | MCFixupKindInfo::FKF_IsTarget},
+      {"fixup_riscv_pcrel_lo12_i", 20, 12, MCFixupKindInfo::FKF_IsPCRel},
+      {"fixup_riscv_pcrel_lo12_s", 0, 32, MCFixupKindInfo::FKF_IsPCRel},
       {"fixup_riscv_jal", 12, 20, MCFixupKindInfo::FKF_IsPCRel},
       {"fixup_riscv_branch", 0, 32, MCFixupKindInfo::FKF_IsPCRel},
       {"fixup_riscv_rvc_jump", 2, 11, MCFixupKindInfo::FKF_IsPCRel},
@@ -657,15 +655,16 @@ static const MCFixup *getPCRelHiFixup(const MCSpecifierExpr &Expr,
   return nullptr;
 }
 
-bool RISCVAsmBackend::evaluateTargetFixup(const MCFixup &Fixup,
-                                          const MCValue &Target,
-                                          uint64_t &Value) {
+std::optional<bool> RISCVAsmBackend::evaluateFixup(MCFixup &Fixup,
+                                                   MCValue &Target,
+                                                   uint64_t &Value) {
   const MCFixup *AUIPCFixup;
   const MCFragment *AUIPCDF;
   MCValue AUIPCTarget;
   switch (Fixup.getTargetKind()) {
   default:
-    llvm_unreachable("Unexpected fixup kind!");
+    // Use default handling for `Value` and `IsResolved`.
+    return {};
   case RISCV::fixup_riscv_pcrel_lo12_i:
   case RISCV::fixup_riscv_pcrel_lo12_s: {
     AUIPCFixup =
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h
index d98361ea5857f..292cacdbd45af 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h
@@ -47,8 +47,8 @@ class RISCVAsmBackend : public MCAsmBackend {
   bool shouldInsertFixupForCodeAlign(MCAssembler &Asm,
                                      MCAlignFragment &AF) override;
 
-  bool evaluateTargetFixup(const MCFixup &Fixup, const MCValue &Target,
-                           uint64_t &Value) override;
+  std::optional<bool> evaluateFixup(MCFixup &Fixup, MCValue &Target,
+                                    uint64_t &Value) override;
 
   bool addReloc(const MCFragment &, const MCFixup &, const MCValue &,
                 uint64_t &FixedValue, bool IsResolved);
diff --git a/llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp b/llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp
index b56d8f2d2eb66..14842c4b93e2c 100644
--- a/llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp
+++ b/llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp
@@ -169,7 +169,8 @@ class X86AsmBackend : public MCAsmBackend {
 
   MCFixupKindInfo getFixupKindInfo(MCFixupKind Kind) const override;
 
-  bool shouldForceRelocation(const MCFixup &, const MCValue &);
+  std::optional<bool> evaluateFixup(MCFixup &Fixup, MCValue &Target,
+                                    uint64_t &Value) override;
 
   void applyFixup(const MCFragment &, const MCFixup &, const MCValue &Target,
                   MutableArrayRef<char> Data, uint64_t Value,
@@ -687,6 +688,40 @@ static unsigned getFixupKindSize(unsigned Kind) {
   }
 }
 
+// Adjust PC-relative fixup offsets, which are calculated from the start of the
+// next instruction.
+std::optional<bool>
+X86AsmBackend::evaluateFixup(MCFixup &Fixup, MCValue &Target, uint64_t &Value) {
+  switch (Fixup.getTargetKind()) {
+  case FK_PCRel_1:
+    Target.setConstant(Target.getConstant() - 1);
+    break;
+  case FK_PCRel_2:
+    Target.setConstant(Target.getConstant() - 2);
+    break;
+  case FK_PCRel_4:
+  case X86::reloc_riprel_4byte:
+  case X86::reloc_riprel_4byte_movq_load:
+  case X86::reloc_riprel_4byte_movq_load_rex2:
+  case X86::reloc_riprel_4byte_relax:
+  case X86::reloc_riprel_4byte_relax_rex:
+  case X86::reloc_riprel_4byte_relax_rex2:
+  case X86::reloc_branch_4byte_pcrel:
+  case X86::reloc_riprel_4byte_relax_evex: {
+    Target.setConstant(Target.getConstant() - 4);
+    auto *Add = Target.getAddSym();
+    // If this is a pc-relative load off _GLOBAL_OFFSET_TABLE_:
+    // leaq _GLOBAL_OFFSET_TABLE_(%rip), %r15
+    // this needs to be a GOTPC32 relocation.
+    if (Add && Add->getName() == "_GLOBAL_OFFSET_TABLE_")
+      Fixup = MCFixup::create(Fixup.getOffset(), Fixup.getValue(),
+                              X86::reloc_global_offset_table);
+  } break;
+  }
+  // Use default handling for `Value` and `IsResolved`.
+  return {};
+}
+
 void X86AsmBackend::applyFixup(const MCFragment &F, const MCFixup &Fixup,
                                const MCValue &Target,
                                MutableArrayRef<char> Data, uint64_t Value,
diff --git a/llvm/lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp b/llvm/lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp
index 1666518e3365c..5ef6f14ece9ac 100644
--- a/llvm/lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp
+++ b/llvm/lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp
@@ -576,18 +576,10 @@ void X86MCCodeEmitter::emitImmediate(const MCOperand &DispOp, SMLoc Loc,
     }
   }
 
-  // If the fixup is pc-relative, we need to bias the value to be relative to
-  // the start of the field, not the end of the field.
   bool PCRel = false;
   switch (FixupKind) {
   case FK_PCRel_1:
-    PCRel = true;
-    ImmOffset -= 1;
-    break;
   case FK_PCRel_2:
-    PCRel = true;
-    ImmOffset -= 2;
-    break;
   case FK_PCRel_4:
   case X86::reloc_riprel_4byte:
   case X86::reloc_riprel_4byte_movq_load:
@@ -598,12 +590,6 @@ void X86MCCodeEmitter::emitImmediate(const MCOperand &DispOp, SMLoc Loc,
   case X86::reloc_branch_4byte_pcrel:
   case X86::reloc_riprel_4byte_relax_evex:
     PCRel = true;
-    ImmOffset -= 4;
-    // If this is a pc-relative load off _GLOBAL_OFFSET_TABLE_:
-    // leaq _GLOBAL_OFFSET_TABLE_(%rip), %r15
-    // this needs to be a GOTPC32 relocation.
-    if (startsWithGlobalOffsetTable(Expr) != GOT_None)
-      FixupKind = X86::reloc_global_offset_table;
     break;
   }
 
@@ -611,7 +597,7 @@ void X86MCCodeEmitter::emitImmediate(const MCOperand &DispOp, SMLoc Loc,
     Expr = MCBinaryExpr::createAdd(Expr, MCConstantExpr::create(ImmOffset, Ctx),
                                    Ctx, Expr->getLoc());
 
-  // Emit a symbolic constant as a fixup and 4 zeros.
+  // Emit a symbolic constant as a fixup and a few zero bytes.
   Fixups.push_back(MCFixup::create(static_cast<uint32_t>(CB.size() - StartByte),
                                    Expr, FixupKind, PCRel));
   emitConstant(0, Size, CB);
diff --git a/llvm/test/MC/ELF/mc-dump.s b/llvm/test/MC/ELF/mc-dump.s
index 2286a0d2510c0..ac9506f18b4b5 100644
--- a/llvm/test/MC/ELF/mc-dump.s
+++ b/llvm/test/MC/ELF/mc-dump.s
@@ -13,7 +13,7 @@
 # CHECK-NEXT:  Symbol @0 _start
 # CHECK-NEXT:0 Org Offset:3 Value:0
 # CHECK-NEXT:3 Relaxable Size:2 <MCInst #1996 <MCOperand Expr:.Ltmp0>>
-# CHECK-NEXT:  Fixup @1 Value:.Ltmp0-1 Kind:4006
+# CHECK-NEXT:  Fixup @1 Value:.Ltmp0 Kind:4006
 # CHECK-NEXT:5 Data Size:16 [48,8b,04,25,00,00,00,00,48,8b,04,25,00,00,00,00]
 # CHECK-NEXT:  Fixup @4 Value:f0@<variant 11> Kind:4021
 # CHECK-NEXT:  Fixup @12 Value:_start@<variant 11> Kind:4021
diff --git a/llvm/test/MC/X86/avx-64-att.s b/llvm/test/MC/X86/avx-64-att.s
index 39ee048c3736d..c1e778f92bb96 100644
--- a/llvm/test/MC/X86/avx-64-att.s
+++ b/llvm/test/MC/X86/avx-64-att.s
@@ -4125,7 +4125,7 @@ _foo:
 
 // CHECK: vblendvps %ymm1, _foo2(%rip), %ymm0, %ymm0
 // CHECK: encoding: [0xc4,0xe3,0x7d,0x4a,0x05,A,A,A,A,0x10]
-// CHECK: fixup A - offset: 5, value: _foo2-5
+// CHECK: fixup A - offset: 5, value: _foo2-1
 _foo2:
   nop
   vblendvps %ymm1, _foo2(%rip), %ymm0, %ymm0
diff --git a/llvm/test/MC/X86/fma4-att.s b/llvm/test/MC/X86/fma4-att.s
index c9bd954e90496..55e5ddd09302e 100644
--- a/llvm/test/MC/X86/fma4-att.s
+++ b/llvm/test/MC/X86/fma4-att.s
@@ -80,62 +80,62 @@
 // PR15040
 // CHECK: vfmaddss  foo(%rip), %xmm1, %xmm0, %xmm0
 // CHECK: encoding: [0xc4,0xe3,0xf9,0x6a,0x05,A,A,A,A,0x10]
-// CHECK: fixup A - offset: 5, value: foo-5, kind: reloc_riprel_4byte
+// CHECK: fixup A - offset: 5, value: foo-1, kind: reloc_riprel_4byte
           vfmaddss  foo(%rip), %xmm1, %xmm0, %xmm0
 
 // CHECK: vfmaddss   %xmm1, foo(%rip), %xmm0, %xmm0
 // CHECK: encoding: [0xc4,0xe3,0x79,0x6a,0x05,A,A,A,A,0x10]
-// CHECK: fixup A - offset: 5, value: foo-5, kind: reloc_riprel_4byte
+// CHECK: fixup A - offset: 5, value: foo-1, kind: reloc_riprel_4byte
           vfmaddss   %xmm1, foo(%rip),%xmm0, %xmm0
 
 // CHECK: vfmaddsd  foo(%rip), %xmm1, %xmm0, %xmm0
 // CHECK: encoding: [0xc4,0xe3,0xf9,0x6b,0x05,A,A,A,A,0x10]
-// CHECK: fixup A - offset: 5, value: foo-5, kind: reloc_riprel_4byte
+// CHECK: fixup A - offset: 5, value: foo-1, kind: reloc_riprel_4byte
           vfmaddsd  foo(%rip), %xmm1, %xmm0, %xmm0
 
 // CHECK: vfmaddsd   %xmm1, foo(%rip), %xmm0, %xmm0
 // CHECK: encoding: [0xc4,0xe3,0x79,0x6b,0x05,A,A,A,A,0x10]
-// CHECK: fixup A - offset: 5, value: foo-5, kind: reloc_riprel_4byte
+// CHECK: fixup A - offset: 5, value: foo-1, kind: reloc_riprel_4byte
           vfmaddsd   %xmm1, foo(%rip),%xmm0, %xmm0
 
 // CHECK: vfmaddps  foo(%rip), %xmm1, %xmm0, %xmm0
 // CHECK: encoding: [0xc4,0xe3,0xf9,0x68,0x05,A,A,A,A,0x10]
-// CHECK: fixup A - offset: 5, value: foo-5, kind: reloc_riprel_4byte
+// CHECK: fixup A - offset: 5, value: foo-1, kind: reloc_riprel_4byte
           vfmaddps  foo(%rip), %xmm1, %xmm0, %xmm0
 
 // CHECK: vfmaddps   %xmm1, foo(%rip), %xmm0, %xmm0
 // CHECK: encoding: [0xc4,0xe3,0x79,0x68,0x05,A,A,A,A,0x10]
-// CHECK: fixup A - offset: 5, value: foo-5, kind: reloc_riprel_4byte
+// CHECK: fixup A - offset: 5, value: foo-1, kind: reloc_riprel_4byte
           vfmaddps   %xmm1, foo(%rip),%xmm0, %xmm0
 
 // CHECK: vfmaddpd  foo(%rip), %xmm1, %xmm0, %xmm0
 // CHECK: encoding: [0xc4,0xe3,0xf9,0x69,0x05,A,A,A,A,0x10]
-// CHECK: fixup A - offset: 5, value: foo-5, kind: reloc_riprel_4byte
+// CHECK: fixup A - offset: 5, value: foo-1, kind: reloc_riprel_4byte
           vfmaddpd  foo(%rip), %xmm1, %xmm0, %xmm0
 
 // CHECK: vfmaddpd   %xmm1, foo(%rip), %xmm0, %xmm0
 // CHECK: encoding: [0xc4,0xe3,0x79,0x69,0x05,A,A,A,A,0x10]
-// CHECK: fixup A - offset: 5, value: foo-5, kind: reloc_riprel_4byte
+// CHECK: fixup A - offset: 5, value: foo-1, kind: reloc_riprel_4byte
           vfmaddpd   %xmm1, foo(%rip),%xmm0, %xmm0
 
 // CHECK: vfmaddps  foo(%rip), %ymm1, %ymm0, %ymm0
 // CHECK: encoding: [0xc4,0xe3,0xfd,0x68,0x05,A,A,A,A,0x10]
-// CHECK: fixup A - offset: 5, value: foo-5, kind: reloc_riprel_4byte
+// CHECK: fixup A - offset: 5, value: foo-1, kind: reloc_riprel_4byte
           vfmaddps  foo(%rip), %ymm1, %ymm0, %ymm0
 
 // CHECK: vfmaddps   %ymm1, foo(%rip), %ymm0, %ymm0
 // CHECK: encoding: [0xc4,0xe3,0x7d,0x68,0x05,A,A,A,A,0x10]
-// CHECK: fixup A - offset: 5, value: foo-5, kind: reloc_riprel_4byte
+// CHECK: fixup A - offset: 5, value: foo-1, kind: reloc_riprel_4byte
           vfmaddps   %ymm1, foo(%rip),%ymm0, %ymm0
 
 // CHECK: vfmaddpd  foo(%rip), %ymm1, %ymm0, %ymm0
 // CHECK: encoding: [0xc4,0xe3,0xfd,0x69,0x05,A,A,A,A,0x10]
-// CHECK: fixup A - offset: 5, value: foo-5, kind: reloc_riprel_4byte
+// CHECK: fixup A - offset: 5, value: foo-1, kind: reloc_riprel_4byte
           vfmaddpd  foo(%rip), %ymm1, %ymm0, %ymm0
 
 // CHECK: vfmaddpd   %ymm1, foo(%rip), %ymm0, %ymm0
 // CHECK: encoding: [0xc4,0xe3,0x7d,0x69,0x05,A,A,A,A,0x10]
-// CHECK: fixup A - offset: 5, value: foo-5, kind: reloc_riprel_4byte
+// CHECK: fixup A - offset: 5, value: foo-1, kind: reloc_riprel_4byte
           vfmaddpd   %ymm1, foo(%rip),%ymm0, %ymm0
 
 // vfmsub
diff --git a/llvm/test/MC/X86/x86-16.s b/llvm/test/MC/X86/x86-16.s
index be0563e5b1d63..9a1963760c99a 100644
--- a/llvm/test/MC/X86/x86-16.s
+++ b/llvm/test/MC/X86/x86-16.s
@@ -1058,13 +1058,13 @@ xresldtrk
 
 // CHECK: jmp foo
 // CHECK:  encoding: [0xe9,A,A]
-// CHECK:  fixup A - offset: 1, value: foo-2, kind: FK_PCRel_2
+// CHECK:  fixup A - offset: 1, value: foo, kind: FK_PCRel_2
 {disp32} jmp foo
 foo:
 
 // CHECK: je foo
 // CHECK:  encoding: [0x0f,0x84,A,A]
-// CHECK:  fixup A - offset: 2, value: foo-2, kind: FK_PCRel_2
+// CHECK:  fixup A - offset: 2, value: foo, kind: FK_PCRel_2
 {disp32} je foo
 
 // CHECK: movl nearer, %ebx
diff --git a/llvm/test/MC/X86/x86-32.s b/llvm/test/MC/X86/x86-32.s
index e3d473a05e59d..2b9dc921844b1 100644
--- a/llvm/test/MC/X86/x86-32.s
+++ b/llvm/test/MC/X86/x86-32.s
@@ -1120,20 +1120,20 @@ ptwritel %eax
 
 // CHECK: jmp foo
 // CHECK:  encoding: [0xe9,A,A,A,A]
-// CHECK:  fixup A - offset: 1, value: foo-4, kind: FK_PCRel_4
+// CHECK:  fixup A - offset: 1, value: foo, kind: FK_PCRel_4
 // CHECK: jmp foo
 // CHECK:  encoding: [0xe9,A,A,A,A]
-// CHECK:  fixup A - offset: 1, value: foo-4, kind: FK_PCRel_4
+// CHECK:  fixup A - offset: 1, value: foo, kind: FK_PCRel_4
 {disp32} jmp foo
 jmp.d32 foo
 foo:
 
 // CHECK: je foo
 // CHECK:  encoding: [0x0f,0x84,A,A,A,A]
-// CHECK:  fixup A - offset: 2, value: foo-4, kind: FK_PCRel_4
+// CHECK:  fixup A - offset: 2, value: foo, kind: FK_PCRel_4
 // CHECK: je foo
 // CHECK:  encoding: [0x0f,0x84,A,A,A,A]
-// CHECK:  fixup A - offset: 2, value: foo-4, kind: FK_PCRel_4
+// CHECK:  fixup A - offset: 2, value: foo, kind: FK_PCRel_4
 {disp32} je foo
 je.d32 foo
 
diff --git a/llvm/test/MC/X86/x86-64.s b/llvm/test/MC/X86/x86-64.s
index 108d1220107e3..657792a610318 100644
--- a/llvm/test/MC/X86/x86-64.s
+++ b/llvm/test/MC/X86/x86-64.s
@@ -600,22 +600,22 @@ fdivp %st(1), %st // CHECK: encoding: [0xde,0xf1]
 movl	foo(%rip), %eax
 // CHECK: movl	foo(%rip), %eax
 // CHECK: encoding: [0x8b,0x05,A,A,A,A]
-// CHECK: fixup A - offset: 2, value: foo-4, kind: reloc_riprel_4byte
+// CHECK: fixup A - offset: 2, value: foo, kind: reloc_riprel_4byte
 
 movb	$12, foo(%rip)
 // CHECK: movb	$12, foo(%rip)
 // CHECK: encoding: [0xc6,0x05,A,A,A,A,0x0c]
-// CHECK:    fixup A - offset: 2, value: foo-5, kind: reloc_riprel_4byte
+// CHECK:    fixup A - offset: 2, value: foo-1, kind: reloc_riprel_4byte
 
 movw	$12, foo(%rip)
 // CHECK: movw	$12, foo(%rip)
 // CHECK: encoding: [0x66,0xc7,0x05,A,A,A,A,0x0c,0x00]
-// CHECK:    fixup A - offset: 3, value: foo-6, kind: reloc_riprel_4byte
+// CHECK:    fixup A - offset: 3, value: foo-2, kind: reloc_riprel_4byte
 
 movl	$12, foo(%rip)
 // CHECK: movl	$12, foo(%rip)
 // CHECK: encoding: [0xc7,0x05,A,A,A,A,0x0c,0x00,0x00,0x00]
-// CHECK:    fixup A - offset: 2, value: foo-8, kind: reloc_riprel_4byte
+// CHECK:    fixup A - offset: 2, value: foo-4, kind: reloc_riprel_4byte
 
 // rdar://37247000
 movl	$12, 1024(%rip)
@@ -625,32 +625,32 @@ movl	$12, 1024(%rip)
 movq	$12, foo(%rip)
 // CHECK:  movq	$12, foo(%rip)
 // CHECK: encoding: [0x48,0xc7,0x05,A,A,A,A,0x0c,0x00,0x00,0x00]
-// CHECK:    fixup A - offset: 3, value: foo-8, kind: reloc_riprel_4byte
+// CHECK:    fixup A - offset: 3, value: foo-4, kind: reloc_riprel_4byte
 
 movl	foo(%eip), %eax
 // CHECK: movl	foo(%eip), %eax
 // CHECK: encoding: [0x67,0x8b,0x05,A,A,A,A]
-// CHECK: fixup A - offset: 3, value: foo-4, kind: reloc_riprel_4byte
+// CHECK: fixup A - offset: 3, value: foo, kind: reloc_riprel_4byte
 
 movb	$12, foo(%eip)
 // CHECK: movb	$12, foo(%eip)
 // CHECK: encoding: [0x67,0xc6,0x05,A,A,A,A,0x0c]
-// CHECK:    fixup A - offset: 3, value: foo-5, kind: reloc_riprel_4byte
+// CHECK:    fixup A - offset: 3, value: foo-1, kind: reloc_riprel_4byte
 
 movw	$12, foo(%eip)
 // CHECK: movw	$12, foo(%eip)
 // CHECK: encoding: [0x67,0x66,0xc7,0x05,A,A,A,A,0x0c,0x00]
-// CHECK:    fixup A - offset: 4, value: foo-6, kind: reloc_riprel_4byte
+// CHECK:    fixup A - offset: 4, value: foo-2, kind: reloc_r...
[truncated]

@llvmbot
Copy link
Member

llvmbot commented Jul 4, 2025

@llvm/pr-subscribers-backend-risc-v

Author: Fangrui Song (MaskRay)

Changes

Move the X86 PC-relative fixup adjustment from
X86MCCodeEmitter::emitImmediate to MCAssembler, leveraging a generalized
evaluateFixup. This saves a MCBinaryExpr. For call foo, the fixup
expression is now foo instead of foo-4. Updated X86 MC tests confirm
no change in generated relocations.

In bolt/lib/Target/X86/X86MCPlusBuilder.cpp, createRelocation needs to
decrease the addend.

Both max-rss and instructions:u show a minor decrease.
https://llvm-compile-time-tracker.com/compare.php?from=ea600576a6f94d6f28925c4b99962cc26b463c29&amp;to=016e8fd4ddf851e5555f606c6394241d68f1a7bb&amp;stat=max-rss&amp;linkStats=on

Next: Update targets that use FKF_IsAlignedDownTo32Bits to define
evaluateFixup and remove FKF_IsAlignedDownTo32Bits from the generic
code.


Patch is 22.41 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/147113.diff

15 Files Affected:

  • (modified) bolt/lib/Target/X86/X86MCPlusBuilder.cpp (+5-3)
  • (modified) llvm/include/llvm/MC/MCAsmBackend.h (+6-3)
  • (modified) llvm/include/llvm/MC/MCValue.h (+1)
  • (modified) llvm/lib/MC/MCAssembler.cpp (+7-7)
  • (modified) llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp (+7-8)
  • (modified) llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h (+2-2)
  • (modified) llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp (+36-1)
  • (modified) llvm/lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp (+1-15)
  • (modified) llvm/test/MC/ELF/mc-dump.s (+1-1)
  • (modified) llvm/test/MC/X86/avx-64-att.s (+1-1)
  • (modified) llvm/test/MC/X86/fma4-att.s (+12-12)
  • (modified) llvm/test/MC/X86/x86-16.s (+2-2)
  • (modified) llvm/test/MC/X86/x86-32.s (+4-4)
  • (modified) llvm/test/MC/X86/x86-64.s (+14-14)
  • (modified) llvm/test/MC/X86/x86_64-encoding.s (+1-1)
diff --git a/bolt/lib/Target/X86/X86MCPlusBuilder.cpp b/bolt/lib/Target/X86/X86MCPlusBuilder.cpp
index a91fd146fa367..21d7562d41572 100644
--- a/bolt/lib/Target/X86/X86MCPlusBuilder.cpp
+++ b/bolt/lib/Target/X86/X86MCPlusBuilder.cpp
@@ -2442,9 +2442,10 @@ class X86MCPlusBuilder : public MCPlusBuilder {
 
     assert(FKI.TargetOffset == 0 && "0-bit relocation offset expected");
     const uint64_t RelOffset = Fixup.getOffset();
+    auto [RelSymbol, RelAddend] = extractFixupExpr(Fixup);
 
     uint32_t RelType;
-    if (FKI.Flags & MCFixupKindInfo::FKF_IsPCRel) {
+    if (Fixup.isPCRel()) {
       switch (FKI.TargetSize) {
       default:
         return std::nullopt;
@@ -2453,6 +2454,9 @@ class X86MCPlusBuilder : public MCPlusBuilder {
       case 32: RelType = ELF::R_X86_64_PC32; break;
       case 64: RelType = ELF::R_X86_64_PC64; break;
       }
+      // Adjust PC-relative fixup offsets, which are calculated from the start
+      // of the next instruction.
+      RelAddend -= FKI.TargetSize / 8;
     } else {
       switch (FKI.TargetSize) {
       default:
@@ -2464,8 +2468,6 @@ class X86MCPlusBuilder : public MCPlusBuilder {
       }
     }
 
-    auto [RelSymbol, RelAddend] = extractFixupExpr(Fixup);
-
     return Relocation({RelOffset, RelSymbol, RelType, RelAddend, 0});
   }
 
diff --git a/llvm/include/llvm/MC/MCAsmBackend.h b/llvm/include/llvm/MC/MCAsmBackend.h
index 54e43b89b7b00..46547560b83ed 100644
--- a/llvm/include/llvm/MC/MCAsmBackend.h
+++ b/llvm/include/llvm/MC/MCAsmBackend.h
@@ -106,9 +106,12 @@ class LLVM_ABI MCAsmBackend {
     return false;
   }
 
-  virtual bool evaluateTargetFixup(const MCFixup &Fixup, const MCValue &Target,
-                                   uint64_t &Value) {
-    llvm_unreachable("Need to implement hook if target has custom fixups");
+  // Evaluate a fixup, returning std::nullopt to use default handling for
+  // `Value` and `IsResolved`. Otherwise, returns `IsResolved` with the
+  // expectation that the hook updates `Value`.
+  virtual std::optional<bool> evaluateFixup(MCFixup &Fixup, MCValue &Target,
+                                            uint64_t &Value) {
+    return {};
   }
 
   void maybeAddReloc(const MCFragment &, const MCFixup &, const MCValue &,
diff --git a/llvm/include/llvm/MC/MCValue.h b/llvm/include/llvm/MC/MCValue.h
index 417c4689d1b47..c9a680ec327a6 100644
--- a/llvm/include/llvm/MC/MCValue.h
+++ b/llvm/include/llvm/MC/MCValue.h
@@ -42,6 +42,7 @@ class MCValue {
   friend class MCExpr;
   MCValue() = default;
   int64_t getConstant() const { return Cst; }
+  void setConstant(int64_t C) { Cst = C; }
   uint32_t getSpecifier() const { return Specifier; }
   void setSpecifier(uint32_t S) { Specifier = S; }
 
diff --git a/llvm/lib/MC/MCAssembler.cpp b/llvm/lib/MC/MCAssembler.cpp
index 4b7845335d822..69192db2a20d3 100644
--- a/llvm/lib/MC/MCAssembler.cpp
+++ b/llvm/lib/MC/MCAssembler.cpp
@@ -161,11 +161,13 @@ bool MCAssembler::evaluateFixup(const MCFragment &F, MCFixup &Fixup,
     return true;
   }
 
-  bool IsResolved = false;
+  // TODO: Require targets to set PCRel at fixup creation time.
   unsigned FixupFlags = getBackend().getFixupKindInfo(Fixup.getKind()).Flags;
-  bool IsPCRel = FixupFlags & MCFixupKindInfo::FKF_IsPCRel;
-  if (FixupFlags & MCFixupKindInfo::FKF_IsTarget) {
-    IsResolved = getBackend().evaluateTargetFixup(Fixup, Target, Value);
+  if (FixupFlags & MCFixupKindInfo::FKF_IsPCRel)
+    Fixup.setPCRel();
+  bool IsResolved = false;
+  if (auto State = getBackend().evaluateFixup(Fixup, Target, Value)) {
+    IsResolved = *State;
   } else {
     const MCSymbol *Add = Target.getAddSym();
     const MCSymbol *Sub = Target.getSubSym();
@@ -177,7 +179,7 @@ bool MCAssembler::evaluateFixup(const MCFragment &F, MCFixup &Fixup,
 
     bool ShouldAlignPC =
         FixupFlags & MCFixupKindInfo::FKF_IsAlignedDownTo32Bits;
-    if (IsPCRel) {
+    if (Fixup.isPCRel()) {
       uint64_t Offset = getFragmentOffset(F) + Fixup.getOffset();
 
       // A number of ARM fixups in Thumb mode require that the effective PC
@@ -202,8 +204,6 @@ bool MCAssembler::evaluateFixup(const MCFragment &F, MCFixup &Fixup,
 
   if (IsResolved && mc::isRelocRelocation(Fixup.getKind()))
     IsResolved = false;
-  if (IsPCRel)
-    Fixup.setPCRel();
   getBackend().applyFixup(F, Fixup, Target, Contents, Value, IsResolved);
   return true;
 }
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
index d298ce769c1dc..79ada59e7a524 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
@@ -70,10 +70,8 @@ MCFixupKindInfo RISCVAsmBackend::getFixupKindInfo(MCFixupKind Kind) const {
       {"fixup_riscv_12_i", 20, 12, 0},
       {"fixup_riscv_lo12_s", 0, 32, 0},
       {"fixup_riscv_pcrel_hi20", 12, 20, MCFixupKindInfo::FKF_IsPCRel},
-      {"fixup_riscv_pcrel_lo12_i", 20, 12,
-       MCFixupKindInfo::FKF_IsPCRel | MCFixupKindInfo::FKF_IsTarget},
-      {"fixup_riscv_pcrel_lo12_s", 0, 32,
-       MCFixupKindInfo::FKF_IsPCRel | MCFixupKindInfo::FKF_IsTarget},
+      {"fixup_riscv_pcrel_lo12_i", 20, 12, MCFixupKindInfo::FKF_IsPCRel},
+      {"fixup_riscv_pcrel_lo12_s", 0, 32, MCFixupKindInfo::FKF_IsPCRel},
       {"fixup_riscv_jal", 12, 20, MCFixupKindInfo::FKF_IsPCRel},
       {"fixup_riscv_branch", 0, 32, MCFixupKindInfo::FKF_IsPCRel},
       {"fixup_riscv_rvc_jump", 2, 11, MCFixupKindInfo::FKF_IsPCRel},
@@ -657,15 +655,16 @@ static const MCFixup *getPCRelHiFixup(const MCSpecifierExpr &Expr,
   return nullptr;
 }
 
-bool RISCVAsmBackend::evaluateTargetFixup(const MCFixup &Fixup,
-                                          const MCValue &Target,
-                                          uint64_t &Value) {
+std::optional<bool> RISCVAsmBackend::evaluateFixup(MCFixup &Fixup,
+                                                   MCValue &Target,
+                                                   uint64_t &Value) {
   const MCFixup *AUIPCFixup;
   const MCFragment *AUIPCDF;
   MCValue AUIPCTarget;
   switch (Fixup.getTargetKind()) {
   default:
-    llvm_unreachable("Unexpected fixup kind!");
+    // Use default handling for `Value` and `IsResolved`.
+    return {};
   case RISCV::fixup_riscv_pcrel_lo12_i:
   case RISCV::fixup_riscv_pcrel_lo12_s: {
     AUIPCFixup =
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h
index d98361ea5857f..292cacdbd45af 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h
@@ -47,8 +47,8 @@ class RISCVAsmBackend : public MCAsmBackend {
   bool shouldInsertFixupForCodeAlign(MCAssembler &Asm,
                                      MCAlignFragment &AF) override;
 
-  bool evaluateTargetFixup(const MCFixup &Fixup, const MCValue &Target,
-                           uint64_t &Value) override;
+  std::optional<bool> evaluateFixup(MCFixup &Fixup, MCValue &Target,
+                                    uint64_t &Value) override;
 
   bool addReloc(const MCFragment &, const MCFixup &, const MCValue &,
                 uint64_t &FixedValue, bool IsResolved);
diff --git a/llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp b/llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp
index b56d8f2d2eb66..14842c4b93e2c 100644
--- a/llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp
+++ b/llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp
@@ -169,7 +169,8 @@ class X86AsmBackend : public MCAsmBackend {
 
   MCFixupKindInfo getFixupKindInfo(MCFixupKind Kind) const override;
 
-  bool shouldForceRelocation(const MCFixup &, const MCValue &);
+  std::optional<bool> evaluateFixup(MCFixup &Fixup, MCValue &Target,
+                                    uint64_t &Value) override;
 
   void applyFixup(const MCFragment &, const MCFixup &, const MCValue &Target,
                   MutableArrayRef<char> Data, uint64_t Value,
@@ -687,6 +688,40 @@ static unsigned getFixupKindSize(unsigned Kind) {
   }
 }
 
+// Adjust PC-relative fixup offsets, which are calculated from the start of the
+// next instruction.
+std::optional<bool>
+X86AsmBackend::evaluateFixup(MCFixup &Fixup, MCValue &Target, uint64_t &Value) {
+  switch (Fixup.getTargetKind()) {
+  case FK_PCRel_1:
+    Target.setConstant(Target.getConstant() - 1);
+    break;
+  case FK_PCRel_2:
+    Target.setConstant(Target.getConstant() - 2);
+    break;
+  case FK_PCRel_4:
+  case X86::reloc_riprel_4byte:
+  case X86::reloc_riprel_4byte_movq_load:
+  case X86::reloc_riprel_4byte_movq_load_rex2:
+  case X86::reloc_riprel_4byte_relax:
+  case X86::reloc_riprel_4byte_relax_rex:
+  case X86::reloc_riprel_4byte_relax_rex2:
+  case X86::reloc_branch_4byte_pcrel:
+  case X86::reloc_riprel_4byte_relax_evex: {
+    Target.setConstant(Target.getConstant() - 4);
+    auto *Add = Target.getAddSym();
+    // If this is a pc-relative load off _GLOBAL_OFFSET_TABLE_:
+    // leaq _GLOBAL_OFFSET_TABLE_(%rip), %r15
+    // this needs to be a GOTPC32 relocation.
+    if (Add && Add->getName() == "_GLOBAL_OFFSET_TABLE_")
+      Fixup = MCFixup::create(Fixup.getOffset(), Fixup.getValue(),
+                              X86::reloc_global_offset_table);
+  } break;
+  }
+  // Use default handling for `Value` and `IsResolved`.
+  return {};
+}
+
 void X86AsmBackend::applyFixup(const MCFragment &F, const MCFixup &Fixup,
                                const MCValue &Target,
                                MutableArrayRef<char> Data, uint64_t Value,
diff --git a/llvm/lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp b/llvm/lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp
index 1666518e3365c..5ef6f14ece9ac 100644
--- a/llvm/lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp
+++ b/llvm/lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp
@@ -576,18 +576,10 @@ void X86MCCodeEmitter::emitImmediate(const MCOperand &DispOp, SMLoc Loc,
     }
   }
 
-  // If the fixup is pc-relative, we need to bias the value to be relative to
-  // the start of the field, not the end of the field.
   bool PCRel = false;
   switch (FixupKind) {
   case FK_PCRel_1:
-    PCRel = true;
-    ImmOffset -= 1;
-    break;
   case FK_PCRel_2:
-    PCRel = true;
-    ImmOffset -= 2;
-    break;
   case FK_PCRel_4:
   case X86::reloc_riprel_4byte:
   case X86::reloc_riprel_4byte_movq_load:
@@ -598,12 +590,6 @@ void X86MCCodeEmitter::emitImmediate(const MCOperand &DispOp, SMLoc Loc,
   case X86::reloc_branch_4byte_pcrel:
   case X86::reloc_riprel_4byte_relax_evex:
     PCRel = true;
-    ImmOffset -= 4;
-    // If this is a pc-relative load off _GLOBAL_OFFSET_TABLE_:
-    // leaq _GLOBAL_OFFSET_TABLE_(%rip), %r15
-    // this needs to be a GOTPC32 relocation.
-    if (startsWithGlobalOffsetTable(Expr) != GOT_None)
-      FixupKind = X86::reloc_global_offset_table;
     break;
   }
 
@@ -611,7 +597,7 @@ void X86MCCodeEmitter::emitImmediate(const MCOperand &DispOp, SMLoc Loc,
     Expr = MCBinaryExpr::createAdd(Expr, MCConstantExpr::create(ImmOffset, Ctx),
                                    Ctx, Expr->getLoc());
 
-  // Emit a symbolic constant as a fixup and 4 zeros.
+  // Emit a symbolic constant as a fixup and a few zero bytes.
   Fixups.push_back(MCFixup::create(static_cast<uint32_t>(CB.size() - StartByte),
                                    Expr, FixupKind, PCRel));
   emitConstant(0, Size, CB);
diff --git a/llvm/test/MC/ELF/mc-dump.s b/llvm/test/MC/ELF/mc-dump.s
index 2286a0d2510c0..ac9506f18b4b5 100644
--- a/llvm/test/MC/ELF/mc-dump.s
+++ b/llvm/test/MC/ELF/mc-dump.s
@@ -13,7 +13,7 @@
 # CHECK-NEXT:  Symbol @0 _start
 # CHECK-NEXT:0 Org Offset:3 Value:0
 # CHECK-NEXT:3 Relaxable Size:2 <MCInst #1996 <MCOperand Expr:.Ltmp0>>
-# CHECK-NEXT:  Fixup @1 Value:.Ltmp0-1 Kind:4006
+# CHECK-NEXT:  Fixup @1 Value:.Ltmp0 Kind:4006
 # CHECK-NEXT:5 Data Size:16 [48,8b,04,25,00,00,00,00,48,8b,04,25,00,00,00,00]
 # CHECK-NEXT:  Fixup @4 Value:f0@<variant 11> Kind:4021
 # CHECK-NEXT:  Fixup @12 Value:_start@<variant 11> Kind:4021
diff --git a/llvm/test/MC/X86/avx-64-att.s b/llvm/test/MC/X86/avx-64-att.s
index 39ee048c3736d..c1e778f92bb96 100644
--- a/llvm/test/MC/X86/avx-64-att.s
+++ b/llvm/test/MC/X86/avx-64-att.s
@@ -4125,7 +4125,7 @@ _foo:
 
 // CHECK: vblendvps %ymm1, _foo2(%rip), %ymm0, %ymm0
 // CHECK: encoding: [0xc4,0xe3,0x7d,0x4a,0x05,A,A,A,A,0x10]
-// CHECK: fixup A - offset: 5, value: _foo2-5
+// CHECK: fixup A - offset: 5, value: _foo2-1
 _foo2:
   nop
   vblendvps %ymm1, _foo2(%rip), %ymm0, %ymm0
diff --git a/llvm/test/MC/X86/fma4-att.s b/llvm/test/MC/X86/fma4-att.s
index c9bd954e90496..55e5ddd09302e 100644
--- a/llvm/test/MC/X86/fma4-att.s
+++ b/llvm/test/MC/X86/fma4-att.s
@@ -80,62 +80,62 @@
 // PR15040
 // CHECK: vfmaddss  foo(%rip), %xmm1, %xmm0, %xmm0
 // CHECK: encoding: [0xc4,0xe3,0xf9,0x6a,0x05,A,A,A,A,0x10]
-// CHECK: fixup A - offset: 5, value: foo-5, kind: reloc_riprel_4byte
+// CHECK: fixup A - offset: 5, value: foo-1, kind: reloc_riprel_4byte
           vfmaddss  foo(%rip), %xmm1, %xmm0, %xmm0
 
 // CHECK: vfmaddss   %xmm1, foo(%rip), %xmm0, %xmm0
 // CHECK: encoding: [0xc4,0xe3,0x79,0x6a,0x05,A,A,A,A,0x10]
-// CHECK: fixup A - offset: 5, value: foo-5, kind: reloc_riprel_4byte
+// CHECK: fixup A - offset: 5, value: foo-1, kind: reloc_riprel_4byte
           vfmaddss   %xmm1, foo(%rip),%xmm0, %xmm0
 
 // CHECK: vfmaddsd  foo(%rip), %xmm1, %xmm0, %xmm0
 // CHECK: encoding: [0xc4,0xe3,0xf9,0x6b,0x05,A,A,A,A,0x10]
-// CHECK: fixup A - offset: 5, value: foo-5, kind: reloc_riprel_4byte
+// CHECK: fixup A - offset: 5, value: foo-1, kind: reloc_riprel_4byte
           vfmaddsd  foo(%rip), %xmm1, %xmm0, %xmm0
 
 // CHECK: vfmaddsd   %xmm1, foo(%rip), %xmm0, %xmm0
 // CHECK: encoding: [0xc4,0xe3,0x79,0x6b,0x05,A,A,A,A,0x10]
-// CHECK: fixup A - offset: 5, value: foo-5, kind: reloc_riprel_4byte
+// CHECK: fixup A - offset: 5, value: foo-1, kind: reloc_riprel_4byte
           vfmaddsd   %xmm1, foo(%rip),%xmm0, %xmm0
 
 // CHECK: vfmaddps  foo(%rip), %xmm1, %xmm0, %xmm0
 // CHECK: encoding: [0xc4,0xe3,0xf9,0x68,0x05,A,A,A,A,0x10]
-// CHECK: fixup A - offset: 5, value: foo-5, kind: reloc_riprel_4byte
+// CHECK: fixup A - offset: 5, value: foo-1, kind: reloc_riprel_4byte
           vfmaddps  foo(%rip), %xmm1, %xmm0, %xmm0
 
 // CHECK: vfmaddps   %xmm1, foo(%rip), %xmm0, %xmm0
 // CHECK: encoding: [0xc4,0xe3,0x79,0x68,0x05,A,A,A,A,0x10]
-// CHECK: fixup A - offset: 5, value: foo-5, kind: reloc_riprel_4byte
+// CHECK: fixup A - offset: 5, value: foo-1, kind: reloc_riprel_4byte
           vfmaddps   %xmm1, foo(%rip),%xmm0, %xmm0
 
 // CHECK: vfmaddpd  foo(%rip), %xmm1, %xmm0, %xmm0
 // CHECK: encoding: [0xc4,0xe3,0xf9,0x69,0x05,A,A,A,A,0x10]
-// CHECK: fixup A - offset: 5, value: foo-5, kind: reloc_riprel_4byte
+// CHECK: fixup A - offset: 5, value: foo-1, kind: reloc_riprel_4byte
           vfmaddpd  foo(%rip), %xmm1, %xmm0, %xmm0
 
 // CHECK: vfmaddpd   %xmm1, foo(%rip), %xmm0, %xmm0
 // CHECK: encoding: [0xc4,0xe3,0x79,0x69,0x05,A,A,A,A,0x10]
-// CHECK: fixup A - offset: 5, value: foo-5, kind: reloc_riprel_4byte
+// CHECK: fixup A - offset: 5, value: foo-1, kind: reloc_riprel_4byte
           vfmaddpd   %xmm1, foo(%rip),%xmm0, %xmm0
 
 // CHECK: vfmaddps  foo(%rip), %ymm1, %ymm0, %ymm0
 // CHECK: encoding: [0xc4,0xe3,0xfd,0x68,0x05,A,A,A,A,0x10]
-// CHECK: fixup A - offset: 5, value: foo-5, kind: reloc_riprel_4byte
+// CHECK: fixup A - offset: 5, value: foo-1, kind: reloc_riprel_4byte
           vfmaddps  foo(%rip), %ymm1, %ymm0, %ymm0
 
 // CHECK: vfmaddps   %ymm1, foo(%rip), %ymm0, %ymm0
 // CHECK: encoding: [0xc4,0xe3,0x7d,0x68,0x05,A,A,A,A,0x10]
-// CHECK: fixup A - offset: 5, value: foo-5, kind: reloc_riprel_4byte
+// CHECK: fixup A - offset: 5, value: foo-1, kind: reloc_riprel_4byte
           vfmaddps   %ymm1, foo(%rip),%ymm0, %ymm0
 
 // CHECK: vfmaddpd  foo(%rip), %ymm1, %ymm0, %ymm0
 // CHECK: encoding: [0xc4,0xe3,0xfd,0x69,0x05,A,A,A,A,0x10]
-// CHECK: fixup A - offset: 5, value: foo-5, kind: reloc_riprel_4byte
+// CHECK: fixup A - offset: 5, value: foo-1, kind: reloc_riprel_4byte
           vfmaddpd  foo(%rip), %ymm1, %ymm0, %ymm0
 
 // CHECK: vfmaddpd   %ymm1, foo(%rip), %ymm0, %ymm0
 // CHECK: encoding: [0xc4,0xe3,0x7d,0x69,0x05,A,A,A,A,0x10]
-// CHECK: fixup A - offset: 5, value: foo-5, kind: reloc_riprel_4byte
+// CHECK: fixup A - offset: 5, value: foo-1, kind: reloc_riprel_4byte
           vfmaddpd   %ymm1, foo(%rip),%ymm0, %ymm0
 
 // vfmsub
diff --git a/llvm/test/MC/X86/x86-16.s b/llvm/test/MC/X86/x86-16.s
index be0563e5b1d63..9a1963760c99a 100644
--- a/llvm/test/MC/X86/x86-16.s
+++ b/llvm/test/MC/X86/x86-16.s
@@ -1058,13 +1058,13 @@ xresldtrk
 
 // CHECK: jmp foo
 // CHECK:  encoding: [0xe9,A,A]
-// CHECK:  fixup A - offset: 1, value: foo-2, kind: FK_PCRel_2
+// CHECK:  fixup A - offset: 1, value: foo, kind: FK_PCRel_2
 {disp32} jmp foo
 foo:
 
 // CHECK: je foo
 // CHECK:  encoding: [0x0f,0x84,A,A]
-// CHECK:  fixup A - offset: 2, value: foo-2, kind: FK_PCRel_2
+// CHECK:  fixup A - offset: 2, value: foo, kind: FK_PCRel_2
 {disp32} je foo
 
 // CHECK: movl nearer, %ebx
diff --git a/llvm/test/MC/X86/x86-32.s b/llvm/test/MC/X86/x86-32.s
index e3d473a05e59d..2b9dc921844b1 100644
--- a/llvm/test/MC/X86/x86-32.s
+++ b/llvm/test/MC/X86/x86-32.s
@@ -1120,20 +1120,20 @@ ptwritel %eax
 
 // CHECK: jmp foo
 // CHECK:  encoding: [0xe9,A,A,A,A]
-// CHECK:  fixup A - offset: 1, value: foo-4, kind: FK_PCRel_4
+// CHECK:  fixup A - offset: 1, value: foo, kind: FK_PCRel_4
 // CHECK: jmp foo
 // CHECK:  encoding: [0xe9,A,A,A,A]
-// CHECK:  fixup A - offset: 1, value: foo-4, kind: FK_PCRel_4
+// CHECK:  fixup A - offset: 1, value: foo, kind: FK_PCRel_4
 {disp32} jmp foo
 jmp.d32 foo
 foo:
 
 // CHECK: je foo
 // CHECK:  encoding: [0x0f,0x84,A,A,A,A]
-// CHECK:  fixup A - offset: 2, value: foo-4, kind: FK_PCRel_4
+// CHECK:  fixup A - offset: 2, value: foo, kind: FK_PCRel_4
 // CHECK: je foo
 // CHECK:  encoding: [0x0f,0x84,A,A,A,A]
-// CHECK:  fixup A - offset: 2, value: foo-4, kind: FK_PCRel_4
+// CHECK:  fixup A - offset: 2, value: foo, kind: FK_PCRel_4
 {disp32} je foo
 je.d32 foo
 
diff --git a/llvm/test/MC/X86/x86-64.s b/llvm/test/MC/X86/x86-64.s
index 108d1220107e3..657792a610318 100644
--- a/llvm/test/MC/X86/x86-64.s
+++ b/llvm/test/MC/X86/x86-64.s
@@ -600,22 +600,22 @@ fdivp %st(1), %st // CHECK: encoding: [0xde,0xf1]
 movl	foo(%rip), %eax
 // CHECK: movl	foo(%rip), %eax
 // CHECK: encoding: [0x8b,0x05,A,A,A,A]
-// CHECK: fixup A - offset: 2, value: foo-4, kind: reloc_riprel_4byte
+// CHECK: fixup A - offset: 2, value: foo, kind: reloc_riprel_4byte
 
 movb	$12, foo(%rip)
 // CHECK: movb	$12, foo(%rip)
 // CHECK: encoding: [0xc6,0x05,A,A,A,A,0x0c]
-// CHECK:    fixup A - offset: 2, value: foo-5, kind: reloc_riprel_4byte
+// CHECK:    fixup A - offset: 2, value: foo-1, kind: reloc_riprel_4byte
 
 movw	$12, foo(%rip)
 // CHECK: movw	$12, foo(%rip)
 // CHECK: encoding: [0x66,0xc7,0x05,A,A,A,A,0x0c,0x00]
-// CHECK:    fixup A - offset: 3, value: foo-6, kind: reloc_riprel_4byte
+// CHECK:    fixup A - offset: 3, value: foo-2, kind: reloc_riprel_4byte
 
 movl	$12, foo(%rip)
 // CHECK: movl	$12, foo(%rip)
 // CHECK: encoding: [0xc7,0x05,A,A,A,A,0x0c,0x00,0x00,0x00]
-// CHECK:    fixup A - offset: 2, value: foo-8, kind: reloc_riprel_4byte
+// CHECK:    fixup A - offset: 2, value: foo-4, kind: reloc_riprel_4byte
 
 // rdar://37247000
 movl	$12, 1024(%rip)
@@ -625,32 +625,32 @@ movl	$12, 1024(%rip)
 movq	$12, foo(%rip)
 // CHECK:  movq	$12, foo(%rip)
 // CHECK: encoding: [0x48,0xc7,0x05,A,A,A,A,0x0c,0x00,0x00,0x00]
-// CHECK:    fixup A - offset: 3, value: foo-8, kind: reloc_riprel_4byte
+// CHECK:    fixup A - offset: 3, value: foo-4, kind: reloc_riprel_4byte
 
 movl	foo(%eip), %eax
 // CHECK: movl	foo(%eip), %eax
 // CHECK: encoding: [0x67,0x8b,0x05,A,A,A,A]
-// CHECK: fixup A - offset: 3, value: foo-4, kind: reloc_riprel_4byte
+// CHECK: fixup A - offset: 3, value: foo, kind: reloc_riprel_4byte
 
 movb	$12, foo(%eip)
 // CHECK: movb	$12, foo(%eip)
 // CHECK: encoding: [0x67,0xc6,0x05,A,A,A,A,0x0c]
-// CHECK:    fixup A - offset: 3, value: foo-5, kind: reloc_riprel_4byte
+// CHECK:    fixup A - offset: 3, value: foo-1, kind: reloc_riprel_4byte
 
 movw	$12, foo(%eip)
 // CHECK: movw	$12, foo(%eip)
 // CHECK: encoding: [0x67,0x66,0xc7,0x05,A,A,A,A,0x0c,0x00]
-// CHECK:    fixup A - offset: 4, value: foo-6, kind: reloc_riprel_4byte
+// CHECK:    fixup A - offset: 4, value: foo-2, kind: reloc_r...
[truncated]

MaskRay added 2 commits July 4, 2025 15:46
Created using spr 1.3.5-bogner
Created using spr 1.3.5-bogner
MaskRay added a commit to MaskRay/llvm-project that referenced this pull request Jul 4, 2025
Move the X86 PC-relative fixup adjustment from
X86MCCodeEmitter::emitImmediate to MCAssembler, leveraging a generalized
evaluateFixup. This saves a MCBinaryExpr. For `call foo`, the fixup
expression is now `foo` instead of `foo-4`. Updated X86 MC tests confirm
no change in generated relocations.

In bolt/lib/Target/X86/X86MCPlusBuilder.cpp, createRelocation needs to
decrease the addend.

Both max-rss and instructions:u show a minor decrease.
https://llvm-compile-time-tracker.com/compare.php?from=ea600576a6f94d6f28925c4b99962cc26b463c29&to=016e8fd4ddf851e5555f606c6394241d68f1a7bb&stat=max-rss&linkStats=on

Next: Update targets that use FKF_IsAlignedDownTo32Bits to define
`evaluateFixup` and remove FKF_IsAlignedDownTo32Bits from the generic
code.

Pull Request: llvm#147113
MaskRay added 5 commits July 4, 2025 20:19
Created using spr 1.3.5-bogner
Created using spr 1.3.5-bogner
Created using spr 1.3.5-bogner
Created using spr 1.3.5-bogner
Created using spr 1.3.5-bogner
MaskRay added a commit to MaskRay/llvm-project that referenced this pull request Jul 5, 2025
Move the X86 PC-relative fixup adjustment from
X86MCCodeEmitter::emitImmediate to MCAssembler, leveraging a generalized
evaluateFixup. This saves a MCBinaryExpr. For `call foo`, the fixup
expression is now `foo` instead of `foo-4`. There is no change in
generated relocations.

In bolt/lib/Target/X86/X86MCPlusBuilder.cpp, createRelocation needs to
decrease the addend.

Both max-rss and instructions:u show a minor decrease.
https://llvm-compile-time-tracker.com/compare.php?from=ea600576a6f94d6f28925c4b99962cc26b463c29&to=016e8fd4ddf851e5555f606c6394241d68f1a7bb&stat=max-rss&linkStats=on

Next: Update targets that use FKF_IsAlignedDownTo32Bits to define
`evaluateFixup` and remove FKF_IsAlignedDownTo32Bits from the generic
code.

Pull Request: llvm#147113
@MaskRay MaskRay requested a review from aengelke July 8, 2025 03:23
Copy link
Contributor

@maksfb maksfb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BOLT part looks good to me. Thanks.

// If this is a pc-relative load off _GLOBAL_OFFSET_TABLE_:
// leaq _GLOBAL_OFFSET_TABLE_(%rip), %r15
// this needs to be a GOTPC32 relocation.
if (Add && Add->getName() == "_GLOBAL_OFFSET_TABLE_")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm missing some context here. What fixup kinds, other than X86::reloc_global_offset_table, can end up hitting this code path? startsWithGlobalOffsetTable does some more checks, and is still called in emitImmediate -- could the different checks cause problems? I don't quite like duplicating checks for magical symbol names.. Couldn't we keep the GOT-check in emitImmediate and just adjust the offset here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In 2020, I implemented this code path for all PC-relative fixups in https://reviews.llvm.org/D47507 (I overlooked adding corresponding mov tests).

The current implementation is guarded by if (Fixup.isPCRel()) {, reflecting the original intention. I think testing the magic symbol during the relocation decision phase (https://maskray.me/blog/2025-03-16-relocation-generation-in-assemblers#relocation-decision-phase) is the correct approach. Transitioning another startsWithGlobalOffsetTable caller in X86MCCodeEmitter.cpp to use X86MCAsmBackend.cpp would be ideal, but that falls outside this change’s scope. X86MCCodeEmitter.cpp has quite a lot of tech debt.

// leaq _GLOBAL_OFFSET_TABLE_(%rip), %r15
// this needs to be a GOTPC32 relocation.
if (Size == 4 && startsWithGlobalOffsetTable(Expr) != GOT_None)
FixupKind = X86::reloc_global_offset_table;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't removing this have an effect on test output?

Copy link
Member Author

@MaskRay MaskRay Jul 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has no effect on all the _GLOBAL_OFFSET_TABLE_ (https://maskray.me/blog/2021-08-29-all-about-global-offset-table#got-optimization#global_offset_table_) uses LLVM emits or hand-written assembly uses in practice

In 2020, I added this piece of code in reviews.llvm.org/D47507. I am actually not sure I got everything correct, but moving the code from MCCodeEmitter (parse time) to AsmBackend (relocation decision time) is the right direction.

% rg '(lea|mov).*_GLOBAL' lld/test/ELF llvm/test/MC/X86/
lld/test/ELF/x86-64-reloc-gotpc64.s
14:  movabsq $_GLOBAL_OFFSET_TABLE_-., %r11

lld/test/ELF/relocation-i686.s
66: movl $_GLOBAL_OFFSET_TABLE_, %eax

lld/test/ELF/x86-64-reloc-gotoff64.s
23:  leaq _GLOBAL_OFFSET_TABLE_(%rip), %rdx

lld/test/ELF/i386-gotpc.s
7:movl $_GLOBAL_OFFSET_TABLE_, %eax

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants