llvm: Default to -mno-relax

[kp]

Summary

Compiling on a RISC-V system fails with 'relocation R_RISCV_ALIGN
requires unimplemented linker relaxation; recompile with -mno-relax'.

Our default linker (ld.lld) doesn't support relaxation, so default to
no-relax so we don't generate object files the linker can't handle.

Sponsored by: Axiado

[mhorne]

When does this error show up exactly? When performing native src builds, or ports, or something else?

Relaxations are supported by ld.bfd, so I don't want to see them disabled universally.

@jhb added the riscv-relaxations flag to LINKER_FEATURES in rS355428 and rS356504, which is meant to help us detect when the linker supports this feature. If there is some case that is not covered by those commits, then we should try to address that case specifically.

[jrtc27]

Agreed; this is too big a hammer (and one day LLD will gain the ability to do the relaxations too; I posted a proof-of-concept for R_RISCV_ALIGN a couple of months ago and there was some activity from others).

[kp]

I run into this trying to build ports, but this is the simplest reproduction case:

# cat t.c
#include <stdio.h>

int main (void)
{
        printf("Hello\n");
        return (0);
}
root@pkgbuilder:~ # cc t.c -o t
ld: error: t.c:(.text+0x0): relocation R_RISCV_ALIGN requires unimplemented linker relaxation; recompile with -mno-relax
cc: error: linker command failed with exit code 1 (use -v to see invocation)
root@pkgbuilder:~ # cc -mno-relax t.c -o t
root@pkgbuilder:~ # ./t
Hello

[jrtc27]

In D25210#556611, @kp wrote:

I run into this trying to build ports, but this is the simplest reproduction case:

# cat t.c
#include <stdio.h>

int main (void)
{
        printf("Hello\n");
        return (0);
}
root@pkgbuilder:~ # cc t.c -o t
ld: error: t.c:(.text+0x0): relocation R_RISCV_ALIGN requires unimplemented linker relaxation; recompile with -mno-relax
cc: error: linker command failed with exit code 1 (use -v to see invocation)
root@pkgbuilder:~ # cc -mno-relax t.c -o t
root@pkgbuilder:~ # ./t
Hello

That test doesn't show anything, we know the flag is required by default, and there's no easy way for Clang to know what your linker is at compile time (it doesn't get LDFLAGS normally, and you don't know what /usr/bin/ld is).

[arichardson]

In D25210#556612, @jrtc27 wrote:

That test doesn't show anything, we know the flag is required by default, and there's no easy way for Clang to know what your linker is at compile time (it doesn't get LDFLAGS normally, and you don't know what /usr/bin/ld is).

Yes, but it is quite unhelpful considering that we know that /usr/bin/ld is ld.lld. Defaulting /usr/bin/cc to -mno-relax would be more user-friendly.

[kp]

In D25210#556612, @jrtc27 wrote:

That test doesn't show anything, we know the flag is required by default, and there's no easy way for Clang to know what your linker is at compile time (it doesn't get LDFLAGS normally, and you don't know what /usr/bin/ld is).

I run into this trying to natively build ports. That doesn't output much in the way of useful debugging information though. The first thing we always try to build is pkg. The pkg build runs autosetup/autosetup-find-tclsh, which does $cc -o "$d/jimsh0" "$d/jimsh0.c". That build fails, failing the entire pkg build.

What are build expected to do other than to add -mno-relax to CFLAGS?

[jrtc27]

Patching the in-tree Clang to default to -mno-relax for FreeBSD might make sense. But out-of-tree can be used with BFD so that's not upstreamable.

[arichardson]

In D25210#556616, @jrtc27 wrote:

Patching the in-tree Clang to default to -mno-relax for FreeBSD might make sense. But out-of-tree can be used with BFD so that's not upstreamable.

I agree. I feel like having a kyua test that compiles and runs helloworld.c would be quite useful...

[mhorne]

In D25210#556612, @jrtc27 wrote:

In D25210#556611, @kp wrote:

I run into this trying to build ports, but this is the simplest reproduction case:

# cat t.c
#include <stdio.h>

int main (void)
{
        printf("Hello\n");
        return (0);
}
root@pkgbuilder:~ # cc t.c -o t
ld: error: t.c:(.text+0x0): relocation R_RISCV_ALIGN requires unimplemented linker relaxation; recompile with -mno-relax
cc: error: linker command failed with exit code 1 (use -v to see invocation)
root@pkgbuilder:~ # cc -mno-relax t.c -o t
root@pkgbuilder:~ # ./t
Hello

That test doesn't show anything, we know the flag is required by default, and there's no easy way for Clang to know what your linker is at compile time (it doesn't get LDFLAGS normally, and you don't know what /usr/bin/ld is).

I wonder if we might just flip llvm's default locally to not emit relaxations until the time that lld supports them?

Edit: didn't refresh the page and I missed the messages suggesting exactly this.

[kp]

That sounds good to me too.

Untested:

diff --git a/contrib/llvm-project/clang/lib/Driver/ToolChains/Arch/RISCV.cpp b/contrib/llvm-project/clang/lib/Driver/ToolChains/Arch/RISCV.cpp
index 8c343b8693f..d049285e4f7 100644
--- a/contrib/llvm-project/clang/lib/Driver/ToolChains/Arch/RISCV.cpp
+++ b/contrib/llvm-project/clang/lib/Driver/ToolChains/Arch/RISCV.cpp
@@ -426,8 +426,8 @@ void riscv::getRISCVTargetFeatures(const Driver &D, const llvm::Triple &Triple,
   if (Args.hasArg(options::OPT_ffixed_x31))
     Features.push_back("+reserve-x31");

-  // -mrelax is default, unless -mno-relax is specified.
-  if (Args.hasFlag(options::OPT_mrelax, options::OPT_mno_relax, true))
+  // -mno-relax is default, unless -mrelax is specified.
+  if (Args.hasFlag(options::OPT_mrelax, options::OPT_mno_relax, false))
     Features.push_back("+relax");
   else
     Features.push_back("-relax");

I've kicked off a build with that, but it'll take a few hours before I have results.

[kp]

Change the LLVM default

[jhb]

I think changing the in-tree compiler locally is the right fix. I do think we should strive to have /usr/bin/cc Just Work(tm) for the native architecture without needing other flags. I think @mhorne 's proposed hello compile and run test isn't a bad idea either.

[mhorne]

In D25210#556690, @jhb wrote:

I think changing the in-tree compiler locally is the right fix. I do think we should strive to have /usr/bin/cc Just Work(tm) for the native architecture without needing other flags. I think @mhorne 's proposed hello compile and run test isn't a bad idea either.

That was @arichardson's suggestion, but I'm in agreement.

We should take note of the other defaults, e.g. that -march and -mabi will default to hard-float if left unspecified. I think this means that a plain cc would fail to link on a riscv64sf world.

[jrtc27]

contrib/llvm-project/clang/lib/Driver/ToolChains/Arch/RISCV.cpp

429 ↗

(On Diff #73033)

Maybe this should be clearly indicated as a local change?

[kp]

Add a comment to indicate this is a FreeBSD-specific change.

[phk]

I can confirm that this patch allows me to compile C programs on a QEMU riscv64 system.