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

2023-12-24 Thread Fangrui Song via cfe-commits

MaskRay wrote:

The current patch doesn't do what the title implies ("Always emit relocations 
for resolved ").

https://github.com/llvm/llvm-project/pull/73793
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2023-12-07 Thread Andreu Carminati via cfe-commits

andcarminati wrote:

> For the driver BareMetal.cpp change, claiming `-mno-relax` should not be done 
> for non-RISCV targets (e.g. AArch32).

Thank you for the update. I updated the PR reverting the backend part and 
addressing this issue just for RISC-V. 

https://github.com/llvm/llvm-project/pull/73793
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2023-12-07 Thread Andreu Carminati via cfe-commits

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 
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 , const 
JobAction ,
 
   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 , const 
JobAction ,
   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 ,
 const MCFixup ,
 const MCValue ) {
@@ -122,13 +122,9 @@ bool RISCVAsmBackend::shouldForceRelocation(const 
MCAssembler ,
 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 ,
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 {

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

2023-12-06 Thread Fangrui Song via cfe-commits

MaskRay wrote:

For the driver BareMetal.cpp change, claiming `-mno-relax` should not be done 
for non-RISCV targets (e.g. AArch32).

https://github.com/llvm/llvm-project/pull/73793
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2023-11-30 Thread Jessica Clarke via cfe-commits

jrtc27 wrote:

> Also, we cannot trust Subtarget features here, because it may be empty in 
> case of LTO codegen, preventing relaxations.

And that's the problem. It's vital that we have the information. Anything else 
is just a hack that papers over the fundamental issue.

https://github.com/llvm/llvm-project/pull/73793
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2023-11-30 Thread Jessica Clarke via cfe-commits

jrtc27 wrote:

As far as I can tell this is pointless. If you want relaxation you need 
R_RISCV_RELAX and R_RISC_ALIGN relocations to be emitted. If you don't want 
relaxation you don't need these. Therefore it seems like all this does is emit 
a whole bunch of useless relocations for the case when you're not enabling 
relaxation at compile time and thus cannot possibly enable it at link time?

https://github.com/llvm/llvm-project/pull/73793
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2023-11-29 Thread Andreu Carminati via cfe-commits

https://github.com/andcarminati edited 
https://github.com/llvm/llvm-project/pull/73793
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits