[Driver] Default enable LoongArch linker relaxation #111488

ywgrit · 2024-10-08T06:18:58Z

No description provided.

github-actions · 2024-10-08T06:19:19Z

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

ywgrit · 2024-10-08T06:19:38Z

@MaskRay @SixWeining @wangleiat @xen0n

llvmbot · 2024-10-08T06:19:54Z

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

@llvm/pr-subscribers-clang

Author: None (ywgrit)

Changes

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

4 Files Affected:

(modified) clang/lib/Driver/ToolChains/Arch/LoongArch.cpp (+7)
(modified) lld/ELF/Writer.cpp (+4-1)
(modified) lld/test/ELF/loongarch-relax-align.s (+169-40)
(modified) lld/test/ELF/loongarch-relax-emit-relocs.s (+2-3)

diff --git a/clang/lib/Driver/ToolChains/Arch/LoongArch.cpp b/clang/lib/Driver/ToolChains/Arch/LoongArch.cpp
index 771adade93813f..74d3ab65ed6643 100644
--- a/clang/lib/Driver/ToolChains/Arch/LoongArch.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/LoongArch.cpp
@@ -134,6 +134,13 @@ void loongarch::getLoongArchTargetFeatures(const Driver &D,
       (!Args.hasArgNoClaim(clang::driver::options::OPT_march_EQ)))
     Features.push_back("+lsx");
 
+  // -mrelax is default, unless -mno-relax is specified.
+  if (Args.hasFlag(options::OPT_mrelax, options::OPT_mno_relax, true)) {
+    Features.push_back("+relax");
+  } else {
+    Features.push_back("-relax");
+  }
+
   std::string ArchName;
   if (const Arg *A = Args.getLastArg(options::OPT_march_EQ))
     ArchName = A->getValue();
diff --git a/lld/ELF/Writer.cpp b/lld/ELF/Writer.cpp
index f4a22ea953ec49..cf0ff432f920ad 100644
--- a/lld/ELF/Writer.cpp
+++ b/lld/ELF/Writer.cpp
@@ -1462,6 +1462,8 @@ template <class ELFT> void Writer<ELFT>::finalizeAddressDependentContent() {
   for (;;) {
     bool changed = ctx.target->needsThunks
                        ? tc.createThunks(pass, ctx.outputSections)
+                   : ctx.arg.emachine == EM_LOONGARCH && !ctx.arg.relax
+                       ? false
                        : ctx.target->relaxOnce(pass);
     bool spilled = ctx.script->spillSections();
     changed |= spilled;
@@ -1545,7 +1547,8 @@ template <class ELFT> void Writer<ELFT>::finalizeAddressDependentContent() {
       finalizeOrderDependentContent();
     }
   }
-  if (!ctx.arg.relocatable)
+  if (!ctx.arg.relocatable &&
+      !(ctx.arg.emachine == EM_LOONGARCH && !ctx.arg.relax))
     ctx.target->finalizeRelax(pass);
 
   if (ctx.arg.relocatable)
diff --git a/lld/test/ELF/loongarch-relax-align.s b/lld/test/ELF/loongarch-relax-align.s
index ab61e15d5caca2..158f1f80ba8489 100644
--- a/lld/test/ELF/loongarch-relax-align.s
+++ b/lld/test/ELF/loongarch-relax-align.s
@@ -6,56 +6,180 @@
 # RUN: ld.lld --section-start=.text=0x10000 --section-start=.text2=0x20000 -e 0 %t.64.o -o %t.64
 # RUN: ld.lld --section-start=.text=0x10000 --section-start=.text2=0x20000 -e 0 %t.32.o --no-relax -o %t.32n
 # RUN: ld.lld --section-start=.text=0x10000 --section-start=.text2=0x20000 -e 0 %t.64.o --no-relax -o %t.64n
-# RUN: llvm-objdump -td --no-show-raw-insn %t.32 | FileCheck %s
-# RUN: llvm-objdump -td --no-show-raw-insn %t.64 | FileCheck %s
-# RUN: llvm-objdump -td --no-show-raw-insn %t.32n | FileCheck %s
-# RUN: llvm-objdump -td --no-show-raw-insn %t.64n | FileCheck %s
+# RUN: llvm-objdump -td --no-show-raw-insn %t.32 | FileCheck %s --check-prefix=RELAX32
+# RUN: llvm-objdump -td --no-show-raw-insn %t.64 | FileCheck %s --check-prefixes=RELAX64,SRELAX64
+# RUN: llvm-objdump -td --no-show-raw-insn %t.32n | FileCheck %s --check-prefix=NORELAX
+# RUN: llvm-objdump -td --no-show-raw-insn %t.64n | FileCheck %s --check-prefix=NORELAX
 
 ## Test the R_LARCH_ALIGN without symbol index.
 # RUN: llvm-mc --filetype=obj --triple=loongarch64 --mattr=+relax %s -o %t.o64.o --defsym=old=1
 # RUN: ld.lld --section-start=.text=0x10000 --section-start=.text2=0x20000 -e 0 %t.o64.o -o %t.o64
 # RUN: ld.lld --section-start=.text=0x10000 --section-start=.text2=0x20000 -e 0 %t.o64.o --no-relax -o %t.o64n
-# RUN: llvm-objdump -td --no-show-raw-insn %t.o64 | FileCheck %s
-# RUN: llvm-objdump -td --no-show-raw-insn %t.o64n | FileCheck %s
+# RUN: llvm-objdump -td --no-show-raw-insn %t.o64 | FileCheck %s --check-prefixes=RELAX64,ORELAX64
+# RUN: llvm-objdump -td --no-show-raw-insn %t.o64n | FileCheck %s --check-prefix=ONORELAX
 
 ## -r keeps section contents unchanged.
 # RUN: ld.lld -r %t.64.o -o %t.64.r
 # RUN: llvm-objdump -dr --no-show-raw-insn %t.64.r | FileCheck %s --check-prefix=CHECKR
 
-# CHECK-DAG: {{0*}}10000 l .text  {{0*}}44 .Ltext_start
-# CHECK-DAG: {{0*}}10038 l .text  {{0*}}0c .L1
-# CHECK-DAG: {{0*}}10040 l .text  {{0*}}04 .L2
-# CHECK-DAG: {{0*}}20000 l .text2 {{0*}}14 .Ltext2_start
-
-# CHECK:      <.Ltext_start>:
-# CHECK-NEXT:   break 1
-# CHECK-NEXT:   break 2
-# CHECK-NEXT:   nop
-# CHECK-NEXT:   nop
-# CHECK-NEXT:   break 3
-# CHECK-NEXT:   break 4
-# CHECK-NEXT:   nop
-# CHECK-NEXT:   nop
-# CHECK-NEXT:   pcalau12i     $a0, 0
-# CHECK-NEXT:   addi.{{[dw]}} $a0, $a0, 0
-# CHECK-NEXT:   pcalau12i     $a0, 0
-# CHECK-NEXT:   addi.{{[dw]}} $a0, $a0, 56
-# CHECK-NEXT:   pcalau12i     $a0, 0
-# CHECK-NEXT:   addi.{{[dw]}} $a0, $a0, 64
-# CHECK-EMPTY:
-# CHECK-NEXT: <.L1>:
-# CHECK-NEXT:   nop
-# CHECK-NEXT:   nop
-# CHECK-EMPTY:
-# CHECK-NEXT: <.L2>:
-# CHECK-NEXT:   break 5
-
-# CHECK:      <.Ltext2_start>:
-# CHECK-NEXT:   pcalau12i     $a0, 0
-# CHECK-NEXT:   addi.{{[dw]}} $a0, $a0, 0
-# CHECK-NEXT:   nop
-# CHECK-NEXT:   nop
-# CHECK-NEXT:   break 6
+
+# RELAX32-DAG: {{0*}}10000 l .text  {{0*}}00 .Lalign_symbol
+# RELAX32-DAG: {{0*}}10000 l .text  {{0*}}44 .Ltext_start
+# RELAX32-DAG: {{0*}}10038 l .text  {{0*}}0c .L1
+# RELAX32-DAG: {{0*}}10040 l .text  {{0*}}04 .L2
+# RELAX32-DAG: {{0*}}20000 l .text2 {{0*}}14 .Ltext2_start
+
+# RELAX32:      <.Ltext_start>:
+# RELAX32-NEXT:   break 1
+# RELAX32-NEXT:   break 2
+# RELAX32-NEXT:   nop
+# RELAX32-NEXT:   nop
+# RELAX32-NEXT:   break 3
+# RELAX32-NEXT:   break 4
+# RELAX32-NEXT:   nop
+# RELAX32-NEXT:   nop
+# RELAX32-NEXT:   pcalau12i     $a0, 0
+# RELAX32-NEXT:   addi.{{[dw]}} $a0, $a0, 0
+# RELAX32-NEXT:   pcalau12i     $a0, 0
+# RELAX32-NEXT:   addi.{{[dw]}} $a0, $a0, 56
+# RELAX32-NEXT:   pcalau12i     $a0, 0
+# RELAX32-NEXT:   addi.{{[dw]}} $a0, $a0, 64
+# RELAX32-EMPTY:
+# RELAX32-NEXT: <.L1>:
+# RELAX32-NEXT:   nop
+# RELAX32-NEXT:   nop
+# RELAX32-EMPTY:
+# RELAX32-NEXT: <.L2>:
+# RELAX32-NEXT:   break 5
+
+# RELAX32:      <.Ltext2_start>:
+# RELAX32-NEXT:   pcalau12i     $a0, 0
+# RELAX32-NEXT:   addi.{{[dw]}} $a0, $a0, 0
+# RELAX32-NEXT:   nop
+# RELAX32-NEXT:   nop
+# RELAX32-NEXT:   break 6
+
+# NORELAX-DAG: {{0*}}10000 l .text  {{0*}}00 .Lalign_symbol
+# NORELAX-DAG: {{0*}}10000 l .text  {{0*}}5c .Ltext_start
+# NORELAX-DAG: {{0*}}1004c l .text  {{0*}}10 .L1
+# NORELAX-DAG: {{0*}}10058 l .text  {{0*}}04 .L2
+# NORELAX-DAG: {{0*}}20000 l .text2 {{0*}}18 .Ltext2_start
+
+# NORELAX:      <.Ltext_start>:
+# NORELAX-NEXT:   break 1
+# NORELAX-NEXT:   break 2
+# NORELAX-NEXT:   nop
+# NORELAX-NEXT:   nop
+# NORELAX-NEXT:   nop
+# NORELAX-NEXT:   break 3
+# NORELAX-NEXT:   nop
+# NORELAX-NEXT:   nop
+# NORELAX-NEXT:   nop
+# NORELAX-NEXT:   break 4
+# NORELAX-NEXT:   nop
+# NORELAX-NEXT:   nop
+# NORELAX-NEXT:   nop
+# NORELAX-NEXT:   pcalau12i     $a0, 0
+# NORELAX-NEXT:   addi.{{[dw]}} $a0, $a0, 0
+# NORELAX-NEXT:   pcalau12i     $a0, 0
+# NORELAX-NEXT:   addi.{{[dw]}} $a0, $a0, 76
+# NORELAX-NEXT:   pcalau12i     $a0, 0
+# NORELAX-NEXT:   addi.{{[dw]}} $a0, $a0, 88
+# NORELAX-EMPTY:
+# NORELAX-NEXT: <.L1>:
+# NORELAX-NEXT:   nop
+# NORELAX-NEXT:   nop
+# NORELAX-NEXT:   nop
+# NORELAX-EMPTY:
+# NORELAX-NEXT: <.L2>:
+# NORELAX-NEXT:   break 5
+
+# NORELAX:      <.Ltext2_start>:
+# NORELAX-NEXT:   pcalau12i     $a0, 0
+# NORELAX-NEXT:   addi.{{[dw]}} $a0, $a0, 0
+# NORELAX-NEXT:   nop
+# NORELAX-NEXT:   nop
+# NORELAX-NEXT:   nop
+# NORELAX-NEXT:   break 6
+
+
+
+# ORELAX64-DAG: {{0*}}00001 l *ABS*  {{0*}}00 old
+# SRELAX64-DAG: {{0*}}10000 l .text  {{0*}}00 .Lalign_symbol
+# RELAX64-DAG: {{0*}}10000 l .text  {{0*}}44 .Ltext_start
+# RELAX64-DAG: {{0*}}10038 l .text  {{0*}}0c .L1
+# RELAX64-DAG: {{0*}}10040 l .text  {{0*}}04 .L2
+# RELAX64-DAG: {{0*}}20000 l .text2 {{0*}}14 .Ltext2_start
+
+# RELAX64:      <.Ltext_start>:
+# RELAX64-NEXT:   break 1
+# RELAX64-NEXT:   break 2
+# RELAX64-NEXT:   nop
+# RELAX64-NEXT:   nop
+# RELAX64-NEXT:   break 3
+# RELAX64-NEXT:   break 4
+# RELAX64-NEXT:   nop
+# RELAX64-NEXT:   nop
+# RELAX64-NEXT:   pcalau12i  $a0, 0
+# RELAX64-NEXT:   addi.d  $a0, $a0, 0
+# RELAX64-NEXT:   pcalau12i  $a0, 0
+# RELAX64-NEXT:   addi.d  $a0, $a0, 56
+# RELAX64-NEXT:   pcalau12i  $a0, 0
+# RELAX64-NEXT:   addi.d  $a0, $a0, 64
+# RELAX64-EMPTY:
+# RELAX64-NEXT: <.L1>:
+# RELAX64-NEXT:   nop
+# RELAX64-NEXT:   nop
+# RELAX64-EMPTY:
+# RELAX64-NEXT: <.L2>:
+# RELAX64-NEXT:   break 5
+
+# RELAX64:      <.Ltext2_start>:
+# RELAX64-NEXT:   pcalau12i  $a0, 0
+# RELAX64-NEXT:   addi.d  $a0, $a0, 0
+# RELAX64-NEXT:   nop
+# RELAX64-NEXT:   nop
+# RELAX64-NEXT:   break 6
+
+
+# ONORELAX-DAG: {{0*}}00001 l *ABS*  {{0*}}00 old
+# ONORELAX-DAG: {{0*}}10000 l .text  {{0*}}4c .Ltext_start
+# ONORELAX-DAG: {{0*}}1003c l .text  {{0*}}10 .L1
+# ONORELAX-DAG: {{0*}}10048 l .text  {{0*}}04 .L2
+# ONORELAX-DAG: {{0*}}20000 l .text2 {{0*}}18 .Ltext2_start
+
+# ONORELAX:      <.Ltext_start>:
+# ONORELAX-NEXT:   break 1
+# ONORELAX-NEXT:   break 2
+# ONORELAX-NEXT:   nop
+# ONORELAX-NEXT:   nop
+# ONORELAX-NEXT:   nop
+# ONORELAX-NEXT:   break 3
+# ONORELAX-NEXT:   break 4
+# ONORELAX-NEXT:   nop
+# ONORELAX-NEXT:   nop
+# ONORELAX-NEXT:   pcalau12i     $a0, 0
+# ONORELAX-NEXT:   addi.{{[dw]}} $a0, $a0, 0
+# ONORELAX-NEXT:   pcalau12i     $a0, 0
+# ONORELAX-NEXT:   addi.{{[dw]}} $a0, $a0, 60
+# ONORELAX-NEXT:   pcalau12i     $a0, 0
+# ONORELAX-NEXT:   addi.{{[dw]}} $a0, $a0, 72
+# ONORELAX-EMPTY:
+# ONORELAX-NEXT: <.L1>:
+# ONORELAX-NEXT:   nop
+# ONORELAX-NEXT:   nop
+# ONORELAX-NEXT:   nop
+# ONORELAX-EMPTY:
+# ONORELAX-NEXT: <.L2>:
+# ONORELAX-NEXT:   break 5
+
+# ONORELAX:      <.Ltext2_start>:
+# ONORELAX-NEXT:   pcalau12i     $a0, 0
+# ONORELAX-NEXT:   addi.{{[dw]}} $a0, $a0, 0
+# ONORELAX-NEXT:   nop
+# ONORELAX-NEXT:   nop
+# ONORELAX-NEXT:   nop
+# ONORELAX-NEXT:   break 6
 
 # CHECKR:      <.Ltext2_start>:
 # CHECKR-NEXT:   pcalau12i $a0, 0
@@ -70,6 +194,11 @@
 # CHECKR-NEXT:   nop
 # CHECKR-NEXT:   break 6
 
+
+
+
+
+
 .macro .fake_p2align_4 max=0
   .ifdef old
     .if \max==0
diff --git a/lld/test/ELF/loongarch-relax-emit-relocs.s b/lld/test/ELF/loongarch-relax-emit-relocs.s
index ba414e8c93f0fb..1feec64d722cab 100644
--- a/lld/test/ELF/loongarch-relax-emit-relocs.s
+++ b/lld/test/ELF/loongarch-relax-emit-relocs.s
@@ -12,10 +12,8 @@
 # RUN: ld.lld -r %t.64.o -o %t.64.r
 # RUN: llvm-objdump -dr %t.64.r | FileCheck %s --check-prefix=CHECKR
 
-## --no-relax should keep original relocations.
-## TODO Due to R_LARCH_RELAX is not relaxed, it plays same as --relax now.
 # RUN: ld.lld -Ttext=0x10000 --emit-relocs --no-relax %t.64.o -o %t.64.norelax
-# RUN: llvm-objdump -dr %t.64.norelax | FileCheck %s
+# RUN: llvm-objdump -dr %t.64.norelax | FileCheck %s --check-prefixes=CHECK,NORELAX
 
 # CHECK:      00010000 <_start>:
 # CHECK-NEXT:   pcalau12i $a0, 0
@@ -27,6 +25,7 @@
 # CHECK-NEXT:   nop
 # CHECK-NEXT:     R_LARCH_ALIGN *ABS*+0xc
 # CHECK-NEXT:   nop
+# NORELAX-NEXT:   nop
 # CHECK-NEXT:   ret
 
 # CHECKR:      <_start>:

llvmbot · 2024-10-08T06:19:55Z

@llvm/pr-subscribers-clang-driver

Author: None (ywgrit)

Changes

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

4 Files Affected:

(modified) clang/lib/Driver/ToolChains/Arch/LoongArch.cpp (+7)
(modified) lld/ELF/Writer.cpp (+4-1)
(modified) lld/test/ELF/loongarch-relax-align.s (+169-40)
(modified) lld/test/ELF/loongarch-relax-emit-relocs.s (+2-3)

diff --git a/clang/lib/Driver/ToolChains/Arch/LoongArch.cpp b/clang/lib/Driver/ToolChains/Arch/LoongArch.cpp
index 771adade93813f..74d3ab65ed6643 100644
--- a/clang/lib/Driver/ToolChains/Arch/LoongArch.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/LoongArch.cpp
@@ -134,6 +134,13 @@ void loongarch::getLoongArchTargetFeatures(const Driver &D,
       (!Args.hasArgNoClaim(clang::driver::options::OPT_march_EQ)))
     Features.push_back("+lsx");
 
+  // -mrelax is default, unless -mno-relax is specified.
+  if (Args.hasFlag(options::OPT_mrelax, options::OPT_mno_relax, true)) {
+    Features.push_back("+relax");
+  } else {
+    Features.push_back("-relax");
+  }
+
   std::string ArchName;
   if (const Arg *A = Args.getLastArg(options::OPT_march_EQ))
     ArchName = A->getValue();
diff --git a/lld/ELF/Writer.cpp b/lld/ELF/Writer.cpp
index f4a22ea953ec49..cf0ff432f920ad 100644
--- a/lld/ELF/Writer.cpp
+++ b/lld/ELF/Writer.cpp
@@ -1462,6 +1462,8 @@ template <class ELFT> void Writer<ELFT>::finalizeAddressDependentContent() {
   for (;;) {
     bool changed = ctx.target->needsThunks
                        ? tc.createThunks(pass, ctx.outputSections)
+                   : ctx.arg.emachine == EM_LOONGARCH && !ctx.arg.relax
+                       ? false
                        : ctx.target->relaxOnce(pass);
     bool spilled = ctx.script->spillSections();
     changed |= spilled;
@@ -1545,7 +1547,8 @@ template <class ELFT> void Writer<ELFT>::finalizeAddressDependentContent() {
       finalizeOrderDependentContent();
     }
   }
-  if (!ctx.arg.relocatable)
+  if (!ctx.arg.relocatable &&
+      !(ctx.arg.emachine == EM_LOONGARCH && !ctx.arg.relax))
     ctx.target->finalizeRelax(pass);
 
   if (ctx.arg.relocatable)
diff --git a/lld/test/ELF/loongarch-relax-align.s b/lld/test/ELF/loongarch-relax-align.s
index ab61e15d5caca2..158f1f80ba8489 100644
--- a/lld/test/ELF/loongarch-relax-align.s
+++ b/lld/test/ELF/loongarch-relax-align.s
@@ -6,56 +6,180 @@
 # RUN: ld.lld --section-start=.text=0x10000 --section-start=.text2=0x20000 -e 0 %t.64.o -o %t.64
 # RUN: ld.lld --section-start=.text=0x10000 --section-start=.text2=0x20000 -e 0 %t.32.o --no-relax -o %t.32n
 # RUN: ld.lld --section-start=.text=0x10000 --section-start=.text2=0x20000 -e 0 %t.64.o --no-relax -o %t.64n
-# RUN: llvm-objdump -td --no-show-raw-insn %t.32 | FileCheck %s
-# RUN: llvm-objdump -td --no-show-raw-insn %t.64 | FileCheck %s
-# RUN: llvm-objdump -td --no-show-raw-insn %t.32n | FileCheck %s
-# RUN: llvm-objdump -td --no-show-raw-insn %t.64n | FileCheck %s
+# RUN: llvm-objdump -td --no-show-raw-insn %t.32 | FileCheck %s --check-prefix=RELAX32
+# RUN: llvm-objdump -td --no-show-raw-insn %t.64 | FileCheck %s --check-prefixes=RELAX64,SRELAX64
+# RUN: llvm-objdump -td --no-show-raw-insn %t.32n | FileCheck %s --check-prefix=NORELAX
+# RUN: llvm-objdump -td --no-show-raw-insn %t.64n | FileCheck %s --check-prefix=NORELAX
 
 ## Test the R_LARCH_ALIGN without symbol index.
 # RUN: llvm-mc --filetype=obj --triple=loongarch64 --mattr=+relax %s -o %t.o64.o --defsym=old=1
 # RUN: ld.lld --section-start=.text=0x10000 --section-start=.text2=0x20000 -e 0 %t.o64.o -o %t.o64
 # RUN: ld.lld --section-start=.text=0x10000 --section-start=.text2=0x20000 -e 0 %t.o64.o --no-relax -o %t.o64n
-# RUN: llvm-objdump -td --no-show-raw-insn %t.o64 | FileCheck %s
-# RUN: llvm-objdump -td --no-show-raw-insn %t.o64n | FileCheck %s
+# RUN: llvm-objdump -td --no-show-raw-insn %t.o64 | FileCheck %s --check-prefixes=RELAX64,ORELAX64
+# RUN: llvm-objdump -td --no-show-raw-insn %t.o64n | FileCheck %s --check-prefix=ONORELAX
 
 ## -r keeps section contents unchanged.
 # RUN: ld.lld -r %t.64.o -o %t.64.r
 # RUN: llvm-objdump -dr --no-show-raw-insn %t.64.r | FileCheck %s --check-prefix=CHECKR
 
-# CHECK-DAG: {{0*}}10000 l .text  {{0*}}44 .Ltext_start
-# CHECK-DAG: {{0*}}10038 l .text  {{0*}}0c .L1
-# CHECK-DAG: {{0*}}10040 l .text  {{0*}}04 .L2
-# CHECK-DAG: {{0*}}20000 l .text2 {{0*}}14 .Ltext2_start
-
-# CHECK:      <.Ltext_start>:
-# CHECK-NEXT:   break 1
-# CHECK-NEXT:   break 2
-# CHECK-NEXT:   nop
-# CHECK-NEXT:   nop
-# CHECK-NEXT:   break 3
-# CHECK-NEXT:   break 4
-# CHECK-NEXT:   nop
-# CHECK-NEXT:   nop
-# CHECK-NEXT:   pcalau12i     $a0, 0
-# CHECK-NEXT:   addi.{{[dw]}} $a0, $a0, 0
-# CHECK-NEXT:   pcalau12i     $a0, 0
-# CHECK-NEXT:   addi.{{[dw]}} $a0, $a0, 56
-# CHECK-NEXT:   pcalau12i     $a0, 0
-# CHECK-NEXT:   addi.{{[dw]}} $a0, $a0, 64
-# CHECK-EMPTY:
-# CHECK-NEXT: <.L1>:
-# CHECK-NEXT:   nop
-# CHECK-NEXT:   nop
-# CHECK-EMPTY:
-# CHECK-NEXT: <.L2>:
-# CHECK-NEXT:   break 5
-
-# CHECK:      <.Ltext2_start>:
-# CHECK-NEXT:   pcalau12i     $a0, 0
-# CHECK-NEXT:   addi.{{[dw]}} $a0, $a0, 0
-# CHECK-NEXT:   nop
-# CHECK-NEXT:   nop
-# CHECK-NEXT:   break 6
+
+# RELAX32-DAG: {{0*}}10000 l .text  {{0*}}00 .Lalign_symbol
+# RELAX32-DAG: {{0*}}10000 l .text  {{0*}}44 .Ltext_start
+# RELAX32-DAG: {{0*}}10038 l .text  {{0*}}0c .L1
+# RELAX32-DAG: {{0*}}10040 l .text  {{0*}}04 .L2
+# RELAX32-DAG: {{0*}}20000 l .text2 {{0*}}14 .Ltext2_start
+
+# RELAX32:      <.Ltext_start>:
+# RELAX32-NEXT:   break 1
+# RELAX32-NEXT:   break 2
+# RELAX32-NEXT:   nop
+# RELAX32-NEXT:   nop
+# RELAX32-NEXT:   break 3
+# RELAX32-NEXT:   break 4
+# RELAX32-NEXT:   nop
+# RELAX32-NEXT:   nop
+# RELAX32-NEXT:   pcalau12i     $a0, 0
+# RELAX32-NEXT:   addi.{{[dw]}} $a0, $a0, 0
+# RELAX32-NEXT:   pcalau12i     $a0, 0
+# RELAX32-NEXT:   addi.{{[dw]}} $a0, $a0, 56
+# RELAX32-NEXT:   pcalau12i     $a0, 0
+# RELAX32-NEXT:   addi.{{[dw]}} $a0, $a0, 64
+# RELAX32-EMPTY:
+# RELAX32-NEXT: <.L1>:
+# RELAX32-NEXT:   nop
+# RELAX32-NEXT:   nop
+# RELAX32-EMPTY:
+# RELAX32-NEXT: <.L2>:
+# RELAX32-NEXT:   break 5
+
+# RELAX32:      <.Ltext2_start>:
+# RELAX32-NEXT:   pcalau12i     $a0, 0
+# RELAX32-NEXT:   addi.{{[dw]}} $a0, $a0, 0
+# RELAX32-NEXT:   nop
+# RELAX32-NEXT:   nop
+# RELAX32-NEXT:   break 6
+
+# NORELAX-DAG: {{0*}}10000 l .text  {{0*}}00 .Lalign_symbol
+# NORELAX-DAG: {{0*}}10000 l .text  {{0*}}5c .Ltext_start
+# NORELAX-DAG: {{0*}}1004c l .text  {{0*}}10 .L1
+# NORELAX-DAG: {{0*}}10058 l .text  {{0*}}04 .L2
+# NORELAX-DAG: {{0*}}20000 l .text2 {{0*}}18 .Ltext2_start
+
+# NORELAX:      <.Ltext_start>:
+# NORELAX-NEXT:   break 1
+# NORELAX-NEXT:   break 2
+# NORELAX-NEXT:   nop
+# NORELAX-NEXT:   nop
+# NORELAX-NEXT:   nop
+# NORELAX-NEXT:   break 3
+# NORELAX-NEXT:   nop
+# NORELAX-NEXT:   nop
+# NORELAX-NEXT:   nop
+# NORELAX-NEXT:   break 4
+# NORELAX-NEXT:   nop
+# NORELAX-NEXT:   nop
+# NORELAX-NEXT:   nop
+# NORELAX-NEXT:   pcalau12i     $a0, 0
+# NORELAX-NEXT:   addi.{{[dw]}} $a0, $a0, 0
+# NORELAX-NEXT:   pcalau12i     $a0, 0
+# NORELAX-NEXT:   addi.{{[dw]}} $a0, $a0, 76
+# NORELAX-NEXT:   pcalau12i     $a0, 0
+# NORELAX-NEXT:   addi.{{[dw]}} $a0, $a0, 88
+# NORELAX-EMPTY:
+# NORELAX-NEXT: <.L1>:
+# NORELAX-NEXT:   nop
+# NORELAX-NEXT:   nop
+# NORELAX-NEXT:   nop
+# NORELAX-EMPTY:
+# NORELAX-NEXT: <.L2>:
+# NORELAX-NEXT:   break 5
+
+# NORELAX:      <.Ltext2_start>:
+# NORELAX-NEXT:   pcalau12i     $a0, 0
+# NORELAX-NEXT:   addi.{{[dw]}} $a0, $a0, 0
+# NORELAX-NEXT:   nop
+# NORELAX-NEXT:   nop
+# NORELAX-NEXT:   nop
+# NORELAX-NEXT:   break 6
+
+
+
+# ORELAX64-DAG: {{0*}}00001 l *ABS*  {{0*}}00 old
+# SRELAX64-DAG: {{0*}}10000 l .text  {{0*}}00 .Lalign_symbol
+# RELAX64-DAG: {{0*}}10000 l .text  {{0*}}44 .Ltext_start
+# RELAX64-DAG: {{0*}}10038 l .text  {{0*}}0c .L1
+# RELAX64-DAG: {{0*}}10040 l .text  {{0*}}04 .L2
+# RELAX64-DAG: {{0*}}20000 l .text2 {{0*}}14 .Ltext2_start
+
+# RELAX64:      <.Ltext_start>:
+# RELAX64-NEXT:   break 1
+# RELAX64-NEXT:   break 2
+# RELAX64-NEXT:   nop
+# RELAX64-NEXT:   nop
+# RELAX64-NEXT:   break 3
+# RELAX64-NEXT:   break 4
+# RELAX64-NEXT:   nop
+# RELAX64-NEXT:   nop
+# RELAX64-NEXT:   pcalau12i  $a0, 0
+# RELAX64-NEXT:   addi.d  $a0, $a0, 0
+# RELAX64-NEXT:   pcalau12i  $a0, 0
+# RELAX64-NEXT:   addi.d  $a0, $a0, 56
+# RELAX64-NEXT:   pcalau12i  $a0, 0
+# RELAX64-NEXT:   addi.d  $a0, $a0, 64
+# RELAX64-EMPTY:
+# RELAX64-NEXT: <.L1>:
+# RELAX64-NEXT:   nop
+# RELAX64-NEXT:   nop
+# RELAX64-EMPTY:
+# RELAX64-NEXT: <.L2>:
+# RELAX64-NEXT:   break 5
+
+# RELAX64:      <.Ltext2_start>:
+# RELAX64-NEXT:   pcalau12i  $a0, 0
+# RELAX64-NEXT:   addi.d  $a0, $a0, 0
+# RELAX64-NEXT:   nop
+# RELAX64-NEXT:   nop
+# RELAX64-NEXT:   break 6
+
+
+# ONORELAX-DAG: {{0*}}00001 l *ABS*  {{0*}}00 old
+# ONORELAX-DAG: {{0*}}10000 l .text  {{0*}}4c .Ltext_start
+# ONORELAX-DAG: {{0*}}1003c l .text  {{0*}}10 .L1
+# ONORELAX-DAG: {{0*}}10048 l .text  {{0*}}04 .L2
+# ONORELAX-DAG: {{0*}}20000 l .text2 {{0*}}18 .Ltext2_start
+
+# ONORELAX:      <.Ltext_start>:
+# ONORELAX-NEXT:   break 1
+# ONORELAX-NEXT:   break 2
+# ONORELAX-NEXT:   nop
+# ONORELAX-NEXT:   nop
+# ONORELAX-NEXT:   nop
+# ONORELAX-NEXT:   break 3
+# ONORELAX-NEXT:   break 4
+# ONORELAX-NEXT:   nop
+# ONORELAX-NEXT:   nop
+# ONORELAX-NEXT:   pcalau12i     $a0, 0
+# ONORELAX-NEXT:   addi.{{[dw]}} $a0, $a0, 0
+# ONORELAX-NEXT:   pcalau12i     $a0, 0
+# ONORELAX-NEXT:   addi.{{[dw]}} $a0, $a0, 60
+# ONORELAX-NEXT:   pcalau12i     $a0, 0
+# ONORELAX-NEXT:   addi.{{[dw]}} $a0, $a0, 72
+# ONORELAX-EMPTY:
+# ONORELAX-NEXT: <.L1>:
+# ONORELAX-NEXT:   nop
+# ONORELAX-NEXT:   nop
+# ONORELAX-NEXT:   nop
+# ONORELAX-EMPTY:
+# ONORELAX-NEXT: <.L2>:
+# ONORELAX-NEXT:   break 5
+
+# ONORELAX:      <.Ltext2_start>:
+# ONORELAX-NEXT:   pcalau12i     $a0, 0
+# ONORELAX-NEXT:   addi.{{[dw]}} $a0, $a0, 0
+# ONORELAX-NEXT:   nop
+# ONORELAX-NEXT:   nop
+# ONORELAX-NEXT:   nop
+# ONORELAX-NEXT:   break 6
 
 # CHECKR:      <.Ltext2_start>:
 # CHECKR-NEXT:   pcalau12i $a0, 0
@@ -70,6 +194,11 @@
 # CHECKR-NEXT:   nop
 # CHECKR-NEXT:   break 6
 
+
+
+
+
+
 .macro .fake_p2align_4 max=0
   .ifdef old
     .if \max==0
diff --git a/lld/test/ELF/loongarch-relax-emit-relocs.s b/lld/test/ELF/loongarch-relax-emit-relocs.s
index ba414e8c93f0fb..1feec64d722cab 100644
--- a/lld/test/ELF/loongarch-relax-emit-relocs.s
+++ b/lld/test/ELF/loongarch-relax-emit-relocs.s
@@ -12,10 +12,8 @@
 # RUN: ld.lld -r %t.64.o -o %t.64.r
 # RUN: llvm-objdump -dr %t.64.r | FileCheck %s --check-prefix=CHECKR
 
-## --no-relax should keep original relocations.
-## TODO Due to R_LARCH_RELAX is not relaxed, it plays same as --relax now.
 # RUN: ld.lld -Ttext=0x10000 --emit-relocs --no-relax %t.64.o -o %t.64.norelax
-# RUN: llvm-objdump -dr %t.64.norelax | FileCheck %s
+# RUN: llvm-objdump -dr %t.64.norelax | FileCheck %s --check-prefixes=CHECK,NORELAX
 
 # CHECK:      00010000 <_start>:
 # CHECK-NEXT:   pcalau12i $a0, 0
@@ -27,6 +25,7 @@
 # CHECK-NEXT:   nop
 # CHECK-NEXT:     R_LARCH_ALIGN *ABS*+0xc
 # CHECK-NEXT:   nop
+# NORELAX-NEXT:   nop
 # CHECK-NEXT:   ret
 
 # CHECKR:      <_start>:

ywgrit · 2024-10-08T06:21:15Z

The relaxation-related code except alignment will be added later, which has been tested locally.

MaskRay

We split clang and lld changes.

MaskRay · 2024-10-09T03:39:37Z

clang/lib/Driver/ToolChains/Arch/LoongArch.cpp

@@ -134,6 +134,13 @@ void loongarch::getLoongArchTargetFeatures(const Driver &D,
      (!Args.hasArgNoClaim(clang::driver::options::OPT_march_EQ)))
    Features.push_back("+lsx");

+  // -mrelax is default, unless -mno-relax is specified.


This code self explains and the comment is not necessary.

MaskRay · 2024-10-09T03:40:22Z

lld/ELF/Writer.cpp

@@ -1462,6 +1462,8 @@ template <class ELFT> void Writer<ELFT>::finalizeAddressDependentContent() {
  for (;;) {
    bool changed = ctx.target->needsThunks
                       ? tc.createThunks(pass, ctx.outputSections)
+                   : ctx.arg.emachine == EM_LOONGARCH && !ctx.arg.relax


Adding ctx.arg.relax to every place is not acceptable. You could change ctx.arg.relax to be always false for loongarch, though.

Adding ctx.arg.relax to every place is not acceptable. You could change ctx.arg.relax to be always false for loongarch, though.

ctx.arg.relax is used here because we want to determine whether or not to apply relaxation based on the parameters passed at the time of linking.

ctx.arg.relax is used here because we want to determine whether or not to apply relaxation based on the parameters passed at the time of linking.

I don't think so. R_LARCH_ALIGN should be handled unconditionally, otherwise the code may be unaligned.

ctx.arg.relax is used here because we want to determine whether or not to apply relaxation based on the parameters passed at the time of linking.

I don't think so. R_LARCH_ALIGN should be handled unconditionally, otherwise the code may be unaligned.

Yes, it was an oversight. So for scenarios outside of align, is it necessary to determine whether or not to apply relaxation via the incoming link parameter, i.e. --relax? For example, in the relax function

Yes, it was an oversight. So for scenarios outside of align, is it necessary to determine whether or not to apply relaxation via the incoming link parameter, i.e. --relax? For example, in the relax function

I'm afraid it's not necessary because R_LARCH_RELAX is converted to R_NONE by function getRelExpr when --no-relax.

case R_LARCH_RELAX: return ctx.arg.relax ? R_RELAX_HINT : R_NONE; case R_LARCH_ALIGN: return R_RELAX_HINT

Is it correct? @MQ-mengqing

Yes, it was an oversight. So for scenarios outside of align, is it necessary to determine whether or not to apply relaxation via the incoming link parameter, i.e. --relax? For example, in the relax function

I'm afraid it's not necessary because R_LARCH_RELAX is converted to R_NONE by function getRelExpr when --no-relax.

case R_LARCH_RELAX: return ctx.arg.relax ? R_RELAX_HINT : R_NONE; case R_LARCH_ALIGN: return R_RELAX_HINT

Is it correct? @MQ-mengqing

Thanks for the reminder, I tested both on RISCV and LoongArch, the relocation which corresponding RelExpr is R_NONE will be ignored. The changes of test files will be reverted.

…ept for the getRelExpr function

SixWeining · 2024-10-14T09:26:27Z

The title should also be updated.

ywgrit · 2024-10-14T09:33:49Z

The title should also be updated.

How about now?

SixWeining · 2024-10-14T12:09:55Z

How about now?

The patch doesn't touch lld, so my suggestion is "[Driver] Default enable LoongArch linker relaxation".

Some additional comments:

Do we need to consider the -gsplit-dwarf issue MC: "error: A dwo section may not contain relocations" when building with fission + RISCV64 #56642 resolved by https://reviews.llvm.org/D130190 ?
The --no-relax option should be passed to linker, see [Clang][RISCV] Forward --no-relax option to linker for RISC-V #76432
Pass -X to linker to discard a plethora of .L symbols due to linker relaxation. See https://reviews.llvm.org/D127826

ywgrit · 2024-10-15T08:31:17Z

How about now?

The patch doesn't touch lld, so my suggestion is "[Driver] Default enable LoongArch linker relaxation".

Thanks, the title has changed.

Some additional comments:

Do we need to consider the -gsplit-dwarf issue MC: "error: A dwo section may not contain relocations" when building with fission + RISCV64 #56642 resolved by https://reviews.llvm.org/D130190 ?

The code has been implemented and tested, just like riscv, I think this part of the code can be put together with this change.

The --no-relax option should be passed to linker, see [Clang][RISCV] Forward --no-relax option to linker for RISC-V #76432

The code remains consistent with RISCV.

Pass -X to linker to discard a plethora of .L symbols due to linker relaxation. See https://reviews.llvm.org/D127826

discard_sec_merge is used by default in GNU ld, and in LoongArch's definition, it converts the use of discard_sec_merge to the use of discard_l, as shown in the code below. So our changes are consistent with RISCV.

  if (link_info.discard == discard_sec_merge)
    link_info.discard = discard_l;

Should we put together the two part of code of pass --no-relax and -X with this change? These three parts of code (including this change) don't seem to have much to do with each other.

SixWeining · 2024-10-15T08:48:02Z

I think that's OK.

ywgrit · 2024-10-15T09:07:04Z

I think that's OK.

Ok, the code will be pushed soon.

MQ-mengqing · 2024-10-16T01:20:32Z

clang/lib/Driver/ToolChains/Arch/LoongArch.cpp

+  } else {
+    Features.push_back("-relax");
+  }
+


We can omit the braces.
https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements

Thanks, it reminds me to check out llvm's codestandard.

MaskRay · 2024-10-16T02:06:36Z

Personally I hope that -mrelax stays

How about now?

The patch doesn't touch lld, so my suggestion is "[Driver] Default enable LoongArch linker relaxation".

Some additional comments:

Do we need to consider the -gsplit-dwarf issue MC: "error: A dwo section may not contain relocations" when building with fission + RISCV64 #56642 resolved by https://reviews.llvm.org/D130190 ?

The --no-relax option should be passed to linker, see [Clang][RISCV] Forward --no-relax option to linker for RISC-V #76432

Pass -X to linker to discard a plethora of .L symbols due to linker relaxation. See https://reviews.llvm.org/D127826

This is a good summary and there are many reasons that linker relaxation could cause trouble.
Therefore, I don't think "matching RISC-V" is a sufficient justification to enable relaxation by default on LoongArch. We need more and without that we should hold off on this patch.

ywgrit · 2024-10-17T03:34:13Z

Personally I hope that -mrelax stays

How about now?

The patch doesn't touch lld, so my suggestion is "[Driver] Default enable LoongArch linker relaxation".
Some additional comments:

Do we need to consider the -gsplit-dwarf issue MC: "error: A dwo section may not contain relocations" when building with fission + RISCV64 #56642 resolved by https://reviews.llvm.org/D130190 ?

The --no-relax option should be passed to linker, see [Clang][RISCV] Forward --no-relax option to linker for RISC-V #76432

Pass -X to linker to discard a plethora of .L symbols due to linker relaxation. See https://reviews.llvm.org/D127826

This is a good summary and there are many reasons that linker relaxation could cause trouble. Therefore, I don't think "matching RISC-V" is a sufficient justification to enable relaxation by default on LoongArch. We need more and without that we should hold off on this patch.

I just read your post about linking relaxation concerns(https://maskray.me/blog/2021-03-14-the-dark-side-of-riscv-linker-relaxation). There are indeed a lot of issues at the moment and I have the following two questions.

Do you have any plans for these issues please?
How about adding the linking relaxation feature to LoongArch without turning it on by default(i.e., hold off on this patch)?

[lld][LoongArch] Enable relaxation when --relax option is passed

32aae75

llvmbot added clang Clang issues not falling into any other category lld clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' lld:ELF backend:loongarch labels Oct 8, 2024

[clang][LoongArch] Fix test after enable relaxation option

8c68051

SixWeining requested review from heiher and wangleiat October 9, 2024 02:00

MaskRay reviewed Oct 9, 2024

View reviewed changes

[lld][LoongArch] Revert test files as ctx.arg.relax is not needed exc…

5c4a042

…ept for the getRelExpr function

ywgrit changed the title ~~[lld][LoongArch] Enable relaxation when --relax option is passed~~ [lld][LoongArch] Add relaxation option Oct 14, 2024

ywgrit changed the title ~~[lld][LoongArch] Add relaxation option~~ [lld][LoongArch] Add the relaxation option Oct 14, 2024

ywgrit changed the title ~~[lld][LoongArch] Add the relaxation option~~ [Driver] Default enable LoongArch linker relaxation Oct 15, 2024

MQ-mengqing reviewed Oct 16, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Driver] Default enable LoongArch linker relaxation #111488

[Driver] Default enable LoongArch linker relaxation #111488

ywgrit commented Oct 8, 2024

github-actions bot commented Oct 8, 2024

ywgrit commented Oct 8, 2024 •

edited

Loading

llvmbot commented Oct 8, 2024 •

edited

Loading

llvmbot commented Oct 8, 2024

ywgrit commented Oct 8, 2024 •

edited

Loading

MaskRay left a comment

MaskRay Oct 9, 2024

MaskRay Oct 9, 2024

ywgrit Oct 9, 2024

SixWeining Oct 14, 2024

ywgrit Oct 14, 2024

SixWeining Oct 14, 2024

ywgrit Oct 14, 2024 •

edited

Loading

SixWeining commented Oct 14, 2024

ywgrit commented Oct 14, 2024

SixWeining commented Oct 14, 2024

ywgrit commented Oct 15, 2024 •

edited

Loading

SixWeining commented Oct 15, 2024

ywgrit commented Oct 15, 2024

MQ-mengqing Oct 16, 2024

ywgrit Oct 16, 2024

MaskRay commented Oct 16, 2024

ywgrit commented Oct 17, 2024 •

edited

Loading

[Driver] Default enable LoongArch linker relaxation #111488

Are you sure you want to change the base?

[Driver] Default enable LoongArch linker relaxation #111488

Conversation

ywgrit commented Oct 8, 2024

github-actions bot commented Oct 8, 2024

ywgrit commented Oct 8, 2024 • edited Loading

llvmbot commented Oct 8, 2024 • edited Loading

llvmbot commented Oct 8, 2024

ywgrit commented Oct 8, 2024 • edited Loading

MaskRay left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ywgrit Oct 14, 2024 • edited Loading

Choose a reason for hiding this comment

SixWeining commented Oct 14, 2024

ywgrit commented Oct 14, 2024

SixWeining commented Oct 14, 2024

ywgrit commented Oct 15, 2024 • edited Loading

SixWeining commented Oct 15, 2024

ywgrit commented Oct 15, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MaskRay commented Oct 16, 2024

ywgrit commented Oct 17, 2024 • edited Loading

ywgrit commented Oct 8, 2024 •

edited

Loading

llvmbot commented Oct 8, 2024 •

edited

Loading

ywgrit commented Oct 8, 2024 •

edited

Loading

ywgrit Oct 14, 2024 •

edited

Loading

ywgrit commented Oct 15, 2024 •

edited

Loading

ywgrit commented Oct 17, 2024 •

edited

Loading