[clang] [llvm] [Driver] Default -msmall-data-limit= to 0 and clean up code (PR #83093)
via cfe-commits
cfe-commits at lists.llvm.org
Mon Feb 26 17:59:39 PST 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-driver
@llvm/pr-subscribers-clang
@llvm/pr-subscribers-backend-risc-v
Author: Fangrui Song (MaskRay)
<details>
<summary>Changes</summary>
D57497 added -msmall-data-limit= as an alias for -G and defaulted it to 8 for
-fno-pic/-fpie.
The behavior is already different from GCC in a few ways:
* GCC doesn't accept -G.
* GCC -fpie doesn't seem to use -msmall-data-limit=.
* GCC emits .srodata.cst* that we don't use (#<!-- -->82214). Writable contents
caused confusion (https://bugs.chromium.org/p/llvm/issues/detail?id=61)
In addition,
* claiming `-shared` means we don't get a desired `-Wunused-command-line-argument` for `clang --target=riscv64-linux-gnu -fpic -c -shared a.c`.
* -mcmodel=large doesn't work for RISC-V yet, so the special case is strange.
* It's quite unusual to emit a warning when an option (unrelated to relocation model) is used with -fpic.
* We don't want future configurations (Android) to continue adding customization to `SetRISCVSmallDataLimit`.
I believe the extra code just doesn't pull its weight and should be
cleaned up. This patch also changes the default to 0. GP relaxation
users are encouraged to specify these customization options explicitly.
---
Full diff: https://github.com/llvm/llvm-project/pull/83093.diff
5 Files Affected:
- (modified) clang/lib/Driver/ToolChains/Clang.cpp (+4-37)
- (modified) clang/test/CodeGen/RISCV/riscv-sdata-module-flag.c (+2-2)
- (removed) clang/test/Driver/riscv-sdata-warning.c (-4)
- (added) clang/test/Driver/riscv-sdata.c (+5)
- (modified) llvm/lib/Target/RISCV/RISCVTargetObjectFile.h (+1-1)
``````````diff
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 6e1b7e8657d0dc..9b9eab3990607f 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -2029,42 +2029,6 @@ void Clang::AddPPCTargetArgs(const ArgList &Args,
}
}
-static void SetRISCVSmallDataLimit(const ToolChain &TC, const ArgList &Args,
- ArgStringList &CmdArgs) {
- const Driver &D = TC.getDriver();
- const llvm::Triple &Triple = TC.getTriple();
- // Default small data limitation is eight.
- const char *SmallDataLimit = "8";
- // Get small data limitation.
- if (Args.getLastArg(options::OPT_shared, options::OPT_fpic,
- options::OPT_fPIC)) {
- // Not support linker relaxation for PIC.
- SmallDataLimit = "0";
- if (Args.hasArg(options::OPT_G)) {
- D.Diag(diag::warn_drv_unsupported_sdata);
- }
- } else if (Args.getLastArgValue(options::OPT_mcmodel_EQ)
- .equals_insensitive("large") &&
- (Triple.getArch() == llvm::Triple::riscv64)) {
- // Not support linker relaxation for RV64 with large code model.
- SmallDataLimit = "0";
- if (Args.hasArg(options::OPT_G)) {
- D.Diag(diag::warn_drv_unsupported_sdata);
- }
- } else if (Triple.isAndroid()) {
- // GP relaxation is not supported on Android.
- SmallDataLimit = "0";
- if (Args.hasArg(options::OPT_G)) {
- D.Diag(diag::warn_drv_unsupported_sdata);
- }
- } else if (Arg *A = Args.getLastArg(options::OPT_G)) {
- SmallDataLimit = A->getValue();
- }
- // Forward the -msmall-data-limit= option.
- CmdArgs.push_back("-msmall-data-limit");
- CmdArgs.push_back(SmallDataLimit);
-}
-
void Clang::AddRISCVTargetArgs(const ArgList &Args,
ArgStringList &CmdArgs) const {
const llvm::Triple &Triple = getToolChain().getTriple();
@@ -2073,7 +2037,10 @@ void Clang::AddRISCVTargetArgs(const ArgList &Args,
CmdArgs.push_back("-target-abi");
CmdArgs.push_back(ABIName.data());
- SetRISCVSmallDataLimit(getToolChain(), Args, CmdArgs);
+ if (Arg *A = Args.getLastArg(options::OPT_G)) {
+ CmdArgs.push_back("-msmall-data-limit");
+ CmdArgs.push_back(A->getValue());
+ }
if (!Args.hasFlag(options::OPT_mimplicit_float,
options::OPT_mno_implicit_float, true))
diff --git a/clang/test/CodeGen/RISCV/riscv-sdata-module-flag.c b/clang/test/CodeGen/RISCV/riscv-sdata-module-flag.c
index 9bd69e7995877e..e91b2e7f225f52 100644
--- a/clang/test/CodeGen/RISCV/riscv-sdata-module-flag.c
+++ b/clang/test/CodeGen/RISCV/riscv-sdata-module-flag.c
@@ -30,14 +30,14 @@
void test(void) {}
-// RV32-DEFAULT: !{i32 8, !"SmallDataLimit", i32 8}
+// RV32-DEFAULT: !{i32 8, !"SmallDataLimit", i32 0}
// RV32-G4: !{i32 8, !"SmallDataLimit", i32 4}
// RV32-S0: !{i32 8, !"SmallDataLimit", i32 0}
// RV32-S2G4: !{i32 8, !"SmallDataLimit", i32 4}
// RV32-T16: !{i32 8, !"SmallDataLimit", i32 16}
// RV32-PIC: !{i32 8, !"SmallDataLimit", i32 0}
-// RV64-DEFAULT: !{i32 8, !"SmallDataLimit", i32 8}
+// RV64-DEFAULT: !{i32 8, !"SmallDataLimit", i32 0}
// RV64-G4: !{i32 8, !"SmallDataLimit", i32 4}
// RV64-S0: !{i32 8, !"SmallDataLimit", i32 0}
// RV64-S2G4: !{i32 8, !"SmallDataLimit", i32 4}
diff --git a/clang/test/Driver/riscv-sdata-warning.c b/clang/test/Driver/riscv-sdata-warning.c
deleted file mode 100644
index ace9a32ee116fd..00000000000000
--- a/clang/test/Driver/riscv-sdata-warning.c
+++ /dev/null
@@ -1,4 +0,0 @@
-// REQUIRES: riscv-registered-target
-// RUN: %clang -S --target=riscv32-unknown-elf -fpic -msmall-data-limit=8 %s 2>&1 \
-// RUN: | FileCheck -check-prefix=CHECK-PIC-SDATA %s
-// CHECK-PIC-SDATA: warning: ignoring '-msmall-data-limit=' with -mcmodel=large for -fpic or RV64
diff --git a/clang/test/Driver/riscv-sdata.c b/clang/test/Driver/riscv-sdata.c
new file mode 100644
index 00000000000000..84cd109813db4c
--- /dev/null
+++ b/clang/test/Driver/riscv-sdata.c
@@ -0,0 +1,5 @@
+// RUN: %clang -### -S --target=riscv64 %s 2>&1 | FileCheck %s
+// RUN: %clang -### -S --target=riscv64 -msmall-data-limit=8 %s 2>&1 | FileCheck %s --check-prefix=EIGHT
+
+// CHECK-NOT: "-msmall-data-limit"
+// EIGHT: "-msmall-data-limit" "8"
diff --git a/llvm/lib/Target/RISCV/RISCVTargetObjectFile.h b/llvm/lib/Target/RISCV/RISCVTargetObjectFile.h
index 0910fbd3d95041..2d4fa211fa303e 100644
--- a/llvm/lib/Target/RISCV/RISCVTargetObjectFile.h
+++ b/llvm/lib/Target/RISCV/RISCVTargetObjectFile.h
@@ -17,7 +17,7 @@ namespace llvm {
class RISCVELFTargetObjectFile : public TargetLoweringObjectFileELF {
MCSection *SmallDataSection;
MCSection *SmallBSSSection;
- unsigned SSThreshold = 8;
+ unsigned SSThreshold = 0;
public:
unsigned getTextSectionAlignment() const override;
``````````
</details>
https://github.com/llvm/llvm-project/pull/83093
More information about the cfe-commits
mailing list