Skip to content
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

AsmPrinter: Remove ELF's special lowerRelativeReference for unnamed_addr function #132684

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

MaskRay
Copy link
Member

@MaskRay MaskRay commented Mar 24, 2025

https://reviews.llvm.org/D17938 introduced lowerRelativeReference to
give ConstantExpr sub (A-B) special semantics in ELF: when A is an
unnamed_addr function, create a PLT-generating relocation. This was
intended for C++ relative vtables, but C++ relative vtable ended up
using DSOLocalEquivalent (lowerDSOLocalEquivalent).

This special treatment of unnamed_addr seems unusual.
Let's remove it. Only COFF needs an overload to generate a @IMGREL32
relocation specifier (llvm/test/MC/COFF/cross-section-relative.ll).

Created using spr 1.3.5-bogner
@MaskRay MaskRay requested a review from pcc March 24, 2025 07:28
@llvmbot
Copy link
Member

llvmbot commented Mar 24, 2025

@llvm/pr-subscribers-backend-x86

Author: Fangrui Song (MaskRay)

Changes

https://reviews.llvm.org/D17938 introduced lowerRelativeReference to
give ConstantExpr sub (A-B) special semantics in ELF: when A is an
unnamed_addr function, create a PLT-generating relocation. This was
intended for C++ relative vtables, but C++ relative vtable ended up
using DSOLocalEquivalent (lowerDSOLocalEquivalent).

This special treatment of unnamed_addr seems unusual.
Let's remove it. Only COFF needs an overload to generate a @IMGREL32
relocation specifier (llvm/test/MC/COFF/cross-section-relative.ll).


Full diff: https://github.com/llvm/llvm-project/pull/132684.diff

4 Files Affected:

  • (modified) llvm/include/llvm/CodeGen/TargetLoweringObjectFileImpl.h (-4)
  • (modified) llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp (-20)
  • (modified) llvm/test/CodeGen/X86/x86-64-plt-relative-reloc.ll (+3-2)
  • (modified) llvm/test/CodeGen/X86/x86-plt-relative-reloc.ll (+2-2)
diff --git a/llvm/include/llvm/CodeGen/TargetLoweringObjectFileImpl.h b/llvm/include/llvm/CodeGen/TargetLoweringObjectFileImpl.h
index 76571690eeda0..293aa9d1faf7d 100644
--- a/llvm/include/llvm/CodeGen/TargetLoweringObjectFileImpl.h
+++ b/llvm/include/llvm/CodeGen/TargetLoweringObjectFileImpl.h
@@ -112,10 +112,6 @@ class TargetLoweringObjectFileELF : public TargetLoweringObjectFile {
   MCSection *getStaticDtorSection(unsigned Priority,
                                   const MCSymbol *KeySym) const override;
 
-  const MCExpr *lowerRelativeReference(const GlobalValue *LHS,
-                                       const GlobalValue *RHS,
-                                       const TargetMachine &TM) const override;
-
   const MCExpr *lowerDSOLocalEquivalent(const DSOLocalEquivalent *Equiv,
                                         const TargetMachine &TM) const override;
 
diff --git a/llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp b/llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
index 0e44acdd1dccc..0f6d0001087d9 100644
--- a/llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
+++ b/llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
@@ -1174,26 +1174,6 @@ MCSection *TargetLoweringObjectFileELF::getStaticDtorSection(
                                   KeySym);
 }
 
-const MCExpr *TargetLoweringObjectFileELF::lowerRelativeReference(
-    const GlobalValue *LHS, const GlobalValue *RHS,
-    const TargetMachine &TM) const {
-  // We may only use a PLT-relative relocation to refer to unnamed_addr
-  // functions.
-  if (!LHS->hasGlobalUnnamedAddr() || !LHS->getValueType()->isFunctionTy())
-    return nullptr;
-
-  // Basic correctness checks.
-  if (LHS->getType()->getPointerAddressSpace() != 0 ||
-      RHS->getType()->getPointerAddressSpace() != 0 || LHS->isThreadLocal() ||
-      RHS->isThreadLocal())
-    return nullptr;
-
-  return MCBinaryExpr::createSub(
-      MCSymbolRefExpr::create(TM.getSymbol(LHS), PLTRelativeSpecifier,
-                              getContext()),
-      MCSymbolRefExpr::create(TM.getSymbol(RHS), getContext()), getContext());
-}
-
 const MCExpr *TargetLoweringObjectFileELF::lowerDSOLocalEquivalent(
     const DSOLocalEquivalent *Equiv, const TargetMachine &TM) const {
   assert(supportDSOLocalEquivalentLowering());
diff --git a/llvm/test/CodeGen/X86/x86-64-plt-relative-reloc.ll b/llvm/test/CodeGen/X86/x86-64-plt-relative-reloc.ll
index f949c83efd03f..02b012cf18199 100644
--- a/llvm/test/CodeGen/X86/x86-64-plt-relative-reloc.ll
+++ b/llvm/test/CodeGen/X86/x86-64-plt-relative-reloc.ll
@@ -12,8 +12,9 @@ declare void @fn2() unnamed_addr
 declare void @fn3()
 @global4 = external unnamed_addr global i8
 
+;; Generate a PC-relative relocation, which might be rejected by the linker if the referenced symbol is preemptible.
 ; CHECK: .long 0
-; CHECK-NEXT: .long (fn1@PLT-vtable)-4
-; CHECK-NEXT: .long (fn2@PLT-vtable)-4
+; CHECK-NEXT: .long (fn1-vtable)-4
+; CHECK-NEXT: .long (fn2-vtable)-4
 ; CHECK-NEXT: .long (fn3-vtable)-4
 ; CHECK-NEXT: .long (global4-vtable)-4
diff --git a/llvm/test/CodeGen/X86/x86-plt-relative-reloc.ll b/llvm/test/CodeGen/X86/x86-plt-relative-reloc.ll
index 8c86cd29d1c81..ea5dd5b5a83a1 100644
--- a/llvm/test/CodeGen/X86/x86-plt-relative-reloc.ll
+++ b/llvm/test/CodeGen/X86/x86-plt-relative-reloc.ll
@@ -11,6 +11,6 @@ declare void @fn2() unnamed_addr
 declare void @fn3()
 
 ; CHECK: .long 0
-; CHECK-NEXT: .long (fn1@PLT-vtable)-4
-; CHECK-NEXT: .long (fn2@PLT-vtable)-4
+; CHECK-NEXT: .long (fn1-vtable)-4
+; CHECK-NEXT: .long (fn2-vtable)-4
 ; CHECK-NEXT: .long (fn3-vtable)-4

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

Successfully merging this pull request may close these issues.

2 participants