AsmPrinter: Remove ELF's special lowerRelativeReference for unnamed_addr function; use lowerDSOLocalEquivalent in more cases #134781
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-x86 @llvm/pr-subscribers-backend-arm 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/134781.diff 9 Files Affected:
diff --git a/llvm/include/llvm/CodeGen/TargetLoweringObjectFileImpl.h b/llvm/include/llvm/CodeGen/TargetLoweringObjectFileImpl.h
index af8d73b4fa064..fa6cb75338d4f 100644
--- a/llvm/include/llvm/CodeGen/TargetLoweringObjectFileImpl.h
+++ b/llvm/include/llvm/CodeGen/TargetLoweringObjectFileImpl.h
@@ -125,10 +125,6 @@ class TargetLoweringObjectFileELF : public TargetLoweringObjectFile {
lowerSymbolDifference(const MCSymbol *LHS, const MCSymbol *RHS,
int64_t Addend,
std::optional<int64_t> PCRelativeOffset) const;
- const MCExpr *lowerRelativeReference(const GlobalValue *LHS,
- const GlobalValue *RHS, int64_t Addend,
- std::optional<int64_t> PCRelativeOffset,
- const TargetMachine &TM) const override;
const MCExpr *lowerDSOLocalEquivalent(const MCSymbol *LHS,
const MCSymbol *RHS, int64_t Addend,
diff --git a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
index d245cd75745d0..cf8f1c878ea5a 100644
--- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
@@ -3496,10 +3496,12 @@ const MCExpr *AsmPrinter::lowerConstant(const Constant *CV,
LHSGV, RHSGV, Addend, PCRelativeOffset, TM);
// (ELF-specific) If the generic symbol difference does not apply, and
- // LHS is a dso_local_equivalent of a dso_preemptable function,
- // reference the PLT entry instead.
- if (DSOEquiv && TM.getTargetTriple().isOSBinFormatELF() &&
- !(LHSGV->isDSOLocal() || LHSGV->isImplicitDSOLocal()))
+ // LHS is a dso_local_equivalent of a function, reference the PLT entry
+ // instead. Note: A default visibility symbol is by default preemptible
+ // during linking, and should not be referenced with PC-relative
+ // relocations. Therefore, use a PLT relocation even if the function is
+ // dso_local.
+ if (DSOEquiv && TM.getTargetTriple().isOSBinFormatELF())
Res = getObjFileLowering().lowerDSOLocalEquivalent(
LHSSym, RHSSym, Addend, PCRelativeOffset, TM);
diff --git a/llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp b/llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
index 4c20c5dc74d9a..c9415292e88f7 100644
--- a/llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
+++ b/llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
@@ -1233,24 +1233,6 @@ const MCExpr *TargetLoweringObjectFileELF::lowerSymbolDifference(
return Res;
}
-const MCExpr *TargetLoweringObjectFileELF::lowerRelativeReference(
- const GlobalValue *LHS, const GlobalValue *RHS, int64_t Addend,
- std::optional<int64_t> PCRelativeOffset, 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 lowerSymbolDifference(TM.getSymbol(LHS), TM.getSymbol(RHS), Addend,
- PCRelativeOffset);
-}
-
// Reference the PLT entry of a function, optionally with a subtrahend (`RHS`).
const MCExpr *TargetLoweringObjectFileELF::lowerDSOLocalEquivalent(
const MCSymbol *LHS, const MCSymbol *RHS, int64_t Addend,
diff --git a/llvm/test/CodeGen/ARM/plt-relative-reloc.ll b/llvm/test/CodeGen/ARM/relative-reloc.ll
similarity index 78%
rename from llvm/test/CodeGen/ARM/plt-relative-reloc.ll
rename to llvm/test/CodeGen/ARM/relative-reloc.ll
index ede891900e6d0..65053726e66bf 100644
--- a/llvm/test/CodeGen/ARM/plt-relative-reloc.ll
+++ b/llvm/test/CodeGen/ARM/relative-reloc.ll
@@ -10,7 +10,8 @@ declare void @fn1() unnamed_addr
declare void @fn2() unnamed_addr
declare void @fn3()
+;; Create a PC-relative relocation that the linker might decline if the addend symbol is preemptible.
; CHECK: .long 0
-; CHECK-NEXT: .long fn1(prel31)-vtable-4
-; CHECK-NEXT: .long fn2(prel31)-vtable-4
+; CHECK-NEXT: .long fn1-vtable-4
+; CHECK-NEXT: .long fn2-vtable-4
; CHECK-NEXT: .long fn3-vtable-4
diff --git a/llvm/test/CodeGen/RISCV/dso_local_equivalent.ll b/llvm/test/CodeGen/RISCV/dso_local_equivalent.ll
index e5e69b7bfe13b..daf46fa699b0f 100644
--- a/llvm/test/CodeGen/RISCV/dso_local_equivalent.ll
+++ b/llvm/test/CodeGen/RISCV/dso_local_equivalent.ll
@@ -26,9 +26,9 @@ declare void @extern_func()
; CHECK-NEXT: .word 0 # 0x0
; CHECK-NEXT: .word %pltpcrel(f0)
; CHECK-NEXT: .word %pltpcrel(f1+4)
-; CHECK-NEXT: .word f2-_ZTV1B-8
+; CHECK-NEXT: .word %pltpcrel(f2+8)
; CHECK-NEXT: .word %pltpcrel(f3+12)
-; CHECK-NEXT: .word f4-_ZTV1B-8
+; CHECK-NEXT: .word %pltpcrel(f4+16)
; CHECK-NEXT: .size _ZTV1B, 28
declare void @f0()
declare void @f1()
diff --git a/llvm/test/CodeGen/RISCV/plt-relative-reloc.ll b/llvm/test/CodeGen/RISCV/relative-reloc.ll
similarity index 84%
rename from llvm/test/CodeGen/RISCV/plt-relative-reloc.ll
rename to llvm/test/CodeGen/RISCV/relative-reloc.ll
index d2dceb773b2e9..6c94b9fce9308 100644
--- a/llvm/test/CodeGen/RISCV/plt-relative-reloc.ll
+++ b/llvm/test/CodeGen/RISCV/relative-reloc.ll
@@ -12,10 +12,11 @@ declare void @fn2() unnamed_addr
declare void @fn3()
@global4 = external unnamed_addr global i8
+;; Create a PC-relative relocation that the linker might decline if the addend symbol is preemptible.
; CHECK: vtable:
; CHECK-NEXT: .word 0 # 0x0
-; CHECK-NEXT: .word %pltpcrel(fn1)
-; CHECK-NEXT: .word %pltpcrel(fn2+4)
+; CHECK-NEXT: .word fn1-vtable-4
+; CHECK-NEXT: .word fn2-vtable-4
; CHECK-NEXT: .word fn3-vtable-4
; CHECK-NEXT: .word global4-vtable-4
; CHECK-NEXT: .size vtable, 20
diff --git a/llvm/test/CodeGen/X86/dso_local_equivalent.ll b/llvm/test/CodeGen/X86/dso_local_equivalent.ll
index ed31f62d5a750..c609042fad030 100644
--- a/llvm/test/CodeGen/X86/dso_local_equivalent.ll
+++ b/llvm/test/CodeGen/X86/dso_local_equivalent.ll
@@ -207,11 +207,27 @@ define void @call_no_name_extern() {
declare hidden void @0()
declare void @1()
+declare void @forward_extern_func()
;; Note that we keep this at the very end because llc emits this after all the
;; functions.
-; CHECK: const:
-; CHECK: .long forward_extern_func@PLT
-@const = constant i32 trunc (i64 ptrtoint (ptr dso_local_equivalent @forward_extern_func to i64) to i32)
-
-declare void @forward_extern_func()
+; CHECK-LABEL: _ZTV1B:
+; CHECK: .long 0
+; CHECK-NEXT: .long 0
+; CHECK-NEXT: .long f0@PLT-_ZTV1B-8
+; CHECK-NEXT: .long f1@PLT-_ZTV1B-8
+; CHECK-NEXT: .long f2@PLT-_ZTV1B-8
+
+@_ZTV1B = dso_local constant { [5 x i32] } { [5 x i32] [
+ i32 0,
+ i32 0,
+ i32 trunc (i64 sub (i64 ptrtoint (ptr dso_local_equivalent @f0 to i64), i64 ptrtoint (ptr getelementptr inbounds ({ [7 x i32] }, ptr @_ZTV1B, i32 0, i32 0, i32 2) to i64)) to i32),
+ i32 trunc (i64 sub (i64 ptrtoint (ptr dso_local_equivalent @f1 to i64), i64 ptrtoint (ptr getelementptr inbounds ({ [7 x i32] }, ptr @_ZTV1B, i32 0, i32 0, i32 2) to i64)) to i32),
+ i32 trunc (i64 sub (i64 ptrtoint (ptr dso_local_equivalent @f2 to i64), i64 ptrtoint (ptr getelementptr inbounds ({ [7 x i32] }, ptr @_ZTV1B, i32 0, i32 0, i32 2) to i64)) to i32)
+] }
+
+declare void @f0()
+declare void @f1()
+define dso_local void @f2() {
+ ret void
+}
diff --git a/llvm/test/CodeGen/X86/x86-plt-relative-reloc.ll b/llvm/test/CodeGen/X86/relative-reloc-32.ll
similarity index 89%
rename from llvm/test/CodeGen/X86/x86-plt-relative-reloc.ll
rename to llvm/test/CodeGen/X86/relative-reloc-32.ll
index d5e80285b160d..7d0b1fd546a00 100644
--- a/llvm/test/CodeGen/X86/x86-plt-relative-reloc.ll
+++ b/llvm/test/CodeGen/X86/relative-reloc-32.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
diff --git a/llvm/test/CodeGen/X86/x86-64-plt-relative-reloc.ll b/llvm/test/CodeGen/X86/relative-reloc-64.ll
similarity index 84%
rename from llvm/test/CodeGen/X86/x86-64-plt-relative-reloc.ll
rename to llvm/test/CodeGen/X86/relative-reloc-64.ll
index 54736c94af248..6f88edfa075b8 100644
--- a/llvm/test/CodeGen/X86/x86-64-plt-relative-reloc.ll
+++ b/llvm/test/CodeGen/X86/relative-reloc-64.ll
@@ -12,8 +12,9 @@ declare void @fn2() unnamed_addr
declare void @fn3()
@global4 = external unnamed_addr global i8
+;; Create a PC-relative relocation that the linker might decline if the addend 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
|
PiJoules
approved these changes
Apr 8, 2025
MaskRay
deleted the
users/MaskRay/spr/asmprinter-remove-elfs-special-lowerrelativereference-for-unnamed_addr-function-use-lowerdsolocalequivalent-in-more-cases
branch
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Apr 8, 2025
…r unnamed_addr function; use lowerDSOLocalEquivalent in more cases 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#134781
yozhu
reviewed
Apr 10, 2025
| // instead. Note: A default visibility symbol is by default preemptible | ||
| // during linking, and should not be referenced with PC-relative | ||
| // relocations. Therefore, use a PLT relocation even if the function is | ||
| // dso_local. |
There was a problem hiding this comment.
For AArch64, this change makes relocations for relative vtable entries always be PLT32, though PREL32 relocation also works (for dso_local) and its target symbol could get changed from function to section-plus-offset (in hope to omit the function symbol in local symbol table).
There was a problem hiding this comment.
Are you talking about the ld -r/--emit-reelocs --discard-all or objcopy --discard-all behavior?
It's true that the PLT-generating relocation, if present, keeps the symbol alive under --discard-all.
However, the advantage is that even in the object file, the dso_local_equivalent semantics are retained: we don't take the address of the symbol.
% cat x.s
f1:
f2:
.data
.long f1@plt - .
.long f2 - .
% llvm-mc -triple=aarch64 -filetype=obj x.s -o x.o && ld.lld x.o -o x -r --discard-all && readelf -W -s x
Symbol table '.symtab' contains 4 entries:
Num: Value Size Type Bind Vis Ndx Name
0: 0000000000000000 0 NOTYPE LOCAL DEFAULT UND
1: 0000000000000000 0 NOTYPE LOCAL DEFAULT 1 f1
2: 0000000000000000 0 SECTION LOCAL DEFAULT 1 .text
3: 0000000000000000 0 SECTION LOCAL DEFAULT 2 .data
There was a problem hiding this comment.
I was referring to the logic in this function, where if it is PREL32 and the target symbol is dso_local, we may update relocation target from "func + vtable_slot_offset" to "section + offset". So if the dso_local function is not referenced elsewhere, we can omit its emission in local symtab.
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).