Skip to content

Commit 8f66fb7

Browse files
committed
[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.
1 parent 1bb8b65 commit 8f66fb7

File tree

5 files changed

+35
-5
lines changed

5 files changed

+35
-5
lines changed

llvm/include/llvm/CodeGen/GlobalMerge.h

-1
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ struct GlobalMergeOptions {
2525
unsigned MinSize = 0;
2626
bool GroupByUse = true;
2727
bool IgnoreSingleUse = true;
28-
bool MergeConst = false;
2928
/// Whether we should merge global variables that have external linkage.
3029
bool MergeExternal = true;
3130
/// Whether we should merge constant global variables.

llvm/lib/CodeGen/GlobalMerge.cpp

+2
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,8 @@ class GlobalMerge : public FunctionPass {
198198

199199
explicit GlobalMerge() : FunctionPass(ID) {
200200
Opt.MaxOffset = GlobalMergeMaxOffset;
201+
Opt.MergeConstantGlobals = EnableGlobalMergeOnConst;
202+
Opt.MergeConstAggressive = GlobalMergeAllConst;
201203
initializeGlobalMergePass(*PassRegistry::getPassRegistry());
202204
}
203205

llvm/lib/Passes/PassBuilder.cpp

+7-1
Original file line numberDiff line numberDiff line change
@@ -1313,7 +1313,9 @@ Expected<GlobalMergeOptions> parseGlobalMergeOptions(StringRef Params) {
13131313
else if (ParamName == "ignore-single-use")
13141314
Result.IgnoreSingleUse = Enable;
13151315
else if (ParamName == "merge-const")
1316-
Result.MergeConst = Enable;
1316+
Result.MergeConstantGlobals = Enable;
1317+
else if (ParamName == "merge-const-aggressive")
1318+
Result.MergeConstAggressive = Enable;
13171319
else if (ParamName == "merge-external")
13181320
Result.MergeExternal = Enable;
13191321
else if (ParamName.consume_front("max-offset=")) {
@@ -1322,6 +1324,10 @@ Expected<GlobalMergeOptions> parseGlobalMergeOptions(StringRef Params) {
13221324
formatv("invalid GlobalMergePass parameter '{0}' ", ParamName)
13231325
.str(),
13241326
inconvertibleErrorCode());
1327+
} else {
1328+
return make_error<StringError>(
1329+
formatv("invalid global-merge pass parameter '{0}' ", Params).str(),
1330+
inconvertibleErrorCode());
13251331
}
13261332
}
13271333
return Result;

llvm/lib/Passes/PassRegistry.def

+4-3
Original file line numberDiff line numberDiff line change
@@ -178,9 +178,10 @@ MODULE_PASS_WITH_PARAMS(
178178
"global-merge", "GlobalMergePass",
179179
[TM = TM](GlobalMergeOptions Opts) { return GlobalMergePass(TM, Opts); },
180180
parseGlobalMergeOptions,
181-
"group-by-use;ignore-single-use;max-offset=N;merge-const;merge-external;"
182-
"no-group-by-use;no-ignore-single-use;no-merge-const;no-merge-external;"
183-
"size-only")
181+
"group-by-use;ignore-single-use;max-offset=N;merge-const;"
182+
"merge-const-aggressive;merge-external;no-group-by-use;"
183+
"no-ignore-single-use;no-merge-const;no-merge-const-aggressive;"
184+
"no-merge-external;size-only")
184185
MODULE_PASS_WITH_PARAMS(
185186
"embed-bitcode", "EmbedBitcodePass",
186187
[](EmbedBitcodeOptions Opts) { return EmbedBitcodePass(Opts); },
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
; RUN: opt -global-merge -global-merge-max-offset=100 -global-merge-on-const -S < %s | FileCheck %s
2+
; RUN: opt -global-merge -global-merge-max-offset=100 -global-merge-on-const -global-merge-all-const -S < %s | FileCheck %s --check-prefix=AGGRESSIVE
3+
; RUN: opt -passes='global-merge<max-offset=100;merge-const>' -S < %s | FileCheck %s
4+
; RUN: opt -passes='global-merge<max-offset=100;merge-const;merge-const-aggressive>' -S < %s | FileCheck %s --check-prefix=AGGRESSIVE
5+
6+
; CHECK: @_MergedGlobals = private constant <{ i32, i32 }> <{ i32 1, i32 2 }>, align 4
7+
; AGGRESSIVE: @_MergedGlobals = private constant <{ i32, i32, i32 }> <{ i32 1, i32 2, i32 3 }>, align 4
8+
9+
@a = internal constant i32 1
10+
@b = internal constant i32 2
11+
@c = internal constant i32 3
12+
13+
define void @use() {
14+
%a = load i32, ptr @a
15+
%b = load i32, ptr @b
16+
ret void
17+
}
18+
19+
define void @use2() {
20+
%c = load i32, ptr @c
21+
ret void
22+
}

0 commit comments

Comments
 (0)