[llvm-branch-commits] [llvm] 47ee66b - [ARM] [Windows] Use IMAGE_SYM_CLASS_STATIC for private functions (#101828)

[Tobias Hieta via llvm-branch-commits]

Author: Martin Storsjö
Date: 2024-08-05T10:22:05+02:00
New Revision: 47ee66b9b969af834cfd9df82273ed9fe0418802

URL: https://github.com/llvm/llvm-project/commit/47ee66b9b969af834cfd9df82273ed9fe0418802
DIFF: https://github.com/llvm/llvm-project/commit/47ee66b9b969af834cfd9df82273ed9fe0418802.diff

LOG: [ARM] [Windows] Use IMAGE_SYM_CLASS_STATIC for private functions (#101828)

For functions with private linkage, pick
IMAGE_SYM_CLASS_STATIC rather than IMAGE_SYM_CLASS_EXTERNAL;
GlobalValue::isInternalLinkage() only checks for
InternalLinkage, while GlobalValue::isLocalLinkage() checks for both
InternalLinkage and PrivateLinkage.

This matches what the AArch64 target does, since commit
3406934e4db4bf95c230db072608ed062c13ad5b.

This activates a preexisting fix for the AArch64 target from
1e7f592a890aad860605cf5220530b3744e107ba, for the ARM target as well.

When a relocation points at a symbol, one usually can convey an offset
to the symbol by encoding it as an immediate in the instruction.
However, for the ARM and AArch64 branch instructions, the immediate
stored in the instruction is ignored by MS link.exe (and lld-link
matches this aspect). (It would be simple to extend lld-link to support
it - but such object files would be incompatible with MS link.exe.)

This was worked around by 1e7f592a890aad860605cf5220530b3744e107ba by
emitting symbols into the object file symbol table, for temporary
symbols that otherwise would have been omitted, if they have the class
IMAGE_SYM_CLASS_STATIC, in order to avoid needing an offset in the
relocated instruction.

This change gives the symbols generated from functions with the IR level
"private" linkage the right class, to activate that workaround.

This fixes https://github.com/llvm/llvm-project/issues/100101, fixing
code generation for coroutines for Windows on ARM. After the change in
f78688134026686288a8d310b493d9327753a022, coroutines generate a function
with private linkage, and calls to this function were previously broken
for this target.

(cherry picked from commit 8dd065d5bc81b0c8ab57f365bb169a5d92928f25)

Added: 
    llvm/test/CodeGen/ARM/Windows/private-func.ll
    llvm/test/MC/ARM/Windows/branch-reloc-offset.s

Modified: 
    llvm/lib/Target/ARM/ARMAsmPrinter.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Target/ARM/ARMAsmPrinter.cpp b/llvm/lib/Target/ARM/ARMAsmPrinter.cpp
index 642739a29d6b0..96d7074e6ef37 100644
--- a/llvm/lib/Target/ARM/ARMAsmPrinter.cpp
+++ b/llvm/lib/Target/ARM/ARMAsmPrinter.cpp
@@ -153,9 +153,9 @@ bool ARMAsmPrinter::runOnMachineFunction(MachineFunction &MF) {
     OptimizationGoals = 0;
 
   if (Subtarget->isTargetCOFF()) {
-    bool Internal = F.hasInternalLinkage();
-    COFF::SymbolStorageClass Scl = Internal ? COFF::IMAGE_SYM_CLASS_STATIC
-                                            : COFF::IMAGE_SYM_CLASS_EXTERNAL;
+    bool Local = F.hasLocalLinkage();
+    COFF::SymbolStorageClass Scl =
+        Local ? COFF::IMAGE_SYM_CLASS_STATIC : COFF::IMAGE_SYM_CLASS_EXTERNAL;
     int Type = COFF::IMAGE_SYM_DTYPE_FUNCTION << COFF::SCT_COMPLEX_TYPE_SHIFT;
 
     OutStreamer->beginCOFFSymbolDef(CurrentFnSym);

diff  --git a/llvm/test/CodeGen/ARM/Windows/private-func.ll b/llvm/test/CodeGen/ARM/Windows/private-func.ll
new file mode 100644
index 0000000000000..2d030ae3fabbb
--- /dev/null
+++ b/llvm/test/CodeGen/ARM/Windows/private-func.ll
@@ -0,0 +1,17 @@
+; RUN: llc -mtriple thumbv7-windows -filetype asm -o - %s | FileCheck %s
+
+define dso_local void @func1() {
+entry:
+  call void @func2()
+  ret void
+}
+
+define private void @func2() {
+entry:
+  ret void
+}
+
+; CHECK:      .def    .Lfunc2;
+; CHECK-NEXT: .scl    3;
+; CHECK-NEXT: .type   32;
+; CHECK-NEXT: .endef

diff  --git a/llvm/test/MC/ARM/Windows/branch-reloc-offset.s b/llvm/test/MC/ARM/Windows/branch-reloc-offset.s
new file mode 100644
index 0000000000000..2e70a723ccf78
--- /dev/null
+++ b/llvm/test/MC/ARM/Windows/branch-reloc-offset.s
@@ -0,0 +1,57 @@
+// RUN: llvm-mc -triple thumbv7-windows-gnu -filetype obj %s -o - | llvm-objdump -D -r - | FileCheck %s
+
+    .text
+main:
+    nop
+    b .Ltarget
+    b .Lother_target
+
+// A private label target in the same section
+    .def .Ltarget
+    .scl 3
+    .type 32
+    .endef
+    .p2align 2
+.Ltarget:
+    bx lr
+
+// A private label target in another section
+    .section "other", "xr"
+    nop
+    nop
+    nop
+    nop
+    nop
+    nop
+    nop
+    nop
+    .def .Lother_target
+    .scl 3
+    .type 32
+    .endef
+    .p2align 2
+.Lother_target:
+    bx lr
+
+// Check that both branches have a relocation with a zero offset.
+//
+// CHECK: 00000000 <main>:
+// CHECK:        0: bf00          nop
+// CHECK:        2: f000 b800     b.w     0x6 <main+0x6>          @ imm = #0x0
+// CHECK:                         00000002:  IMAGE_REL_ARM_BRANCH24T      .Ltarget
+// CHECK:        6: f000 b800     b.w     0xa <main+0xa>          @ imm = #0x0
+// CHECK:                         00000006:  IMAGE_REL_ARM_BRANCH24T      .Lother_target
+// CHECK:        a: bf00          nop
+// CHECK: 0000000c <.Ltarget>:
+// CHECK:        c: 4770          bx      lr
+// CHECK: 00000000 <other>:
+// CHECK:        0: bf00          nop
+// CHECK:        2: bf00          nop
+// CHECK:        4: bf00          nop
+// CHECK:        6: bf00          nop
+// CHECK:        8: bf00          nop
+// CHECK:        a: bf00          nop
+// CHECK:        c: bf00          nop
+// CHECK:        e: bf00          nop
+// CHECK: 00000010 <.Lother_target>:
+// CHECK:       10: 4770          bx      lr