From 8f66fb784291c897a965a9ee4c280e314dc8cee4 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Tue, 18 Mar 2025 15:04:36 +0100 Subject: [PATCH] [GlobalMerge] Fix handling of const options For the NewPM, the merge-const option was assigned to an unused option field. Assign it to the correct one. The merge-const-aggressive option was not supported -- and invalid options were silently ignored. Accept it and error on invalid options. For the LegacyPM, the corresponding cl::opt options were ignored when called via opt rather than llc. --- llvm/include/llvm/CodeGen/GlobalMerge.h | 1 - llvm/lib/CodeGen/GlobalMerge.cpp | 2 ++ llvm/lib/Passes/PassBuilder.cpp | 8 ++++++- llvm/lib/Passes/PassRegistry.def | 7 +++--- llvm/test/Transforms/GlobalMerge/constants.ll | 22 +++++++++++++++++++ 5 files changed, 35 insertions(+), 5 deletions(-) create mode 100644 llvm/test/Transforms/GlobalMerge/constants.ll diff --git a/llvm/include/llvm/CodeGen/GlobalMerge.h b/llvm/include/llvm/CodeGen/GlobalMerge.h index f1fb467fc7757..2220e5cfff5fb 100644 --- a/llvm/include/llvm/CodeGen/GlobalMerge.h +++ b/llvm/include/llvm/CodeGen/GlobalMerge.h @@ -25,7 +25,6 @@ struct GlobalMergeOptions { unsigned MinSize = 0; bool GroupByUse = true; bool IgnoreSingleUse = true; - bool MergeConst = false; /// Whether we should merge global variables that have external linkage. bool MergeExternal = true; /// Whether we should merge constant global variables. diff --git a/llvm/lib/CodeGen/GlobalMerge.cpp b/llvm/lib/CodeGen/GlobalMerge.cpp index 1aedc447935b7..d0918acbe48fa 100644 --- a/llvm/lib/CodeGen/GlobalMerge.cpp +++ b/llvm/lib/CodeGen/GlobalMerge.cpp @@ -198,6 +198,8 @@ class GlobalMerge : public FunctionPass { explicit GlobalMerge() : FunctionPass(ID) { Opt.MaxOffset = GlobalMergeMaxOffset; + Opt.MergeConstantGlobals = EnableGlobalMergeOnConst; + Opt.MergeConstAggressive = GlobalMergeAllConst; initializeGlobalMergePass(*PassRegistry::getPassRegistry()); } diff --git a/llvm/lib/Passes/PassBuilder.cpp b/llvm/lib/Passes/PassBuilder.cpp index 7dfff2479d3cf..27c3335932633 100644 --- a/llvm/lib/Passes/PassBuilder.cpp +++ b/llvm/lib/Passes/PassBuilder.cpp @@ -1313,7 +1313,9 @@ Expected parseGlobalMergeOptions(StringRef Params) { else if (ParamName == "ignore-single-use") Result.IgnoreSingleUse = Enable; else if (ParamName == "merge-const") - Result.MergeConst = Enable; + Result.MergeConstantGlobals = Enable; + else if (ParamName == "merge-const-aggressive") + Result.MergeConstAggressive = Enable; else if (ParamName == "merge-external") Result.MergeExternal = Enable; else if (ParamName.consume_front("max-offset=")) { @@ -1322,6 +1324,10 @@ Expected parseGlobalMergeOptions(StringRef Params) { formatv("invalid GlobalMergePass parameter '{0}' ", ParamName) .str(), inconvertibleErrorCode()); + } else { + return make_error( + formatv("invalid global-merge pass parameter '{0}' ", Params).str(), + inconvertibleErrorCode()); } } return Result; diff --git a/llvm/lib/Passes/PassRegistry.def b/llvm/lib/Passes/PassRegistry.def index 81f2ea52c2e84..60e3d01da5fec 100644 --- a/llvm/lib/Passes/PassRegistry.def +++ b/llvm/lib/Passes/PassRegistry.def @@ -178,9 +178,10 @@ MODULE_PASS_WITH_PARAMS( "global-merge", "GlobalMergePass", [TM = TM](GlobalMergeOptions Opts) { return GlobalMergePass(TM, Opts); }, parseGlobalMergeOptions, - "group-by-use;ignore-single-use;max-offset=N;merge-const;merge-external;" - "no-group-by-use;no-ignore-single-use;no-merge-const;no-merge-external;" - "size-only") + "group-by-use;ignore-single-use;max-offset=N;merge-const;" + "merge-const-aggressive;merge-external;no-group-by-use;" + "no-ignore-single-use;no-merge-const;no-merge-const-aggressive;" + "no-merge-external;size-only") MODULE_PASS_WITH_PARAMS( "embed-bitcode", "EmbedBitcodePass", [](EmbedBitcodeOptions Opts) { return EmbedBitcodePass(Opts); }, diff --git a/llvm/test/Transforms/GlobalMerge/constants.ll b/llvm/test/Transforms/GlobalMerge/constants.ll new file mode 100644 index 0000000000000..d5c30da2533b0 --- /dev/null +++ b/llvm/test/Transforms/GlobalMerge/constants.ll @@ -0,0 +1,22 @@ +; RUN: opt -global-merge -global-merge-max-offset=100 -global-merge-on-const -S < %s | FileCheck %s +; RUN: opt -global-merge -global-merge-max-offset=100 -global-merge-on-const -global-merge-all-const -S < %s | FileCheck %s --check-prefix=AGGRESSIVE +; RUN: opt -passes='global-merge' -S < %s | FileCheck %s +; RUN: opt -passes='global-merge' -S < %s | FileCheck %s --check-prefix=AGGRESSIVE + +; CHECK: @_MergedGlobals = private constant <{ i32, i32 }> <{ i32 1, i32 2 }>, align 4 +; AGGRESSIVE: @_MergedGlobals = private constant <{ i32, i32, i32 }> <{ i32 1, i32 2, i32 3 }>, align 4 + +@a = internal constant i32 1 +@b = internal constant i32 2 +@c = internal constant i32 3 + +define void @use() { + %a = load i32, ptr @a + %b = load i32, ptr @b + ret void +} + +define void @use2() { + %c = load i32, ptr @c + ret void +}