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

Revert "[LoongArch] Use R_LARCH_ALIGN with section symbol (#84741)" #92584

Conversation

MaskRay
Copy link
Member

@MaskRay MaskRay commented May 17, 2024

This reverts commit 01f7989.

This unusual special case has been discussed on the binutils mailing
list. The approach will be revisited:
https://sourceware.org/pipermail/binutils/2024-May/134092.html

Created using spr 1.3.5-bogner
@llvmbot
Copy link
Collaborator

llvmbot commented May 17, 2024

@llvm/pr-subscribers-lld-elf
@llvm/pr-subscribers-mc

@llvm/pr-subscribers-lld

Author: Fangrui Song (MaskRay)

Changes

This reverts commit 01f7989.

This unusual special case has been discussed on the binutils mailing
list. The approach will be revisited:
https://sourceware.org/pipermail/binutils/2024-May/134092.html


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

6 Files Affected:

  • (modified) lld/ELF/InputSection.cpp (+1-5)
  • (removed) lld/test/ELF/loongarch-relax-align-ldr.s (-28)
  • (modified) lld/test/ELF/loongarch-relax-emit-relocs.s (+2-3)
  • (modified) llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchAsmBackend.cpp (+5-2)
  • (modified) llvm/test/MC/LoongArch/Relocations/relax-addsub.s (+1-1)
  • (modified) llvm/test/MC/LoongArch/Relocations/relax-align.s (+6-8)
diff --git a/lld/ELF/InputSection.cpp b/lld/ELF/InputSection.cpp
index 2a1ccd997f8bc..e6c5996c0b392 100644
--- a/lld/ELF/InputSection.cpp
+++ b/lld/ELF/InputSection.cpp
@@ -471,11 +471,7 @@ void InputSection::copyRelocations(uint8_t *buf,
         addend += sec->getFile<ELFT>()->mipsGp0;
       }
 
-      if (config->emachine == EM_LOONGARCH && type == R_LARCH_ALIGN)
-        // LoongArch psABI v2.30, the R_LARCH_ALIGN requires symbol index.
-        // If it use the section symbol, the addend should not be changed.
-        p->r_addend = addend;
-      else if (RelTy::IsRela)
+      if (RelTy::IsRela)
         p->r_addend = sym.getVA(addend) - section->getOutputSection()->addr;
       // For SHF_ALLOC sections relocated by REL, append a relocation to
       // sec->relocations so that relocateAlloc transitively called by
diff --git a/lld/test/ELF/loongarch-relax-align-ldr.s b/lld/test/ELF/loongarch-relax-align-ldr.s
deleted file mode 100644
index 6534dc906cfd0..0000000000000
--- a/lld/test/ELF/loongarch-relax-align-ldr.s
+++ /dev/null
@@ -1,28 +0,0 @@
-# REQUIRES: loongarch
-## Test `ld -r` not changes the addend of R_LARCH_ALIGN.
-
-# RUN: llvm-mc --filetype=obj --triple=loongarch64 --mattr=+relax %s -o %t.64.o
-# RUN: ld.lld -r %t.64.o %t.64.o -o %t.64.r
-# RUN: llvm-objdump -dr --no-show-raw-insn %t.64.r | FileCheck %s
-
-# CHECK:      <.text>:
-# CHECK-NEXT:   break 1
-# CHECK-NEXT:   nop
-# CHECK-NEXT:   {{0*}}04:  R_LARCH_ALIGN        .text+0x804
-# CHECK-NEXT:   nop
-# CHECK-NEXT:   nop
-# CHECK-NEXT:   break 2
-# CHECK-NEXT:   break 0
-# CHECK-NEXT:   break 0
-# CHECK-NEXT:   break 0
-# CHECK-NEXT:   break 1
-# CHECK-NEXT:   nop
-# CHECK-NEXT:   {{0*}}24:  R_LARCH_ALIGN        .text+0x804
-# CHECK-NEXT:   nop
-# CHECK-NEXT:   nop
-# CHECK-NEXT:   break 2
-
-.text
-break 1
-.p2align 4, , 8
-break 2
diff --git a/lld/test/ELF/loongarch-relax-emit-relocs.s b/lld/test/ELF/loongarch-relax-emit-relocs.s
index 9007f8fcc114f..581fce8c95caa 100644
--- a/lld/test/ELF/loongarch-relax-emit-relocs.s
+++ b/lld/test/ELF/loongarch-relax-emit-relocs.s
@@ -25,7 +25,7 @@
 # CHECK-NEXT:     R_LARCH_PCALA_LO12 _start
 # CHECK-NEXT:     R_LARCH_RELAX *ABS*
 # CHECK-NEXT:   nop
-# CHECK-NEXT:     R_LARCH_ALIGN .text+0x4
+# CHECK-NEXT:     R_LARCH_ALIGN .Lla-relax-align0+0x4
 # CHECK-NEXT:   nop
 # CHECK-NEXT:   ret
 
@@ -37,12 +37,11 @@
 # CHECKR-NEXT:     R_LARCH_PCALA_LO12 _start
 # CHECKR-NEXT:     R_LARCH_RELAX *ABS*
 # CHECKR-NEXT:   nop
-# CHECKR-NEXT:     R_LARCH_ALIGN .text+0x4
+# CHECKR-NEXT:     R_LARCH_ALIGN .Lla-relax-align0+0x4
 # CHECKR-NEXT:   nop
 # CHECKR-NEXT:   nop
 # CHECKR-NEXT:   ret
 
-.text
 .global _start
 _start:
   la.pcrel $a0, _start
diff --git a/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchAsmBackend.cpp b/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchAsmBackend.cpp
index 98f5014a34b1d..de492f2b1f0a4 100644
--- a/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchAsmBackend.cpp
+++ b/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchAsmBackend.cpp
@@ -226,8 +226,11 @@ bool LoongArchAsmBackend::shouldInsertFixupForCodeAlign(
       MCFixup::create(0, Dummy, MCFixupKind(LoongArch::fixup_loongarch_align));
   const MCSymbolRefExpr *MCSym = getSecToAlignSym()[Sec];
   if (MCSym == nullptr) {
-    // Use section symbol directly.
-    MCSym = MCSymbolRefExpr::create(Sec->getBeginSymbol(), Ctx);
+    // Create a symbol and make the value of symbol is zero.
+    MCSymbol *Sym = Ctx.createNamedTempSymbol("la-relax-align");
+    Sym->setFragment(&*Sec->getBeginSymbol()->getFragment());
+    Asm.registerSymbol(*Sym);
+    MCSym = MCSymbolRefExpr::create(Sym, Ctx);
     getSecToAlignSym()[Sec] = MCSym;
   }
 
diff --git a/llvm/test/MC/LoongArch/Relocations/relax-addsub.s b/llvm/test/MC/LoongArch/Relocations/relax-addsub.s
index 0e27d6301bb3c..18e0ede5e2937 100644
--- a/llvm/test/MC/LoongArch/Relocations/relax-addsub.s
+++ b/llvm/test/MC/LoongArch/Relocations/relax-addsub.s
@@ -28,7 +28,7 @@
 
 # RELAX:       Relocations [
 # RELAX-NEXT:    Section ({{.*}}) .rela.text {
-# RELAX-NEXT:      0x4 R_LARCH_ALIGN .text 0x4
+# RELAX-NEXT:      0x4 R_LARCH_ALIGN {{.*}} 0x4
 # RELAX-NEXT:      0x10 R_LARCH_PCALA_HI20 .L1 0x0
 # RELAX-NEXT:      0x10 R_LARCH_RELAX - 0x0
 # RELAX-NEXT:      0x14 R_LARCH_PCALA_LO12 .L1 0x0
diff --git a/llvm/test/MC/LoongArch/Relocations/relax-align.s b/llvm/test/MC/LoongArch/Relocations/relax-align.s
index 0246d5b46431c..294fd9fb916c7 100644
--- a/llvm/test/MC/LoongArch/Relocations/relax-align.s
+++ b/llvm/test/MC/LoongArch/Relocations/relax-align.s
@@ -63,19 +63,17 @@ ret
 ## Test the symbol index is different from .text.
 .section .text2, "ax"
 .p2align 4
-.p2align 4, , 4
 break 7
 
 # RELOC:            Relocations [
 # RELAX-RELOC-NEXT:   Section ({{.*}}) .rela.text {
-# RELAX-RELOC-NEXT:     0x24 R_LARCH_ALIGN .text 0x4
-# RELAX-RELOC-NEXT:     0x34 R_LARCH_ALIGN .text 0x5
-# RELAX-RELOC-NEXT:     0x50 R_LARCH_ALIGN .text 0x4
-# RELAX-RELOC-NEXT:     0x60 R_LARCH_ALIGN .text 0xB04
-# RELAX-RELOC-NEXT:     0x70 R_LARCH_ALIGN .text 0x4
+# RELAX-RELOC-NEXT:     0x24 R_LARCH_ALIGN .Lla-relax-align0 0x4
+# RELAX-RELOC-NEXT:     0x34 R_LARCH_ALIGN .Lla-relax-align0 0x5
+# RELAX-RELOC-NEXT:     0x50 R_LARCH_ALIGN .Lla-relax-align0 0x4
+# RELAX-RELOC-NEXT:     0x60 R_LARCH_ALIGN .Lla-relax-align0 0xB04
+# RELAX-RELOC-NEXT:     0x70 R_LARCH_ALIGN .Lla-relax-align0 0x4
 # RELAX-RELOC-NEXT:   }
 # RELAX-RELOC-NEXT:   Section ({{.*}}) .rela.text2 {
-# RELAX-RELOC-NEXT:     0x0 R_LARCH_ALIGN .text2 0x4
-# RELAX-RELOC-NEXT:     0xC R_LARCH_ALIGN .text2 0x404
+# RELAX-RELOC-NEXT:     0x0 R_LARCH_ALIGN .Lla-relax-align1 0x4
 # RELAX-RELOC-NEXT:   }
 # RELOC-NEXT:       ]

@MaskRay MaskRay requested a review from SixWeining May 17, 2024 17:35
@MaskRay
Copy link
Member Author

MaskRay commented May 17, 2024

@MQ-mengqing

@MaskRay MaskRay merged commit faf39f4 into main May 18, 2024
8 of 9 checks passed
@MaskRay MaskRay deleted the users/MaskRay/spr/revert-loongarch-use-r_larch_align-with-section-symbol-84741 branch May 18, 2024 02:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants