[PATCH] D129525: [GlobalOpt] Drop SRA split limit for struct types.
Florian Hahn via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Jul 13 11:52:10 PDT 2022
fhahn updated this revision to Diff 444366.
fhahn added a comment.
In D129525#3644388 <https://reviews.llvm.org/D129525#3644388>, @nikic wrote:
> This will also bypass the check if you have an array that is nested inside a struct. Besides, I don't think we should be making decisions based on the global value type here, which is a another vacuous concept that shouldn't exist (e.g. in rust the global type will often be something like `[N x i8]`, in "bag of bytes" representation).
Right , I think this should be similar to the previous cut-off for struct with arrays inside, e.g.: https://llvm.godbolt.org/z/crje3aYzW . I added additional tests with a struct containing an array and a plain array.
I agree that the type shouldn't really matter. But I think changing the way the cut-off is handled should be done independently of the change to use an offset-based approach.
> Does raising the limit to the next power of two cover your test case?
Unfortunately the struct has stores to 100+ fields in the workload.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D129525/new/
https://reviews.llvm.org/D129525
Files:
llvm/lib/Transforms/IPO/GlobalOpt.cpp
llvm/test/Transforms/GlobalOpt/sra-many-stores.ll
Index: llvm/test/Transforms/GlobalOpt/sra-many-stores.ll
===================================================================
--- llvm/test/Transforms/GlobalOpt/sra-many-stores.ll
+++ llvm/test/Transforms/GlobalOpt/sra-many-stores.ll
@@ -5,15 +5,11 @@
@global = internal global %struct.widget zeroinitializer
-
;.
-; CHECK: @[[GLOBAL:[a-zA-Z0-9_$"\\.-]+]] = internal unnamed_addr global [[STRUCT_WIDGET:%.*]] zeroinitializer
-; CHECK: @[[GLOBAL_ARRAY_IN_STRUCT:[a-zA-Z0-9_$"\\.-]+]] = internal unnamed_addr global [[STRUCT_WITH_ARRAY:%.*]] zeroinitializer
; CHECK: @[[GLOBAL_ARRAY:[a-zA-Z0-9_$"\\.-]+]] = internal unnamed_addr global [100 x i64] zeroinitializer
;.
define internal void @read_struct() {
; CHECK-LABEL: @read_struct(
-; CHECK-NEXT: [[TMP:%.*]] = load ptr, ptr getelementptr inbounds ([[STRUCT_WIDGET:%.*]], ptr @global, i64 0, i32 16), align 8
; CHECK-NEXT: ret void
;
%tmp = load ptr, ptr getelementptr inbounds (%struct.widget, ptr @global, i64 0, i32 16), align 8
@@ -22,22 +18,6 @@
define void @write_struct() {
; CHECK-LABEL: @write_struct(
-; CHECK-NEXT: store ptr null, ptr @global, align 8
-; CHECK-NEXT: store ptr null, ptr getelementptr inbounds ([[STRUCT_WIDGET:%.*]], ptr @global, i64 0, i32 1), align 8
-; CHECK-NEXT: store ptr null, ptr getelementptr inbounds ([[STRUCT_WIDGET]], ptr @global, i64 0, i32 2), align 8
-; CHECK-NEXT: store ptr null, ptr getelementptr inbounds ([[STRUCT_WIDGET]], ptr @global, i64 0, i32 3), align 8
-; CHECK-NEXT: store ptr null, ptr getelementptr inbounds ([[STRUCT_WIDGET]], ptr @global, i64 0, i32 4), align 8
-; CHECK-NEXT: store ptr null, ptr getelementptr inbounds ([[STRUCT_WIDGET]], ptr @global, i64 0, i32 5), align 8
-; CHECK-NEXT: store ptr null, ptr getelementptr inbounds ([[STRUCT_WIDGET]], ptr @global, i64 0, i32 6), align 8
-; CHECK-NEXT: store ptr null, ptr getelementptr inbounds ([[STRUCT_WIDGET]], ptr @global, i64 0, i32 7), align 8
-; CHECK-NEXT: store ptr null, ptr getelementptr inbounds ([[STRUCT_WIDGET]], ptr @global, i64 0, i32 8), align 8
-; CHECK-NEXT: store ptr null, ptr getelementptr inbounds ([[STRUCT_WIDGET]], ptr @global, i64 0, i32 9), align 8
-; CHECK-NEXT: store ptr null, ptr getelementptr inbounds ([[STRUCT_WIDGET]], ptr @global, i64 0, i32 10), align 8
-; CHECK-NEXT: store ptr null, ptr getelementptr inbounds ([[STRUCT_WIDGET]], ptr @global, i64 0, i32 11), align 8
-; CHECK-NEXT: store ptr null, ptr getelementptr inbounds ([[STRUCT_WIDGET]], ptr @global, i64 0, i32 12), align 8
-; CHECK-NEXT: store ptr null, ptr getelementptr inbounds ([[STRUCT_WIDGET]], ptr @global, i64 0, i32 13), align 8
-; CHECK-NEXT: store ptr null, ptr getelementptr inbounds ([[STRUCT_WIDGET]], ptr @global, i64 0, i32 14), align 8
-; CHECK-NEXT: store ptr null, ptr getelementptr inbounds ([[STRUCT_WIDGET]], ptr @global, i64 0, i32 15), align 8
; CHECK-NEXT: tail call fastcc void @read_struct()
; CHECK-NEXT: ret void
;
@@ -68,7 +48,6 @@
define internal void @read_non_array_field() {
; CHECK-LABEL: @read_non_array_field(
-; CHECK-NEXT: [[TMP:%.*]] = load i64, ptr getelementptr inbounds ([[STRUCT_WITH_ARRAY:%.*]], ptr @global.array_in_struct, i64 0, i32 1), align 8
; CHECK-NEXT: ret void
;
%tmp = load i64, ptr getelementptr inbounds (%struct.with.array, ptr @global.array_in_struct, i64 0, i32 1), align 8
@@ -77,22 +56,6 @@
define void @store_to_struct_array() {
; CHECK-LABEL: @store_to_struct_array(
-; CHECK-NEXT: store i64 0, ptr @global.array_in_struct, align 8
-; CHECK-NEXT: store i64 0, ptr getelementptr inbounds ([[STRUCT_WITH_ARRAY:%.*]], ptr @global.array_in_struct, i64 0, i32 0, i32 1), align 8
-; CHECK-NEXT: store i64 0, ptr getelementptr inbounds ([[STRUCT_WITH_ARRAY]], ptr @global.array_in_struct, i64 0, i32 0, i32 2), align 8
-; CHECK-NEXT: store i64 0, ptr getelementptr inbounds ([[STRUCT_WITH_ARRAY]], ptr @global.array_in_struct, i64 0, i32 0, i32 3), align 8
-; CHECK-NEXT: store i64 0, ptr getelementptr inbounds ([[STRUCT_WITH_ARRAY]], ptr @global.array_in_struct, i64 0, i32 0, i32 4), align 8
-; CHECK-NEXT: store i64 0, ptr getelementptr inbounds ([[STRUCT_WITH_ARRAY]], ptr @global.array_in_struct, i64 0, i32 0, i32 5), align 8
-; CHECK-NEXT: store i64 0, ptr getelementptr inbounds ([[STRUCT_WITH_ARRAY]], ptr @global.array_in_struct, i64 0, i32 0, i32 6), align 8
-; CHECK-NEXT: store i64 0, ptr getelementptr inbounds ([[STRUCT_WITH_ARRAY]], ptr @global.array_in_struct, i64 0, i32 0, i32 7), align 8
-; CHECK-NEXT: store i64 0, ptr getelementptr inbounds ([[STRUCT_WITH_ARRAY]], ptr @global.array_in_struct, i64 0, i32 0, i32 8), align 8
-; CHECK-NEXT: store i64 0, ptr getelementptr inbounds ([[STRUCT_WITH_ARRAY]], ptr @global.array_in_struct, i64 0, i32 0, i32 9), align 8
-; CHECK-NEXT: store i64 0, ptr getelementptr inbounds ([[STRUCT_WITH_ARRAY]], ptr @global.array_in_struct, i64 0, i32 0, i32 10), align 8
-; CHECK-NEXT: store i64 0, ptr getelementptr inbounds ([[STRUCT_WITH_ARRAY]], ptr @global.array_in_struct, i64 0, i32 0, i32 11), align 8
-; CHECK-NEXT: store i64 0, ptr getelementptr inbounds ([[STRUCT_WITH_ARRAY]], ptr @global.array_in_struct, i64 0, i32 0, i32 12), align 8
-; CHECK-NEXT: store i64 0, ptr getelementptr inbounds ([[STRUCT_WITH_ARRAY]], ptr @global.array_in_struct, i64 0, i32 0, i32 13), align 8
-; CHECK-NEXT: store i64 0, ptr getelementptr inbounds ([[STRUCT_WITH_ARRAY]], ptr @global.array_in_struct, i64 0, i32 0, i32 14), align 8
-; CHECK-NEXT: store i64 0, ptr getelementptr inbounds ([[STRUCT_WITH_ARRAY]], ptr @global.array_in_struct, i64 0, i32 0, i32 15), align 8
; CHECK-NEXT: tail call fastcc void @read_non_array_field()
; CHECK-NEXT: ret void
;
Index: llvm/lib/Transforms/IPO/GlobalOpt.cpp
===================================================================
--- llvm/lib/Transforms/IPO/GlobalOpt.cpp
+++ llvm/lib/Transforms/IPO/GlobalOpt.cpp
@@ -463,8 +463,9 @@
if (Types.size() == 1 && Types.begin()->second == GV->getValueType())
return nullptr;
+ Constant *OrigInit = GV->getInitializer();
// Don't perform SRA if we would have to split into many globals.
- if (Types.size() > 16)
+ if (!isa<StructType>(OrigInit->getType()) && Types.size() > 16)
return nullptr;
// Sort by offset.
@@ -488,7 +489,6 @@
return nullptr;
// Collect initializers for new globals.
- Constant *OrigInit = GV->getInitializer();
DenseMap<uint64_t, Constant *> Initializers;
for (const auto &Pair : Types) {
Constant *NewInit = ConstantFoldLoadFromConst(OrigInit, Pair.second,
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D129525.444366.patch
Type: text/x-patch
Size: 6641 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220713/efe71bad/attachment.bin>
More information about the llvm-commits
mailing list