From 269df4cd7681063243bb05b23dbf50fcc394338b Mon Sep 17 00:00:00 2001 From: Alex Bradbury Date: Fri, 8 Nov 2024 14:00:06 +0000 Subject: [PATCH 1/2] [RISCV] When using global merging, don't enable merging of external globals by default AArch64 left this disabled after seeing some cases of slightly worse codegen that weren't tracked down, so I suggest as a path to incrementally moving towards enable globals merging we follow suit, and evaluate turning on later. This patch disables merging of external globals, but also adds a flag to override that. This reduces churn in test cases, simplifies benchmarking runs, and this flag can be removed later. --- llvm/lib/Target/RISCV/RISCVTargetMachine.cpp | 10 ++++++++- .../global-merge-minsize-smalldata-nonzero.ll | 7 +++--- .../global-merge-minsize-smalldata-zero.ll | 7 +++--- .../CodeGen/RISCV/global-merge-minsize.ll | 6 ++--- .../test/CodeGen/RISCV/global-merge-offset.ll | 8 +++---- llvm/test/CodeGen/RISCV/global-merge.ll | 22 +++++++++++++++++-- 6 files changed, 44 insertions(+), 16 deletions(-) diff --git a/llvm/lib/Target/RISCV/RISCVTargetMachine.cpp b/llvm/lib/Target/RISCV/RISCVTargetMachine.cpp index a5afbcfd79710..6a97755c279a2 100644 --- a/llvm/lib/Target/RISCV/RISCVTargetMachine.cpp +++ b/llvm/lib/Target/RISCV/RISCVTargetMachine.cpp @@ -53,6 +53,13 @@ static cl::opt EnableGlobalMerge("riscv-enable-global-merge", cl::Hidden, cl::desc("Enable the global merge pass")); +static cl::opt ForceEnableGlobalMergeExternalGlobals( + "riscv-force-enable-global-merge-external-globals", cl::Hidden, + cl::init(false), + cl::desc( + "If the global merge pass is enabled, force enable global merging of " + "external globals (overriding any logic that might disable it)")); + static cl::opt EnableMachineCombiner("riscv-enable-machine-combiner", cl::desc("Enable the machine combiner pass"), @@ -472,7 +479,8 @@ bool RISCVPassConfig::addPreISel() { if (EnableGlobalMerge == cl::BOU_TRUE) { addPass(createGlobalMergePass(TM, /* MaxOffset */ 2047, /* OnlyOptimizeForSize */ false, - /* MergeExternalByDefault */ true)); + /* MergeExternalByDefault */ + ForceEnableGlobalMergeExternalGlobals)); } return false; diff --git a/llvm/test/CodeGen/RISCV/global-merge-minsize-smalldata-nonzero.ll b/llvm/test/CodeGen/RISCV/global-merge-minsize-smalldata-nonzero.ll index c547138930212..39c677ac20b3a 100644 --- a/llvm/test/CodeGen/RISCV/global-merge-minsize-smalldata-nonzero.ll +++ b/llvm/test/CodeGen/RISCV/global-merge-minsize-smalldata-nonzero.ll @@ -1,8 +1,9 @@ ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py -; RUN: llc -mtriple=riscv64 -riscv-enable-global-merge -verify-machineinstrs < %s \ -; RUN: | FileCheck %s -check-prefix=SMALL-DATA +; RUN: llc -mtriple=riscv64 -riscv-enable-global-merge -riscv-force-enable-global-merge-external-globals \ +; RUN: -verify-machineinstrs < %s | FileCheck %s -check-prefix=SMALL-DATA ; RUN: llc -mtriple=riscv64 -riscv-enable-global-merge -global-merge-min-data-size=0 \ -; RUN: -verify-machineinstrs < %s | FileCheck %s -check-prefix=MINSIZE +; RUN: -riscv-force-enable-global-merge-external-globals -verify-machineinstrs < %s \ +; RUN: | FileCheck %s -check-prefix=MINSIZE @ig1 = internal global i32 0, align 4 @ig2 = internal global i32 0, align 4 diff --git a/llvm/test/CodeGen/RISCV/global-merge-minsize-smalldata-zero.ll b/llvm/test/CodeGen/RISCV/global-merge-minsize-smalldata-zero.ll index 8e4d72af00ebc..d2b714577db9a 100644 --- a/llvm/test/CodeGen/RISCV/global-merge-minsize-smalldata-zero.ll +++ b/llvm/test/CodeGen/RISCV/global-merge-minsize-smalldata-zero.ll @@ -1,8 +1,9 @@ ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py -; RUN: llc -mtriple=riscv32 -riscv-enable-global-merge -verify-machineinstrs < %s \ -; RUN: | FileCheck %s -check-prefix=SMALL-DATA +; RUN: llc -mtriple=riscv32 -riscv-enable-global-merge -riscv-force-enable-global-merge-external-globals \ +; RUN: -verify-machineinstrs < %s | FileCheck %s -check-prefix=SMALL-DATA ; RUN: llc -mtriple=riscv64 -riscv-enable-global-merge -global-merge-min-data-size=5 \ -; RUN: -verify-machineinstrs < %s | FileCheck %s -check-prefix=MINSIZE +; RUN: -riscv-force-enable-global-merge-external-globals -verify-machineinstrs < %s \ +; RUN: | FileCheck %s -check-prefix=MINSIZE @ig1 = internal global i32 0, align 4 @ig2 = internal global i32 0, align 4 diff --git a/llvm/test/CodeGen/RISCV/global-merge-minsize.ll b/llvm/test/CodeGen/RISCV/global-merge-minsize.ll index e405425832acb..696d163bdcb2c 100644 --- a/llvm/test/CodeGen/RISCV/global-merge-minsize.ll +++ b/llvm/test/CodeGen/RISCV/global-merge-minsize.ll @@ -1,8 +1,8 @@ ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py -; RUN: llc -mtriple=riscv32 -riscv-enable-global-merge -verify-machineinstrs < %s \ -; RUN: | FileCheck %s -check-prefix=RV32 +; RUN: llc -mtriple=riscv32 -riscv-enable-global-merge -riscv-force-enable-global-merge-external-globals \ +; RUN: -verify-machineinstrs < %s | FileCheck %s -check-prefix=RV32 ; RUN: llc -mtriple=riscv32 -riscv-enable-global-merge -global-merge-min-data-size=5 \ -; RUN: -verify-machineinstrs < %s | FileCheck %s -check-prefix=RV32-MINSIZE +; RUN: -riscv-force-enable-global-merge-external-globals -verify-machineinstrs < %s | FileCheck %s -check-prefix=RV32-MINSIZE @ig1 = internal global i32 0, align 4 @ig2 = internal global i32 0, align 4 diff --git a/llvm/test/CodeGen/RISCV/global-merge-offset.ll b/llvm/test/CodeGen/RISCV/global-merge-offset.ll index 13afcba181719..0c0881ddf2873 100644 --- a/llvm/test/CodeGen/RISCV/global-merge-offset.ll +++ b/llvm/test/CodeGen/RISCV/global-merge-offset.ll @@ -1,12 +1,12 @@ ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py ; RUN: sed 's/ArrSize/100/g' %s | llc -mtriple=riscv32 -riscv-enable-global-merge \ -; RUN: -verify-machineinstrs | FileCheck %s +; RUN: -riscv-force-enable-global-merge-external-globals -verify-machineinstrs | FileCheck %s ; RUN: sed 's/ArrSize/100/g' %s | llc -mtriple=riscv64 -riscv-enable-global-merge \ -; RUN: -verify-machineinstrs | FileCheck %s +; RUN: -riscv-force-enable-global-merge-external-globals -verify-machineinstrs | FileCheck %s ; RUN: sed 's/ArrSize/101/g' %s | llc -mtriple=riscv32 -riscv-enable-global-merge \ -; RUN: -verify-machineinstrs | FileCheck %s -check-prefix=CHECK-TOOBIG +; RUN: -riscv-force-enable-global-merge-external-globals -verify-machineinstrs | FileCheck %s -check-prefix=CHECK-TOOBIG ; RUN: sed 's/ArrSize/101/g' %s | llc -mtriple=riscv64 -riscv-enable-global-merge \ -; RUN: -verify-machineinstrs | FileCheck %s -check-prefix=CHECK-TOOBIG +; RUN: -riscv-force-enable-global-merge-external-globals -verify-machineinstrs | FileCheck %s -check-prefix=CHECK-TOOBIG ; This test demonstrates that the MaxOffset is set correctly for RISC-V by ; constructing an input that is at the limit and comparing. diff --git a/llvm/test/CodeGen/RISCV/global-merge.ll b/llvm/test/CodeGen/RISCV/global-merge.ll index 20379ee2e7dac..633ba719c6a30 100644 --- a/llvm/test/CodeGen/RISCV/global-merge.ll +++ b/llvm/test/CodeGen/RISCV/global-merge.ll @@ -3,6 +3,12 @@ ; RUN: | FileCheck %s ; RUN: llc -mtriple=riscv64 -riscv-enable-global-merge -verify-machineinstrs < %s \ ; RUN: | FileCheck %s +; RUN: llc -mtriple=riscv32 -riscv-enable-global-merge \ +; RUN: -riscv-force-enable-global-merge-external-globals -verify-machineinstrs < %s \ +; RUN: | FileCheck -check-prefix=CHECK-WEXTERN %s +; RUN: llc -mtriple=riscv64 -riscv-enable-global-merge \ +; RUN: -riscv-force-enable-global-merge-external-globals -verify-machineinstrs < %s \ +; RUN: | FileCheck -check-prefix=CHECK-WEXTERN %s @ig1 = internal global i32 0, align 4 @ig2 = internal global i32 0, align 4 @@ -21,9 +27,21 @@ define void @f1(i32 %a) nounwind { ; CHECK-NEXT: sw a0, %lo(.L_MergedGlobals)(a1) ; CHECK-NEXT: addi a1, a1, %lo(.L_MergedGlobals) ; CHECK-NEXT: sw a0, 4(a1) -; CHECK-NEXT: sw a0, 8(a1) -; CHECK-NEXT: sw a0, 12(a1) +; CHECK-NEXT: lui a1, %hi(eg1) +; CHECK-NEXT: sw a0, %lo(eg1)(a1) +; CHECK-NEXT: lui a1, %hi(eg2) +; CHECK-NEXT: sw a0, %lo(eg2)(a1) ; CHECK-NEXT: ret +; +; CHECK-WEXTERN-LABEL: f1: +; CHECK-WEXTERN: # %bb.0: +; CHECK-WEXTERN-NEXT: lui a1, %hi(.L_MergedGlobals) +; CHECK-WEXTERN-NEXT: sw a0, %lo(.L_MergedGlobals)(a1) +; CHECK-WEXTERN-NEXT: addi a1, a1, %lo(.L_MergedGlobals) +; CHECK-WEXTERN-NEXT: sw a0, 4(a1) +; CHECK-WEXTERN-NEXT: sw a0, 8(a1) +; CHECK-WEXTERN-NEXT: sw a0, 12(a1) +; CHECK-WEXTERN-NEXT: ret store i32 %a, ptr @ig1, align 4 store i32 %a, ptr @ig2, align 4 store i32 %a, ptr @eg1, align 4 From fbeca728cac58f40e7d33d1bc5cbfade52ff8354 Mon Sep 17 00:00:00 2001 From: Alex Bradbury Date: Fri, 8 Nov 2024 14:57:35 +0000 Subject: [PATCH 2/2] [RISCV] Enable global merging by default Stacks on top of . From the discussion at the round-table at the RISC-V Summit it was clear people see cases where global merging would help. So the direction of enabling it by default and iteratively working to enable it in more cases or to improve the heuristics seems sensible. This patch tries to make a minimal step in that direction. --- llvm/lib/Target/RISCV/RISCVTargetMachine.cpp | 8 +++++++- llvm/test/CodeGen/RISCV/O3-pipeline.ll | 1 + .../RISCV/global-merge-minsize-smalldata-nonzero.ll | 4 ++-- .../RISCV/global-merge-minsize-smalldata-zero.ll | 4 ++-- llvm/test/CodeGen/RISCV/global-merge-minsize.ll | 4 ++-- llvm/test/CodeGen/RISCV/global-merge-offset.ll | 8 ++++---- llvm/test/CodeGen/RISCV/global-merge.ll | 10 ++++------ 7 files changed, 22 insertions(+), 17 deletions(-) diff --git a/llvm/lib/Target/RISCV/RISCVTargetMachine.cpp b/llvm/lib/Target/RISCV/RISCVTargetMachine.cpp index 6a97755c279a2..4fc185525d8d2 100644 --- a/llvm/lib/Target/RISCV/RISCVTargetMachine.cpp +++ b/llvm/lib/Target/RISCV/RISCVTargetMachine.cpp @@ -476,7 +476,13 @@ bool RISCVPassConfig::addPreISel() { addPass(createBarrierNoopPass()); } - if (EnableGlobalMerge == cl::BOU_TRUE) { + if ((TM->getOptLevel() != CodeGenOptLevel::None && + EnableGlobalMerge == cl::BOU_UNSET) || + EnableGlobalMerge == cl::BOU_TRUE) { + // FIXME: Like AArch64, we disable extern global merging by default due to + // concerns it might regress some workloads. Unlike AArch64, we don't + // currently support enabling the pass in an "OnlyOptimizeForSize" mode. + // Investigating and addressing both items are TODO. addPass(createGlobalMergePass(TM, /* MaxOffset */ 2047, /* OnlyOptimizeForSize */ false, /* MergeExternalByDefault */ diff --git a/llvm/test/CodeGen/RISCV/O3-pipeline.ll b/llvm/test/CodeGen/RISCV/O3-pipeline.ll index a74c842c59552..8fd9ae9850366 100644 --- a/llvm/test/CodeGen/RISCV/O3-pipeline.ll +++ b/llvm/test/CodeGen/RISCV/O3-pipeline.ll @@ -74,6 +74,7 @@ ; CHECK-NEXT: Exception handling preparation ; CHECK-NEXT: A No-Op Barrier Pass ; CHECK-NEXT: FunctionPass Manager +; CHECK-NEXT: Merge internal globals ; CHECK-NEXT: Dominator Tree Construction ; CHECK-NEXT: Basic Alias Analysis (stateless AA impl) ; CHECK-NEXT: Function Alias Analysis Results diff --git a/llvm/test/CodeGen/RISCV/global-merge-minsize-smalldata-nonzero.ll b/llvm/test/CodeGen/RISCV/global-merge-minsize-smalldata-nonzero.ll index 39c677ac20b3a..156a34db82745 100644 --- a/llvm/test/CodeGen/RISCV/global-merge-minsize-smalldata-nonzero.ll +++ b/llvm/test/CodeGen/RISCV/global-merge-minsize-smalldata-nonzero.ll @@ -1,7 +1,7 @@ ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py -; RUN: llc -mtriple=riscv64 -riscv-enable-global-merge -riscv-force-enable-global-merge-external-globals \ +; RUN: llc -mtriple=riscv64 -riscv-force-enable-global-merge-external-globals \ ; RUN: -verify-machineinstrs < %s | FileCheck %s -check-prefix=SMALL-DATA -; RUN: llc -mtriple=riscv64 -riscv-enable-global-merge -global-merge-min-data-size=0 \ +; RUN: llc -mtriple=riscv64 -global-merge-min-data-size=0 \ ; RUN: -riscv-force-enable-global-merge-external-globals -verify-machineinstrs < %s \ ; RUN: | FileCheck %s -check-prefix=MINSIZE diff --git a/llvm/test/CodeGen/RISCV/global-merge-minsize-smalldata-zero.ll b/llvm/test/CodeGen/RISCV/global-merge-minsize-smalldata-zero.ll index d2b714577db9a..e41f14d394b7c 100644 --- a/llvm/test/CodeGen/RISCV/global-merge-minsize-smalldata-zero.ll +++ b/llvm/test/CodeGen/RISCV/global-merge-minsize-smalldata-zero.ll @@ -1,7 +1,7 @@ ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py -; RUN: llc -mtriple=riscv32 -riscv-enable-global-merge -riscv-force-enable-global-merge-external-globals \ +; RUN: llc -mtriple=riscv32 -riscv-force-enable-global-merge-external-globals \ ; RUN: -verify-machineinstrs < %s | FileCheck %s -check-prefix=SMALL-DATA -; RUN: llc -mtriple=riscv64 -riscv-enable-global-merge -global-merge-min-data-size=5 \ +; RUN: llc -mtriple=riscv64 -global-merge-min-data-size=5 \ ; RUN: -riscv-force-enable-global-merge-external-globals -verify-machineinstrs < %s \ ; RUN: | FileCheck %s -check-prefix=MINSIZE diff --git a/llvm/test/CodeGen/RISCV/global-merge-minsize.ll b/llvm/test/CodeGen/RISCV/global-merge-minsize.ll index 696d163bdcb2c..1d65d9d1732ba 100644 --- a/llvm/test/CodeGen/RISCV/global-merge-minsize.ll +++ b/llvm/test/CodeGen/RISCV/global-merge-minsize.ll @@ -1,7 +1,7 @@ ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py -; RUN: llc -mtriple=riscv32 -riscv-enable-global-merge -riscv-force-enable-global-merge-external-globals \ +; RUN: llc -mtriple=riscv32 -riscv-force-enable-global-merge-external-globals \ ; RUN: -verify-machineinstrs < %s | FileCheck %s -check-prefix=RV32 -; RUN: llc -mtriple=riscv32 -riscv-enable-global-merge -global-merge-min-data-size=5 \ +; RUN: llc -mtriple=riscv32 -global-merge-min-data-size=5 \ ; RUN: -riscv-force-enable-global-merge-external-globals -verify-machineinstrs < %s | FileCheck %s -check-prefix=RV32-MINSIZE @ig1 = internal global i32 0, align 4 diff --git a/llvm/test/CodeGen/RISCV/global-merge-offset.ll b/llvm/test/CodeGen/RISCV/global-merge-offset.ll index 0c0881ddf2873..bb8264ee43854 100644 --- a/llvm/test/CodeGen/RISCV/global-merge-offset.ll +++ b/llvm/test/CodeGen/RISCV/global-merge-offset.ll @@ -1,11 +1,11 @@ ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py -; RUN: sed 's/ArrSize/100/g' %s | llc -mtriple=riscv32 -riscv-enable-global-merge \ +; RUN: sed 's/ArrSize/100/g' %s | llc -mtriple=riscv32 \ ; RUN: -riscv-force-enable-global-merge-external-globals -verify-machineinstrs | FileCheck %s -; RUN: sed 's/ArrSize/100/g' %s | llc -mtriple=riscv64 -riscv-enable-global-merge \ +; RUN: sed 's/ArrSize/100/g' %s | llc -mtriple=riscv64 \ ; RUN: -riscv-force-enable-global-merge-external-globals -verify-machineinstrs | FileCheck %s -; RUN: sed 's/ArrSize/101/g' %s | llc -mtriple=riscv32 -riscv-enable-global-merge \ +; RUN: sed 's/ArrSize/101/g' %s | llc -mtriple=riscv32 \ ; RUN: -riscv-force-enable-global-merge-external-globals -verify-machineinstrs | FileCheck %s -check-prefix=CHECK-TOOBIG -; RUN: sed 's/ArrSize/101/g' %s | llc -mtriple=riscv64 -riscv-enable-global-merge \ +; RUN: sed 's/ArrSize/101/g' %s | llc -mtriple=riscv64 \ ; RUN: -riscv-force-enable-global-merge-external-globals -verify-machineinstrs | FileCheck %s -check-prefix=CHECK-TOOBIG ; This test demonstrates that the MaxOffset is set correctly for RISC-V by diff --git a/llvm/test/CodeGen/RISCV/global-merge.ll b/llvm/test/CodeGen/RISCV/global-merge.ll index 633ba719c6a30..8b4043ed476f7 100644 --- a/llvm/test/CodeGen/RISCV/global-merge.ll +++ b/llvm/test/CodeGen/RISCV/global-merge.ll @@ -1,12 +1,10 @@ ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py -; RUN: llc -mtriple=riscv32 -riscv-enable-global-merge -verify-machineinstrs < %s \ -; RUN: | FileCheck %s -; RUN: llc -mtriple=riscv64 -riscv-enable-global-merge -verify-machineinstrs < %s \ -; RUN: | FileCheck %s -; RUN: llc -mtriple=riscv32 -riscv-enable-global-merge \ +; RUN: llc -mtriple=riscv32 -verify-machineinstrs < %s | FileCheck %s +; RUN: llc -mtriple=riscv64 -verify-machineinstrs < %s | FileCheck %s +; RUN: llc -mtriple=riscv32 \ ; RUN: -riscv-force-enable-global-merge-external-globals -verify-machineinstrs < %s \ ; RUN: | FileCheck -check-prefix=CHECK-WEXTERN %s -; RUN: llc -mtriple=riscv64 -riscv-enable-global-merge \ +; RUN: llc -mtriple=riscv64 \ ; RUN: -riscv-force-enable-global-merge-external-globals -verify-machineinstrs < %s \ ; RUN: | FileCheck -check-prefix=CHECK-WEXTERN %s