AsmPrinter: Remove ELF's special lowerRelativeReference for unnamed_addr function #132684
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Created using spr 1.3.5-bogner
|
@llvm/pr-subscribers-backend-arm @llvm/pr-subscribers-backend-x86 Author: Fangrui Song (MaskRay) Changeshttps://reviews.llvm.org/D17938 introduced lowerRelativeReference to This special treatment of Full diff: https://github.com/llvm/llvm-project/pull/132684.diff 4 Files Affected:
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
|
|
@pcc :) |
pcc
approved these changes
Mar 31, 2025
Created using spr 1.3.5-bogner
Created using spr 1.3.5-bogner
MaskRay
deleted the
users/MaskRay/spr/asmprinter-remove-elfs-special-lowerrelativereference-for-unnamed_addr-function
branch
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Apr 1, 2025
…r unnamed_addr function 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). Pull Request: llvm/llvm-project#132684
|
This broke the build of libc++abi for Fuchsia with the following error: Fuchsia uses C++ relative vtables by default so it looks like this is still being used. @PiJoules is familiar with the implementation of relative vtables and should be able to help with debugging. Would it be possible to revert this change in the meantime? |
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Apr 1, 2025
…rence for unnamed_addr function" (#133935) Reverts llvm/llvm-project#132684
|
It's correct that that specifically with the |
Ankur-0429
pushed a commit
to Ankur-0429/llvm-project
that referenced
this pull request
Apr 2, 2025
…nnamed_addr function" (llvm#133935) Reverts llvm#132684
Thanks! I forgot adjusting the code even if I added the comment |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
https://reviews.llvm.org/D17938 introduced lowerRelativeReference to
give ConstantExpr sub (A-B) special semantics in ELF: when
Ais anunnamed_addrfunction, create a PLT-generating relocation. This wasintended for C++ relative vtables, but C++ relative vtable ended up
using DSOLocalEquivalent (lowerDSOLocalEquivalent).
This special treatment of
unnamed_addrseems unusual.Let's remove it. Only COFF needs an overload to generate a @IMGREL32
relocation specifier (llvm/test/MC/COFF/cross-section-relative.ll).