[clang] [llvm] [RISCV] Always emit relocations for resolved symbols and relax (PR #73793)

Andreu Carminati via cfe-commits cfe-commits at lists.llvm.org
Thu Dec 7 08:30:23 PST 2023


https://github.com/andcarminati updated https://github.com/llvm/llvm-project/pull/73793

>From a7ba3e4e7a84c49e80fe3e05c1a8ca83e7fd8c6e Mon Sep 17 00:00:00 2001
From: Andreu Carminati <andreu.carminati at hightec-rt.com>
Date: Tue, 28 Nov 2023 15:26:49 +0100
Subject: [PATCH 1/2] [RISCV][MC] Always emit relocations for resolved symbols
 and relax

If relaxation is not itended, it can be disabled in the linker. Also,
we cannot trust Subtarget features here, because it may be empty in case
of LTO codegen, preventing relaxations.

Also forward --no-relax option to linker.
---
 clang/lib/Driver/ToolChains/BareMetal.cpp     |  3 ++
 .../lib/Driver/ToolChains/RISCVToolchain.cpp  |  3 ++
 clang/test/Driver/baremetal.cpp               | 10 ++++++
 .../RISCV/MCTargetDesc/RISCVAsmBackend.cpp    | 12 +++----
 llvm/test/CodeGen/RISCV/compress.ll           | 31 +++++++++++++------
 5 files changed, 41 insertions(+), 18 deletions(-)

diff --git a/clang/lib/Driver/ToolChains/BareMetal.cpp b/clang/lib/Driver/ToolChains/BareMetal.cpp
index 42c8336e626c7..fc955d79780e5 100644
--- a/clang/lib/Driver/ToolChains/BareMetal.cpp
+++ b/clang/lib/Driver/ToolChains/BareMetal.cpp
@@ -443,6 +443,9 @@ void baremetal::Linker::ConstructJob(Compilation &C, const JobAction &JA,
 
   CmdArgs.push_back("-Bstatic");
 
+  if (Args.hasArg(options::OPT_mno_relax))
+    CmdArgs.push_back("--no-relax");
+
   if (Triple.isARM() || Triple.isThumb()) {
     bool IsBigEndian = arm::isARMBigEndian(Triple, Args);
     if (IsBigEndian)
diff --git a/clang/lib/Driver/ToolChains/RISCVToolchain.cpp b/clang/lib/Driver/ToolChains/RISCVToolchain.cpp
index 7e6abd1444287..0be7d1a889949 100644
--- a/clang/lib/Driver/ToolChains/RISCVToolchain.cpp
+++ b/clang/lib/Driver/ToolChains/RISCVToolchain.cpp
@@ -156,6 +156,9 @@ void RISCV::Linker::ConstructJob(Compilation &C, const JobAction &JA,
   if (!D.SysRoot.empty())
     CmdArgs.push_back(Args.MakeArgString("--sysroot=" + D.SysRoot));
 
+  if (Args.hasArg(options::OPT_mno_relax))
+    CmdArgs.push_back("--no-relax");
+
   bool IsRV64 = ToolChain.getArch() == llvm::Triple::riscv64;
   CmdArgs.push_back("-m");
   if (IsRV64) {
diff --git a/clang/test/Driver/baremetal.cpp b/clang/test/Driver/baremetal.cpp
index c04f4506a0994..134bf427e3dc1 100644
--- a/clang/test/Driver/baremetal.cpp
+++ b/clang/test/Driver/baremetal.cpp
@@ -460,3 +460,13 @@
 // RUN:   | FileCheck --check-prefix=CHECK-CLANGRT-ARCH %s
 // CHECK-CLANGRT-ARCH: "-lclang_rt.builtins-armv6m"
 // CHECK-CLANGRT-ARCH-NOT: "-lclang_rt.builtins"
+
+// RUN: %clang %s -### 2>&1 --target=riscv64-unknown-elf -nostdinc -mno-relax \
+// RUN:     --sysroot=%S/Inputs/basic_riscv64_tree/riscv64-unknown-elf \
+// RUN:   | FileCheck --check-prefix=CHECK-RV64-NORELAX %s
+// CHECK-RV64-NORELAX: "--no-relax"
+
+// RUN: %clang %s -### 2>&1 --target=riscv64-unknown-elf -nostdinc \
+// RUN:     --sysroot=%S/Inputs/basic_riscv64_tree/riscv64-unknown-elf \
+// RUN:   | FileCheck --check-prefix=CHECK-RV64-RELAX %s
+// CHECK-RV64-RELAX-NOT: "--no-relax"
\ No newline at end of file
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
index dfc3c9e9908d8..d4efaaf2666e4 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
@@ -103,9 +103,9 @@ RISCVAsmBackend::getFixupKindInfo(MCFixupKind Kind) const {
   return Infos[Kind - FirstTargetFixupKind];
 }
 
-// If linker relaxation is enabled, or the relax option had previously been
-// enabled, always emit relocations even if the fixup can be resolved. This is
-// necessary for correctness as offsets may change during relaxation.
+// Always emit relocations for relative addresses, even if the fixup can be
+// resolved. This is necessary for correctness as offsets may change during
+// relaxation.
 bool RISCVAsmBackend::shouldForceRelocation(const MCAssembler &Asm,
                                             const MCFixup &Fixup,
                                             const MCValue &Target) {
@@ -122,13 +122,9 @@ bool RISCVAsmBackend::shouldForceRelocation(const MCAssembler &Asm,
     if (Target.isAbsolute())
       return false;
     break;
-  case RISCV::fixup_riscv_got_hi20:
-  case RISCV::fixup_riscv_tls_got_hi20:
-  case RISCV::fixup_riscv_tls_gd_hi20:
-    return true;
   }
 
-  return STI.hasFeature(RISCV::FeatureRelax) || ForceRelocs;
+  return true;
 }
 
 bool RISCVAsmBackend::fixupNeedsRelaxationAdvanced(const MCFixup &Fixup,
diff --git a/llvm/test/CodeGen/RISCV/compress.ll b/llvm/test/CodeGen/RISCV/compress.ll
index 479b7e524cd34..fd7c4e9cc9934 100644
--- a/llvm/test/CodeGen/RISCV/compress.ll
+++ b/llvm/test/CodeGen/RISCV/compress.ll
@@ -1,3 +1,4 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 3
 ; This test is designed to run twice, once with function attributes and once
 ; with target attributes added on the command line.
 ;
@@ -50,35 +51,45 @@ define i32 @simple_arith(i32 %a, i32 %b) #0 {
 define i32 @select(i32 %a, ptr %b) #0 {
 ; RV32IC-LABEL: <select>:
 ; RV32IC:         c.lw a2, 0(a1)
-; RV32IC-NEXT:    c.beqz a2, 0x18
+; RV32IC-NEXT:    c.beqz a2, 0x14 <select+0x2>
 ; RV32IC-NEXT:    c.mv a0, a2
+; RV32IC-LABEL: <.LBB1_2>:
 ; RV32IC-NEXT:    c.lw a2, 0(a1)
-; RV32IC-NEXT:    c.bnez a2, 0x1e
+; RV32IC-NEXT:    c.bnez a2, 0x1a <.LBB1_2+0x2>
 ; RV32IC-NEXT:    c.mv a0, a2
+; RV32IC-LABEL: <.LBB1_4>:
 ; RV32IC-NEXT:    c.lw a2, 0(a1)
-; RV32IC-NEXT:    bltu a2, a0, 0x26
+; RV32IC-NEXT:    bltu a2, a0, 0x20 <.LBB1_4+0x2>
 ; RV32IC-NEXT:    c.mv a0, a2
+; RV32IC-LABEL: <.LBB1_6>:
 ; RV32IC-NEXT:    c.lw a2, 0(a1)
-; RV32IC-NEXT:    bgeu a0, a2, 0x2e
+; RV32IC-NEXT:    bgeu a0, a2, 0x28 <.LBB1_6+0x2>
 ; RV32IC-NEXT:    c.mv a0, a2
+; RV32IC-LABEL: <.LBB1_8>:
 ; RV32IC-NEXT:    c.lw a2, 0(a1)
-; RV32IC-NEXT:    bltu a0, a2, 0x36
+; RV32IC-NEXT:    bltu a0, a2, 0x30 <.LBB1_8+0x2>
 ; RV32IC-NEXT:    c.mv a0, a2
+; RV32IC-LABEL:  <.LBB1_10>:
 ; RV32IC-NEXT:    c.lw a2, 0(a1)
-; RV32IC-NEXT:    bgeu a2, a0, 0x3e
+; RV32IC-NEXT:    bgeu a2, a0, 0x38 <.LBB1_10+0x2>
 ; RV32IC-NEXT:    c.mv a0, a2
+; RV32IC-LABEL: <.LBB1_12>:
 ; RV32IC-NEXT:    c.lw a2, 0(a1)
-; RV32IC-NEXT:    blt a2, a0, 0x46
+; RV32IC-NEXT:    blt a2, a0, 0x40 <.LBB1_12+0x2>
 ; RV32IC-NEXT:    c.mv a0, a2
+; RV32IC-LABEL: <.LBB1_14>:
 ; RV32IC-NEXT:    c.lw a2, 0(a1)
-; RV32IC-NEXT:    bge a0, a2, 0x4e
+; RV32IC-NEXT:    bge a0, a2, 0x48 <.LBB1_14+0x2>
 ; RV32IC-NEXT:    c.mv a0, a2
+; RV32IC-LABEL: <.LBB1_16>:
 ; RV32IC-NEXT:    c.lw a2, 0(a1)
-; RV32IC-NEXT:    blt a0, a2, 0x56
+; RV32IC-NEXT:    blt a0, a2, 0x50 <.LBB1_16+0x2>
 ; RV32IC-NEXT:    c.mv a0, a2
+; RV32IC-LABEL: <.LBB1_18>:
 ; RV32IC-NEXT:    c.lw a1, 0(a1)
-; RV32IC-NEXT:    bge a1, a0, 0x5e
+; RV32IC-NEXT:    bge a1, a0, 0x58 <.LBB1_18+0x2>
 ; RV32IC-NEXT:    c.mv a0, a1
+; RV32IC-LABEL: <.LBB1_20>:
 ; RV32IC-NEXT:    c.jr ra
   %val1 = load volatile i32, ptr %b
   %tst1 = icmp eq i32 0, %val1

>From 82b02930e02c3006c23ba53d2883ac8bd47dd5a4 Mon Sep 17 00:00:00 2001
From: Andreu Carminati <andreu.carminati at hightec-rt.com>
Date: Thu, 7 Dec 2023 17:27:23 +0100
Subject: [PATCH 2/2] [Clang][Driver] Forward --no-relax option to linker

In the case of -mno-relax option. Otherwise, we cannot prevent relaxation
if we split compilation and linking.
---
 clang/lib/Driver/ToolChains/BareMetal.cpp     |  2 +-
 clang/test/Driver/baremetal.cpp               |  2 ++
 clang/test/Driver/riscv32-toolchain.c         | 16 ++++++++++
 clang/test/Driver/riscv64-toolchain.c         | 16 ++++++++++
 .../RISCV/MCTargetDesc/RISCVAsmBackend.cpp    | 12 ++++---
 llvm/test/CodeGen/RISCV/compress.ll           | 31 ++++++-------------
 6 files changed, 53 insertions(+), 26 deletions(-)

diff --git a/clang/lib/Driver/ToolChains/BareMetal.cpp b/clang/lib/Driver/ToolChains/BareMetal.cpp
index fc955d79780e5..007a24f2b81ae 100644
--- a/clang/lib/Driver/ToolChains/BareMetal.cpp
+++ b/clang/lib/Driver/ToolChains/BareMetal.cpp
@@ -443,7 +443,7 @@ void baremetal::Linker::ConstructJob(Compilation &C, const JobAction &JA,
 
   CmdArgs.push_back("-Bstatic");
 
-  if (Args.hasArg(options::OPT_mno_relax))
+  if (TC.getTriple().isRISCV() && Args.hasArg(options::OPT_mno_relax))
     CmdArgs.push_back("--no-relax");
 
   if (Triple.isARM() || Triple.isThumb()) {
diff --git a/clang/test/Driver/baremetal.cpp b/clang/test/Driver/baremetal.cpp
index 134bf427e3dc1..7511d7d1adb4d 100644
--- a/clang/test/Driver/baremetal.cpp
+++ b/clang/test/Driver/baremetal.cpp
@@ -461,11 +461,13 @@
 // CHECK-CLANGRT-ARCH: "-lclang_rt.builtins-armv6m"
 // CHECK-CLANGRT-ARCH-NOT: "-lclang_rt.builtins"
 
+// Check that "--no-relax" is forwarded to the linker for RISC-V.
 // RUN: %clang %s -### 2>&1 --target=riscv64-unknown-elf -nostdinc -mno-relax \
 // RUN:     --sysroot=%S/Inputs/basic_riscv64_tree/riscv64-unknown-elf \
 // RUN:   | FileCheck --check-prefix=CHECK-RV64-NORELAX %s
 // CHECK-RV64-NORELAX: "--no-relax"
 
+// Check that "--no-relax" is not forwarded to the linker for RISC-V.
 // RUN: %clang %s -### 2>&1 --target=riscv64-unknown-elf -nostdinc \
 // RUN:     --sysroot=%S/Inputs/basic_riscv64_tree/riscv64-unknown-elf \
 // RUN:   | FileCheck --check-prefix=CHECK-RV64-RELAX %s
diff --git a/clang/test/Driver/riscv32-toolchain.c b/clang/test/Driver/riscv32-toolchain.c
index 63690c946ca0b..701da05da9189 100644
--- a/clang/test/Driver/riscv32-toolchain.c
+++ b/clang/test/Driver/riscv32-toolchain.c
@@ -215,6 +215,22 @@
 
 // RUN: %clang --target=riscv32 %s -emit-llvm -S -o - | FileCheck %s
 
+// Check that "--no-relax" is forwarded to the linker for RISC-V (RISCVToolchain.cpp).
+// RUN: env "PATH=" %clang %s -### 2>&1 -mno-relax \
+// RUN:   --target=riscv32-unknown-elf --rtlib=platform --unwindlib=platform --sysroot= \
+// RUN:   -march=rv32imac -mabi=lp32\
+// RUN:   --gcc-toolchain=%S/Inputs/multilib_riscv_elf_sdk 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-RV32-NORELAX %s
+// CHECK-RV32-NORELAX: "--no-relax"
+
+// Check that "--no-relax" is not forwarded to the linker for RISC-V (RISCVToolchain.cpp).
+// RUN:env "PATH=" %clang %s -### 2>&1 \
+// RUN:   --target=riscv32-unknown-elf --rtlib=platform --unwindlib=platform --sysroot= \
+// RUN:   -march=rv32imac -mabi=lp32\
+// RUN:   --gcc-toolchain=%S/Inputs/multilib_riscv_elf_sdk 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-RV32-RELAX %s
+// CHECK-RV32-RELAX-NOT: "--no-relax"
+
 typedef __builtin_va_list va_list;
 typedef __SIZE_TYPE__ size_t;
 typedef __PTRDIFF_TYPE__ ptrdiff_t;
diff --git a/clang/test/Driver/riscv64-toolchain.c b/clang/test/Driver/riscv64-toolchain.c
index f177bff33dd4d..da07c57d8e1fc 100644
--- a/clang/test/Driver/riscv64-toolchain.c
+++ b/clang/test/Driver/riscv64-toolchain.c
@@ -171,6 +171,22 @@
 
 // RUN: %clang --target=riscv64 %s -emit-llvm -S -o - | FileCheck %s
 
+// Check that "--no-relax" is forwarded to the linker for RISC-V (RISCVToolchain.cpp).
+// RUN: env "PATH=" %clang %s -### 2>&1 -mno-relax \
+// RUN:   --target=riscv64-unknown-elf --rtlib=platform --unwindlib=platform --sysroot= \
+// RUN:   -march=rv64imac -mabi=lp64\
+// RUN:   --gcc-toolchain=%S/Inputs/multilib_riscv_elf_sdk 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-RV64-NORELAX %s
+// CHECK-RV64-NORELAX: "--no-relax"
+
+// Check that "--no-relax" is not forwarded to the linker for RISC-V (RISCVToolchain.cpp).
+// RUN:env "PATH=" %clang %s -### 2>&1 \
+// RUN:   --target=riscv64-unknown-elf --rtlib=platform --unwindlib=platform --sysroot= \
+// RUN:   -march=rv64imac -mabi=lp64\
+// RUN:   --gcc-toolchain=%S/Inputs/multilib_riscv_elf_sdk 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-RV64-RELAX %s
+// CHECK-RV64-RELAX-NOT: "--no-relax"
+
 typedef __builtin_va_list va_list;
 typedef __SIZE_TYPE__ size_t;
 typedef __PTRDIFF_TYPE__ ptrdiff_t;
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
index d4efaaf2666e4..dfc3c9e9908d8 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
@@ -103,9 +103,9 @@ RISCVAsmBackend::getFixupKindInfo(MCFixupKind Kind) const {
   return Infos[Kind - FirstTargetFixupKind];
 }
 
-// Always emit relocations for relative addresses, even if the fixup can be
-// resolved. This is necessary for correctness as offsets may change during
-// relaxation.
+// If linker relaxation is enabled, or the relax option had previously been
+// enabled, always emit relocations even if the fixup can be resolved. This is
+// necessary for correctness as offsets may change during relaxation.
 bool RISCVAsmBackend::shouldForceRelocation(const MCAssembler &Asm,
                                             const MCFixup &Fixup,
                                             const MCValue &Target) {
@@ -122,9 +122,13 @@ bool RISCVAsmBackend::shouldForceRelocation(const MCAssembler &Asm,
     if (Target.isAbsolute())
       return false;
     break;
+  case RISCV::fixup_riscv_got_hi20:
+  case RISCV::fixup_riscv_tls_got_hi20:
+  case RISCV::fixup_riscv_tls_gd_hi20:
+    return true;
   }
 
-  return true;
+  return STI.hasFeature(RISCV::FeatureRelax) || ForceRelocs;
 }
 
 bool RISCVAsmBackend::fixupNeedsRelaxationAdvanced(const MCFixup &Fixup,
diff --git a/llvm/test/CodeGen/RISCV/compress.ll b/llvm/test/CodeGen/RISCV/compress.ll
index fd7c4e9cc9934..479b7e524cd34 100644
--- a/llvm/test/CodeGen/RISCV/compress.ll
+++ b/llvm/test/CodeGen/RISCV/compress.ll
@@ -1,4 +1,3 @@
-; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 3
 ; This test is designed to run twice, once with function attributes and once
 ; with target attributes added on the command line.
 ;
@@ -51,45 +50,35 @@ define i32 @simple_arith(i32 %a, i32 %b) #0 {
 define i32 @select(i32 %a, ptr %b) #0 {
 ; RV32IC-LABEL: <select>:
 ; RV32IC:         c.lw a2, 0(a1)
-; RV32IC-NEXT:    c.beqz a2, 0x14 <select+0x2>
+; RV32IC-NEXT:    c.beqz a2, 0x18
 ; RV32IC-NEXT:    c.mv a0, a2
-; RV32IC-LABEL: <.LBB1_2>:
 ; RV32IC-NEXT:    c.lw a2, 0(a1)
-; RV32IC-NEXT:    c.bnez a2, 0x1a <.LBB1_2+0x2>
+; RV32IC-NEXT:    c.bnez a2, 0x1e
 ; RV32IC-NEXT:    c.mv a0, a2
-; RV32IC-LABEL: <.LBB1_4>:
 ; RV32IC-NEXT:    c.lw a2, 0(a1)
-; RV32IC-NEXT:    bltu a2, a0, 0x20 <.LBB1_4+0x2>
+; RV32IC-NEXT:    bltu a2, a0, 0x26
 ; RV32IC-NEXT:    c.mv a0, a2
-; RV32IC-LABEL: <.LBB1_6>:
 ; RV32IC-NEXT:    c.lw a2, 0(a1)
-; RV32IC-NEXT:    bgeu a0, a2, 0x28 <.LBB1_6+0x2>
+; RV32IC-NEXT:    bgeu a0, a2, 0x2e
 ; RV32IC-NEXT:    c.mv a0, a2
-; RV32IC-LABEL: <.LBB1_8>:
 ; RV32IC-NEXT:    c.lw a2, 0(a1)
-; RV32IC-NEXT:    bltu a0, a2, 0x30 <.LBB1_8+0x2>
+; RV32IC-NEXT:    bltu a0, a2, 0x36
 ; RV32IC-NEXT:    c.mv a0, a2
-; RV32IC-LABEL:  <.LBB1_10>:
 ; RV32IC-NEXT:    c.lw a2, 0(a1)
-; RV32IC-NEXT:    bgeu a2, a0, 0x38 <.LBB1_10+0x2>
+; RV32IC-NEXT:    bgeu a2, a0, 0x3e
 ; RV32IC-NEXT:    c.mv a0, a2
-; RV32IC-LABEL: <.LBB1_12>:
 ; RV32IC-NEXT:    c.lw a2, 0(a1)
-; RV32IC-NEXT:    blt a2, a0, 0x40 <.LBB1_12+0x2>
+; RV32IC-NEXT:    blt a2, a0, 0x46
 ; RV32IC-NEXT:    c.mv a0, a2
-; RV32IC-LABEL: <.LBB1_14>:
 ; RV32IC-NEXT:    c.lw a2, 0(a1)
-; RV32IC-NEXT:    bge a0, a2, 0x48 <.LBB1_14+0x2>
+; RV32IC-NEXT:    bge a0, a2, 0x4e
 ; RV32IC-NEXT:    c.mv a0, a2
-; RV32IC-LABEL: <.LBB1_16>:
 ; RV32IC-NEXT:    c.lw a2, 0(a1)
-; RV32IC-NEXT:    blt a0, a2, 0x50 <.LBB1_16+0x2>
+; RV32IC-NEXT:    blt a0, a2, 0x56
 ; RV32IC-NEXT:    c.mv a0, a2
-; RV32IC-LABEL: <.LBB1_18>:
 ; RV32IC-NEXT:    c.lw a1, 0(a1)
-; RV32IC-NEXT:    bge a1, a0, 0x58 <.LBB1_18+0x2>
+; RV32IC-NEXT:    bge a1, a0, 0x5e
 ; RV32IC-NEXT:    c.mv a0, a1
-; RV32IC-LABEL: <.LBB1_20>:
 ; RV32IC-NEXT:    c.jr ra
   %val1 = load volatile i32, ptr %b
   %tst1 = icmp eq i32 0, %val1



More information about the cfe-commits mailing list