Skip to content

Commit

Permalink
[RISCV][clang] Don't enable -mrelax-all for -O0 on RISC-V (#88538)
Browse files Browse the repository at this point in the history
-O0 implies -mrelax-all as an assembler compile time optimization.
-mrelax-all allows the assembler to complete layout in 2 passes instead
of doing iterative branch relaxation.

Jump offsets larger than +/-1MiB require an indirect jump on RISC-V.
This can't be done by the assembler, so we use a branch relaxation MIR
pass and use register scavenging to find a free register.

The conditional branch offsets for RISC-V are also somewhat small so we
support MC layer branch relaxation to make life easier for assembly
programmers. This may also cover up bugs in our function size estimation
in MachineIR.

Enabling -mrelax-all causes the MC layer relaxation to agressively relax
branches. This increases code size and can create cases where we need an
indirect jump, but we can't create one. This leads to linker failures.

The easiest way to avoid this is to not default to -mrelax-all for -O0
and sacrifice the compile time optimization. That's what this patch
does.

Fixes #87127
  • Loading branch information
topperc authored Apr 22, 2024
1 parent dd79632 commit 6b1b4c1
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 1 deletion.
10 changes: 10 additions & 0 deletions clang/lib/Driver/ToolChains/Clang.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -847,6 +847,16 @@ static bool UseRelaxAll(Compilation &C, const ArgList &Args) {
if (Arg *A = Args.getLastArg(options::OPT_O_Group))
RelaxDefault = A->getOption().matches(options::OPT_O0);

// RISC-V requires an indirect jump for offsets larger than 1MiB. This cannot
// be done by assembler branch relaxation as it needs a free temporary
// register. Because of this, branch relaxation is handled by a MachineIR
// pass before the assembler. Forcing assembler branch relaxation for -O0
// makes the MachineIR branch relaxation inaccurate and it will miss cases
// where an indirect branch is necessary. To avoid this issue we are
// sacrificing the compile time improvement of using -mrelax-all for -O0.
if (C.getDefaultToolChain().getTriple().isRISCV())
RelaxDefault = false;

if (RelaxDefault) {
RelaxDefault = false;
for (const auto &Act : C.getActions()) {
Expand Down
8 changes: 7 additions & 1 deletion clang/test/Driver/integrated-as.c
Original file line number Diff line number Diff line change
@@ -1,10 +1,16 @@
// XFAIL: target={{.*}}-aix{{.*}}

// RUN: %clang -### -c -save-temps -integrated-as %s 2>&1 | FileCheck %s
// RUN: %clang -### -c -save-temps -integrated-as --target=x86_64 %s 2>&1 | FileCheck %s

// CHECK: cc1as
// CHECK: -mrelax-all

// RISC-V does not enable -mrelax-all
// RUN: %clang -### -c -save-temps -integrated-as --target=riscv64 %s 2>&1 | FileCheck %s -check-prefix=RISCV-RELAX

// RISCV-RELAX: cc1as
// RISCV-RELAX-NOT: -mrelax-all

// RUN: %clang -### -fintegrated-as -c -save-temps %s 2>&1 | FileCheck %s -check-prefix FIAS

// FIAS: cc1as
Expand Down

0 comments on commit 6b1b4c1

Please sign in to comment.