[clang] [Driver] Don't default to -mrelax-all for non-RISCV -O0 (PR #90013)
Fangrui Song via cfe-commits
cfe-commits at lists.llvm.org
Wed Apr 24 20:14:18 PDT 2024
https://github.com/MaskRay created https://github.com/llvm/llvm-project/pull/90013
Some assembly mnemonics may assemble to instructions of different
lengths. The longer form is to support instructions like a long branch.
-mrelax-all enables `MCRelaxAll`, which expands instructions to the long
form regardless of whether a short form suffices, while -mno-relax-all
only expands instructions when needed.
```
// x86 example
void foo(int a) {
// -mno-relax-all or gas: short jump (2 bytes)
// -mrelax-all: near jump (6 bytes)
if (a) bar();
}
```
The -mrelax-all default for non-RISCV -O0 increases code size with
non-perceivable compile time difference for a stage-2 x86-64 build of
lld.
```
-mrelax-all: file size: 58.2MiB VM size: 49.7MiB
-mno-relax-all: file size: 60.9MiB VM size: 52.4MiB
```
There is no compile time difference (other than noise) GNU assembler
doesn't expand instructions by default. Let's remove the -mrelax-all default.
>From f1cbcac55c0816882277a1a3ecf1af47e7558bb3 Mon Sep 17 00:00:00 2001
From: Fangrui Song <i at maskray.me>
Date: Wed, 24 Apr 2024 20:14:08 -0700
Subject: [PATCH] =?UTF-8?q?[=F0=9D=98=80=F0=9D=97=BD=F0=9D=97=BF]=20initia?=
=?UTF-8?q?l=20version?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Created using spr 1.3.5-bogner
---
clang/lib/Driver/ToolChains/Clang.cpp | 52 ++++++---------------------
clang/test/Driver/integrated-as.c | 2 +-
2 files changed, 11 insertions(+), 43 deletions(-)
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 5f5d720cf759f4..651a2b5aac368b 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -835,46 +835,6 @@ static void addPGOAndCoverageFlags(const ToolChain &TC, Compilation &C,
}
}
-/// Check whether the given input tree contains any compilation actions.
-static bool ContainsCompileAction(const Action *A) {
- if (isa<CompileJobAction>(A) || isa<BackendJobAction>(A))
- return true;
-
- return llvm::any_of(A->inputs(), ContainsCompileAction);
-}
-
-/// Check if -relax-all should be passed to the internal assembler.
-/// This is done by default when compiling non-assembler source with -O0.
-static bool UseRelaxAll(Compilation &C, const ArgList &Args) {
- bool RelaxDefault = true;
-
- 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()) {
- if (ContainsCompileAction(Act)) {
- RelaxDefault = true;
- break;
- }
- }
- }
-
- return Args.hasFlag(options::OPT_mrelax_all, options::OPT_mno_relax_all,
- RelaxDefault);
-}
-
static void
RenderDebugEnablingArgs(const ArgList &Args, ArgStringList &CmdArgs,
llvm::codegenoptions::DebugInfoKind DebugInfoKind,
@@ -2472,8 +2432,16 @@ static void CollectArgsForIntegratedAssembler(Compilation &C,
const ArgList &Args,
ArgStringList &CmdArgs,
const Driver &D) {
- if (UseRelaxAll(C, Args))
- CmdArgs.push_back("-mrelax-all");
+ // Default to -mno-relax-all.
+ //
+ // Note: 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.
+ Args.addOptInFlag(CmdArgs, options::OPT_mrelax_all,
+ options::OPT_mno_relax_all);
// Only default to -mincremental-linker-compatible if we think we are
// targeting the MSVC linker.
diff --git a/clang/test/Driver/integrated-as.c b/clang/test/Driver/integrated-as.c
index e78fde873cf47f..b0a26f6011b0c7 100644
--- a/clang/test/Driver/integrated-as.c
+++ b/clang/test/Driver/integrated-as.c
@@ -3,7 +3,7 @@
// RUN: %clang -### -c -save-temps -integrated-as --target=x86_64 %s 2>&1 | FileCheck %s
// CHECK: cc1as
-// CHECK: -mrelax-all
+// CHECK-NOT: -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
More information about the cfe-commits
mailing list