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

[llvm] Support indirect symbol replacement with GOTPCREL for x86_64 ELF #67754

Merged
merged 1 commit into from
Nov 15, 2023

Conversation

PiJoules
Copy link
Contributor

There's an existing check in LLVM that can replace an offset to a DSO-local symbol that is just a pointer to another symbol with a GOTPCREL reloc. This is exactly what the RTTI proxy in the relative vtables ABI is. This feature is supported for different macho platforms, but not for ELF. This extends that support.

Additionally allow negative offsets before checking if we can support GOTPCREL. These offsets can be negative.

@PiJoules PiJoules self-assigned this Sep 29, 2023
@llvmbot llvmbot added the mc Machine (object) code label Sep 29, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Sep 29, 2023

@llvm/pr-subscribers-mc

@llvm/pr-subscribers-backend-x86

Changes

There's an existing check in LLVM that can replace an offset to a DSO-local symbol that is just a pointer to another symbol with a GOTPCREL reloc. This is exactly what the RTTI proxy in the relative vtables ABI is. This feature is supported for different macho platforms, but not for ELF. This extends that support.

Additionally allow negative offsets before checking if we can support GOTPCREL. These offsets can be negative.


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

4 Files Affected:

  • (modified) llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp (-5)
  • (modified) llvm/lib/Target/X86/X86TargetObjectFile.cpp (+10)
  • (modified) llvm/lib/Target/X86/X86TargetObjectFile.h (+7)
  • (added) llvm/test/MC/ELF/rtti-proxy-gotpcrel.ll (+13)
diff --git a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
index 0c4ea1b3d9f04d9..4a79c6157b17125 100644
--- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
@@ -3479,12 +3479,7 @@ static void handleIndirectSymViaGOTPCRel(AsmPrinter &AP, const MCExpr **ME,
   //
   //    gotpcrelcst := <offset from @foo base> + <cst>
   //
-  // If gotpcrelcst is positive it means that we can safely fold the pc rel
-  // displacement into the GOTPCREL. We can also can have an extra offset <cst>
-  // if the target knows how to encode it.
   int64_t GOTPCRelCst = Offset + MV.getConstant();
-  if (GOTPCRelCst < 0)
-    return;
   if (!AP.getObjFileLowering().supportGOTPCRelWithOffset() && GOTPCRelCst != 0)
     return;
 
diff --git a/llvm/lib/Target/X86/X86TargetObjectFile.cpp b/llvm/lib/Target/X86/X86TargetObjectFile.cpp
index b88ad5a478f390a..53c692060f08cf1 100644
--- a/llvm/lib/Target/X86/X86TargetObjectFile.cpp
+++ b/llvm/lib/Target/X86/X86TargetObjectFile.cpp
@@ -56,3 +56,13 @@ const MCExpr *X86ELFTargetObjectFile::getDebugThreadLocalSymbol(
     const MCSymbol *Sym) const {
   return MCSymbolRefExpr::create(Sym, MCSymbolRefExpr::VK_DTPOFF, getContext());
 }
+
+const MCExpr *X86ELFTargetObjectFile::getIndirectSymViaGOTPCRel(
+    const GlobalValue *GV, const MCSymbol *Sym, const MCValue &MV,
+    int64_t Offset, MachineModuleInfo *MMI, MCStreamer &Streamer) const {
+  int64_t FinalOffset = Offset + MV.getConstant();
+  const MCExpr *Res =
+      MCSymbolRefExpr::create(Sym, MCSymbolRefExpr::VK_GOTPCREL, getContext());
+  const MCExpr *Off = MCConstantExpr::create(FinalOffset, getContext());
+  return MCBinaryExpr::createAdd(Res, Off, getContext());
+}
diff --git a/llvm/lib/Target/X86/X86TargetObjectFile.h b/llvm/lib/Target/X86/X86TargetObjectFile.h
index f4bf52c83771ff4..ed9390d1fad1a26 100644
--- a/llvm/lib/Target/X86/X86TargetObjectFile.h
+++ b/llvm/lib/Target/X86/X86TargetObjectFile.h
@@ -42,9 +42,16 @@ namespace llvm {
   public:
     X86ELFTargetObjectFile() {
       PLTRelativeVariantKind = MCSymbolRefExpr::VK_PLT;
+      SupportIndirectSymViaGOTPCRel = true;
     }
     /// Describe a TLS variable address within debug info.
     const MCExpr *getDebugThreadLocalSymbol(const MCSymbol *Sym) const override;
+
+    const MCExpr *
+    getIndirectSymViaGOTPCRel(const GlobalValue *GV, const MCSymbol *Sym,
+                              const MCValue &MV, int64_t Offset,
+                              MachineModuleInfo *MMI,
+                              MCStreamer &Streamer) const override;
   };
 
 } // end namespace llvm
diff --git a/llvm/test/MC/ELF/rtti-proxy-gotpcrel.ll b/llvm/test/MC/ELF/rtti-proxy-gotpcrel.ll
new file mode 100644
index 000000000000000..a61af96b399895f
--- /dev/null
+++ b/llvm/test/MC/ELF/rtti-proxy-gotpcrel.ll
@@ -0,0 +1,13 @@
+; RUN: llc %s -mtriple=x86_64-unknown-fuchsia  -o - | FileCheck %s
+
+@vtable = dso_local unnamed_addr constant i32 trunc (i64 sub (i64 ptrtoint (i8** @rtti.proxy to i64), i64 ptrtoint (i32* @vtable to i64)) to i32), align 4
+@vtable_with_offset = dso_local unnamed_addr constant { [2 x i32] } { [2 x i32] [i32 0, i32 trunc (i64 sub (i64 ptrtoint (i8** @rtti.proxy to i64), i64 ptrtoint ({ [2 x i32] }* @vtable_with_offset to i64)) to i32)] }, align 4
+@rtti = external global i8, align 8
+@rtti.proxy = linkonce_odr hidden unnamed_addr constant i8* @rtti
+
+; CHECK-NOT: rtti.proxy
+; CHECK-LABEL: vtable:
+; CHECK:         .{{word|long}}   rtti@GOTPCREL+0{{$}}
+
+; CHECK-LABEL: vtable_with_offset:
+; CHECK:         .{{word|long}}   rtti@GOTPCREL+4{{$}}

@petrhosek
Copy link
Member

Can you split the change to allow negative offsets to a separate PR and add a test for it?

@PiJoules
Copy link
Contributor Author

PiJoules commented Nov 14, 2023

Can you split the change to allow negative offsets to a separate PR and add a test for it?

#72308

There's an existing check in LLVM that can replace an offset to a
DSO-local symbol that is just a pointer to another symbol with a
GOTPCREL reloc. This is exactly what the RTTI proxy in the relative
vtables ABI is. This feature is supported for different macho platforms,
but not for ELF. This extends that support.
@PiJoules PiJoules force-pushed the support-gotpcrel-replacement-only-x86 branch from 4ecb382 to 1511335 Compare November 14, 2023 21:35
@PiJoules PiJoules merged commit 322799a into llvm:main Nov 15, 2023
3 checks passed
@PiJoules PiJoules deleted the support-gotpcrel-replacement-only-x86 branch November 15, 2023 23:13
zahiraam pushed a commit to zahiraam/llvm-project that referenced this pull request Nov 20, 2023
…LF (llvm#67754)

There's an existing check in LLVM that can replace an offset to a
DSO-local symbol that is just a pointer to another symbol with a
GOTPCREL reloc. This is exactly what the RTTI proxy in the relative
vtables ABI is. This feature is supported for different macho platforms,
but not for ELF. This extends that support.
PiJoules added a commit to PiJoules/llvm-project that referenced this pull request Jan 13, 2024
…v64 ELF

This is similar to llvm#67754 but
adds support for ELF aarch64 and riscv64 now that GOTPCREL-equivalent
relocations have been added for those archs.
PiJoules added a commit to PiJoules/llvm-project that referenced this pull request Jan 16, 2024
…v64 ELF

This is similar to llvm#67754 but
adds support for ELF aarch64 and riscv64 now that GOTPCREL-equivalent
relocations have been added for those archs.
PiJoules added a commit that referenced this pull request Jan 16, 2024
#78003)

…v64 ELF

This is similar to #67754 but
adds support for ELF aarch64 and riscv64 now that GOTPCREL-equivalent
relocations have been added for those archs.
justinfargnoli pushed a commit to justinfargnoli/llvm-project that referenced this pull request Jan 28, 2024
llvm#78003)

…v64 ELF

This is similar to llvm#67754 but
adds support for ELF aarch64 and riscv64 now that GOTPCREL-equivalent
relocations have been added for those archs.
smeenai added a commit that referenced this pull request Feb 16, 2024
R_ARM_GOT_PREL is equivalent to GOTPCREL on other architectures, so we
can use it for indirect symbol replacement the same way. This is the
equivalent of #67754 for x86_64
and #78003 for AArch64 and
64-bit RISC-V.
qihangkong pushed a commit to rvgpu/llvm that referenced this pull request Apr 18, 2024
R_ARM_GOT_PREL is equivalent to GOTPCREL on other architectures, so we
can use it for indirect symbol replacement the same way. This is the
equivalent of llvm/llvm-project#67754 for x86_64
and llvm/llvm-project#78003 for AArch64 and
64-bit RISC-V.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:X86 mc Machine (object) code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants