[clang] [clang][Driver][RISCV] Honor -m[no-]global-merge for RISC-V (PR #79372)

Jonathon Penix via cfe-commits cfe-commits at lists.llvm.org
Wed Jan 24 15:34:26 PST 2024


https://github.com/jonathonpenix updated https://github.com/llvm/llvm-project/pull/79372

>From 37cf78919afbfb6eb4e1bc36a3d8fbb678ffd821 Mon Sep 17 00:00:00 2001
From: Jonathon Penix <jpenix at quicinc.com>
Date: Tue, 23 Jan 2024 15:10:50 -0800
Subject: [PATCH 1/2] [clang][Driver][RISCV] Honor -m[no-]global-merge for
 RISC-V

The GlobalMerge pass was enabled for RISC-V in [1] and left off by
default. My understanding from [2] is that the concern around enabling
the pass by default stemmed primarily from GlobalMerge preventing GP
relaxation, which negatively impacted a few benchmarks.

However, as a) this pass provides code size benefits in some cases and b)
this shouldn't be a concern for situations where GP relaxation is disabled
anyway, we'd like to be able to manually enable/disable this functionality
without having to specify -mllvm flags and in a way that is consistent with
other targets (Arm/AArch64). So, leave the pass off by default still, but
honor the -m[no]-global-merge flag for RISC-V.

[1] https://reviews.llvm.org/D130481
[2] https://reviews.llvm.org/D129178
---
 clang/lib/Driver/ToolChains/Clang.cpp | 10 ++++++++++
 clang/test/Driver/mglobal-merge.c     | 13 +++++++++++++
 2 files changed, 23 insertions(+)

diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index bcba7cbbdb58c2..08bbec5881cf7a 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -2129,6 +2129,16 @@ void Clang::AddRISCVTargetArgs(const ArgList &Args,
           << A->getSpelling() << Val;
     }
   }
+
+  // Forward the -mglobal-merge option for explicit control over the pass.
+  if (Arg *A = Args.getLastArg(options::OPT_mglobal_merge,
+                               options::OPT_mno_global_merge)) {
+    CmdArgs.push_back("-mllvm");
+    if (A->getOption().matches(options::OPT_mno_global_merge))
+      CmdArgs.push_back("-riscv-enable-global-merge=false");
+    else
+      CmdArgs.push_back("-riscv-enable-global-merge=true");
+  }
 }
 
 void Clang::AddSparcTargetArgs(const ArgList &Args,
diff --git a/clang/test/Driver/mglobal-merge.c b/clang/test/Driver/mglobal-merge.c
index 4ea7ae03e78f41..d825527536a861 100644
--- a/clang/test/Driver/mglobal-merge.c
+++ b/clang/test/Driver/mglobal-merge.c
@@ -6,12 +6,17 @@
 // RUN:   -mno-global-merge
 // RUN: FileCheck --check-prefix=CHECK-NGM-AARCH64 < %t %s
 
+// RUN: %clang -target riscv32-unknown-unknown -### -fsyntax-only %s 2> %t \
+// RUN:   -mno-global-merge
+// RUN: FileCheck --check-prefix=CHECK-NGM-RISCV < %t %s
+
 // RUN: %clang -target x86_64-unknown-unknown -### -fsyntax-only %s 2> %t \
 // RUN:   -mno-global-merge
 // RUN: FileCheck --check-prefix=CHECK-NONE < %t %s
 
 // CHECK-NGM-ARM: "-mllvm" "-arm-global-merge=false"
 // CHECK-NGM-AARCH64: "-mllvm" "-aarch64-enable-global-merge=false"
+// CHECK-NGM-RISCV: "-mllvm" "-riscv-enable-global-merge=false"
 
 // RUN: %clang -target armv7-unknown-unknown -### -fsyntax-only %s 2> %t \
 // RUN:   -mglobal-merge
@@ -21,12 +26,17 @@
 // RUN:   -mglobal-merge
 // RUN: FileCheck --check-prefix=CHECK-GM-AARCH64 < %t %s
 
+// RUN: %clang -target riscv32-unknown-unknown -### -fsyntax-only %s 2> %t \
+// RUN:   -mglobal-merge
+// RUN: FileCheck --check-prefix=CHECK-GM-RISCV < %t %s
+
 // RUN: %clang -target x86_64-unknown-unknown -### -fsyntax-only %s 2> %t \
 // RUN:   -mglobal-merge
 // RUN: FileCheck --check-prefix=CHECK-NONE < %t %s
 
 // CHECK-GM-ARM: "-mllvm" "-arm-global-merge=true"
 // CHECK-GM-AARCH64: "-mllvm" "-aarch64-enable-global-merge=true"
+// CHECK-GM-RISCV: "-mllvm" "-riscv-enable-global-merge=true"
 
 // RUN: %clang -target armv7-unknown-unknown -### -fsyntax-only %s 2> %t
 // RUN: FileCheck --check-prefix=CHECK-NONE < %t %s
@@ -34,6 +44,9 @@
 // RUN: %clang -target aarch64-unknown-unknown -### -fsyntax-only %s 2> %t
 // RUN: FileCheck --check-prefix=CHECK-NONE < %t %s
 
+// RUN: %clang -target riscv32-unknown-unknown -### -fsyntax-only %s 2> %t
+// RUN: FileCheck --check-prefix=CHECK-NONE < %t %s
+
 // RUN: %clang -target x86_64-unknown-unknown -### -fsyntax-only %s 2> %t
 // RUN: FileCheck --check-prefix=CHECK-NONE < %t %s
 

>From 3657d76bfbad18bd3a487832284bee2d799f9f99 Mon Sep 17 00:00:00 2001
From: Jonathon Penix <jpenix at quicinc.com>
Date: Wed, 24 Jan 2024 15:25:49 -0800
Subject: [PATCH 2/2] Don't explicitly specify =true when passing
 -riscv-enable-global-merge

---
 clang/lib/Driver/ToolChains/Clang.cpp | 2 +-
 clang/test/Driver/mglobal-merge.c     | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 08bbec5881cf7a..f184575ddc9f39 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -2137,7 +2137,7 @@ void Clang::AddRISCVTargetArgs(const ArgList &Args,
     if (A->getOption().matches(options::OPT_mno_global_merge))
       CmdArgs.push_back("-riscv-enable-global-merge=false");
     else
-      CmdArgs.push_back("-riscv-enable-global-merge=true");
+      CmdArgs.push_back("-riscv-enable-global-merge");
   }
 }
 
diff --git a/clang/test/Driver/mglobal-merge.c b/clang/test/Driver/mglobal-merge.c
index d825527536a861..544c8496bae9ad 100644
--- a/clang/test/Driver/mglobal-merge.c
+++ b/clang/test/Driver/mglobal-merge.c
@@ -36,7 +36,7 @@
 
 // CHECK-GM-ARM: "-mllvm" "-arm-global-merge=true"
 // CHECK-GM-AARCH64: "-mllvm" "-aarch64-enable-global-merge=true"
-// CHECK-GM-RISCV: "-mllvm" "-riscv-enable-global-merge=true"
+// CHECK-GM-RISCV: "-mllvm" "-riscv-enable-global-merge"
 
 // RUN: %clang -target armv7-unknown-unknown -### -fsyntax-only %s 2> %t
 // RUN: FileCheck --check-prefix=CHECK-NONE < %t %s



More information about the cfe-commits mailing list