[llvm] [SimplifyCFG] Don't separate a load/store from its gep during sinking (PR #102318)
Nikita Popov via llvm-commits
llvm-commits at lists.llvm.org
Fri Sep 20 00:43:20 PDT 2024
https://github.com/nikic updated https://github.com/llvm/llvm-project/pull/102318
>From 14b0c712dd0e18e9425b351d030f2f03b30c78d0 Mon Sep 17 00:00:00 2001
From: Nikita Popov <npopov at redhat.com>
Date: Wed, 7 Aug 2024 14:41:25 +0200
Subject: [PATCH 1/2] [SimplifyCFG] Don't separate a load/store from its gep
during sinking
If we can sink the a load/store, but not the gep producing its
pointer operand, don't sink the load/store either. This may
prevent the gep from being folded into an addressing mode, and
may also negatively affect futher analysis.
---
llvm/lib/Transforms/Utils/SimplifyCFG.cpp | 14 +++++
.../SimplifyCFG/X86/sink-common-code.ll | 61 +++++++++++--------
2 files changed, 49 insertions(+), 26 deletions(-)
diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
index 69c4475a494cbe..d2927a516d25a3 100644
--- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -2343,6 +2343,16 @@ static bool sinkCommonCodeFromPredecessors(BasicBlock *BB,
bool followedByDeoptOrUnreachable = IsBlockFollowedByDeoptOrUnreachable(BB);
if (!followedByDeoptOrUnreachable) {
+ // Check whether this is the pointer operand of a load/store.
+ auto IsMemOperand = [](Use &U) {
+ auto *I = cast<Instruction>(U.getUser());
+ if (isa<LoadInst>(I))
+ return U.getOperandNo() == LoadInst::getPointerOperandIndex();
+ if (isa<StoreInst>(I))
+ return U.getOperandNo() == StoreInst::getPointerOperandIndex();
+ return false;
+ };
+
// Okay, we *could* sink last ScanIdx instructions. But how many can we
// actually sink before encountering instruction that is unprofitable to
// sink?
@@ -2354,6 +2364,10 @@ static bool sinkCommonCodeFromPredecessors(BasicBlock *BB,
return InstructionsToSink.contains(V);
})) {
++NumPHIInsts;
+ // Do not separate a load/store from the gep producing the address.
+ if (IsMemOperand(U) &&
+ any_of(It->second, [](Value *V) { return isa<GEPOperator>(V); }))
+ return false;
// FIXME: this check is overly optimistic. We may end up not sinking
// said instruction, due to the very same profitability check.
// See @creating_too_many_phis in sink-common-code.ll.
diff --git a/llvm/test/Transforms/SimplifyCFG/X86/sink-common-code.ll b/llvm/test/Transforms/SimplifyCFG/X86/sink-common-code.ll
index 39b1bec164b9e5..170f8d1592c2fe 100644
--- a/llvm/test/Transforms/SimplifyCFG/X86/sink-common-code.ll
+++ b/llvm/test/Transforms/SimplifyCFG/X86/sink-common-code.ll
@@ -304,14 +304,14 @@ define i32 @test10(i1 zeroext %flag, i32 %x, ptr %y, ptr %s) {
; CHECK-NEXT: br i1 [[FLAG:%.*]], label [[IF_THEN:%.*]], label [[IF_ELSE:%.*]]
; CHECK: if.then:
; CHECK-NEXT: call void @bar(i32 5)
+; CHECK-NEXT: store volatile i32 [[X:%.*]], ptr [[S:%.*]], align 4
; CHECK-NEXT: br label [[IF_END:%.*]]
; CHECK: if.else:
; CHECK-NEXT: call void @bar(i32 6)
-; CHECK-NEXT: [[GEPB:%.*]] = getelementptr inbounds [[STRUCT_ANON:%.*]], ptr [[S:%.*]], i32 0, i32 1
+; CHECK-NEXT: [[GEPB:%.*]] = getelementptr inbounds [[STRUCT_ANON:%.*]], ptr [[S]], i32 0, i32 1
+; CHECK-NEXT: store volatile i32 [[X]], ptr [[GEPB]], align 4
; CHECK-NEXT: br label [[IF_END]]
; CHECK: if.end:
-; CHECK-NEXT: [[GEPB_SINK:%.*]] = phi ptr [ [[GEPB]], [[IF_ELSE]] ], [ [[S]], [[IF_THEN]] ]
-; CHECK-NEXT: store volatile i32 [[X:%.*]], ptr [[GEPB_SINK]], align 4
; CHECK-NEXT: ret i32 1
;
entry:
@@ -518,23 +518,25 @@ declare void @llvm.dbg.value(metadata, metadata, metadata)
!11 = !DILocation(line: 1, column: 14, scope: !8)
-; The load should be commoned.
+; The load should not be commoned, as it will get separated from the GEP
+; instruction producing the address.
define i32 @test15(i1 zeroext %flag, i32 %w, i32 %x, i32 %y, ptr %s) {
; CHECK-LABEL: @test15(
; CHECK-NEXT: entry:
; CHECK-NEXT: br i1 [[FLAG:%.*]], label [[IF_THEN:%.*]], label [[IF_ELSE:%.*]]
; CHECK: if.then:
; CHECK-NEXT: call void @bar(i32 1)
+; CHECK-NEXT: [[SV1:%.*]] = load i32, ptr [[S:%.*]], align 4
; CHECK-NEXT: br label [[IF_END:%.*]]
; CHECK: if.else:
; CHECK-NEXT: call void @bar(i32 4)
-; CHECK-NEXT: [[GEPB:%.*]] = getelementptr inbounds [[STRUCT_ANON:%.*]], ptr [[S:%.*]], i32 0, i32 1
+; CHECK-NEXT: [[GEPB:%.*]] = getelementptr inbounds [[STRUCT_ANON:%.*]], ptr [[S]], i32 0, i32 1
+; CHECK-NEXT: [[SV2:%.*]] = load i32, ptr [[GEPB]], align 4
; CHECK-NEXT: br label [[IF_END]]
; CHECK: if.end:
-; CHECK-NEXT: [[GEPB_SINK:%.*]] = phi ptr [ [[GEPB]], [[IF_ELSE]] ], [ [[S]], [[IF_THEN]] ]
+; CHECK-NEXT: [[SV2_SINK:%.*]] = phi i32 [ [[SV2]], [[IF_ELSE]] ], [ [[SV1]], [[IF_THEN]] ]
; CHECK-NEXT: [[DOTSINK:%.*]] = phi i64 [ 57, [[IF_ELSE]] ], [ 56, [[IF_THEN]] ]
-; CHECK-NEXT: [[SV2:%.*]] = load i32, ptr [[GEPB_SINK]], align 4
-; CHECK-NEXT: [[EXT2:%.*]] = zext i32 [[SV2]] to i64
+; CHECK-NEXT: [[EXT2:%.*]] = zext i32 [[SV2_SINK]] to i64
; CHECK-NEXT: [[CMP2:%.*]] = icmp eq i64 [[EXT2]], [[DOTSINK]]
; CHECK-NEXT: ret i32 1
;
@@ -1803,17 +1805,19 @@ define i64 @multi_use_in_block_inconsistent(i1 %cond, ptr %p, i64 %a, i64 %b) {
; CHECK: if:
; CHECK-NEXT: call void @dummy()
; CHECK-NEXT: [[GEP1_A:%.*]] = getelementptr i8, ptr [[P:%.*]], i64 [[A:%.*]]
+; CHECK-NEXT: [[V_A:%.*]] = load i64, ptr [[GEP1_A]], align 8
+; CHECK-NEXT: [[GEP2_A:%.*]] = getelementptr i8, ptr [[GEP1_A]], i64 [[V_A]]
; CHECK-NEXT: br label [[JOIN:%.*]]
; CHECK: else:
; CHECK-NEXT: [[GEP1_B:%.*]] = getelementptr i8, ptr [[P]], i64 [[A]]
+; CHECK-NEXT: [[V_B:%.*]] = load i64, ptr [[P]], align 8
+; CHECK-NEXT: [[GEP2_B:%.*]] = getelementptr i8, ptr [[GEP1_B]], i64 [[V_B]]
; CHECK-NEXT: br label [[JOIN]]
; CHECK: join:
-; CHECK-NEXT: [[P_SINK:%.*]] = phi ptr [ [[P]], [[ELSE]] ], [ [[GEP1_A]], [[IF]] ]
-; CHECK-NEXT: [[GEP1_B_SINK:%.*]] = phi ptr [ [[GEP1_B]], [[ELSE]] ], [ [[GEP1_A]], [[IF]] ]
-; CHECK-NEXT: [[V_B:%.*]] = load i64, ptr [[P_SINK]], align 8
-; CHECK-NEXT: [[GEP2_B:%.*]] = getelementptr i8, ptr [[GEP1_B_SINK]], i64 [[V_B]]
-; CHECK-NEXT: call void @use.ptr(ptr [[GEP2_B]])
-; CHECK-NEXT: ret i64 [[V_B]]
+; CHECK-NEXT: [[PHI1:%.*]] = phi i64 [ [[V_A]], [[IF]] ], [ [[V_B]], [[ELSE]] ]
+; CHECK-NEXT: [[PHI2:%.*]] = phi ptr [ [[GEP2_A]], [[IF]] ], [ [[GEP2_B]], [[ELSE]] ]
+; CHECK-NEXT: call void @use.ptr(ptr [[PHI2]])
+; CHECK-NEXT: ret i64 [[PHI1]]
;
br i1 %cond, label %if, label %else
@@ -1873,14 +1877,15 @@ define i64 @load_with_non_sunk_gep_both(i1 %cond, ptr %p.a, ptr %p.b, i64 %a, i6
; CHECK: if:
; CHECK-NEXT: call void @dummy()
; CHECK-NEXT: [[GEP_A:%.*]] = getelementptr i8, ptr [[P_A:%.*]], i64 [[A:%.*]]
+; CHECK-NEXT: [[V_A:%.*]] = load i64, ptr [[GEP_A]], align 8
; CHECK-NEXT: br label [[JOIN:%.*]]
; CHECK: else:
; CHECK-NEXT: [[GEP_B:%.*]] = getelementptr i8, ptr [[P_B:%.*]], i64 [[B:%.*]]
+; CHECK-NEXT: [[V_B:%.*]] = load i64, ptr [[GEP_B]], align 8
; CHECK-NEXT: br label [[JOIN]]
; CHECK: join:
-; CHECK-NEXT: [[GEP_B_SINK:%.*]] = phi ptr [ [[GEP_B]], [[ELSE]] ], [ [[GEP_A]], [[IF]] ]
-; CHECK-NEXT: [[V_B:%.*]] = load i64, ptr [[GEP_B_SINK]], align 8
-; CHECK-NEXT: ret i64 [[V_B]]
+; CHECK-NEXT: [[V:%.*]] = phi i64 [ [[V_A]], [[IF]] ], [ [[V_B]], [[ELSE]] ]
+; CHECK-NEXT: ret i64 [[V]]
;
br i1 %cond, label %if, label %else
@@ -1905,14 +1910,15 @@ define i64 @load_with_non_sunk_gep_left(i1 %cond, ptr %p.a, ptr %p.b, i64 %b) {
; CHECK-NEXT: br i1 [[COND:%.*]], label [[IF:%.*]], label [[ELSE:%.*]]
; CHECK: if:
; CHECK-NEXT: call void @dummy()
+; CHECK-NEXT: [[V_A:%.*]] = load i64, ptr [[P_A:%.*]], align 8
; CHECK-NEXT: br label [[JOIN:%.*]]
; CHECK: else:
; CHECK-NEXT: [[GEP_B:%.*]] = getelementptr i8, ptr [[P_B:%.*]], i64 [[B:%.*]]
+; CHECK-NEXT: [[V_B:%.*]] = load i64, ptr [[GEP_B]], align 8
; CHECK-NEXT: br label [[JOIN]]
; CHECK: join:
-; CHECK-NEXT: [[GEP_B_SINK:%.*]] = phi ptr [ [[GEP_B]], [[ELSE]] ], [ [[P_A:%.*]], [[IF]] ]
-; CHECK-NEXT: [[V_B:%.*]] = load i64, ptr [[GEP_B_SINK]], align 8
-; CHECK-NEXT: ret i64 [[V_B]]
+; CHECK-NEXT: [[V:%.*]] = phi i64 [ [[V_A]], [[IF]] ], [ [[V_B]], [[ELSE]] ]
+; CHECK-NEXT: ret i64 [[V]]
;
br i1 %cond, label %if, label %else
@@ -1933,15 +1939,18 @@ join:
define i64 @load_with_non_sunk_gep_right(i1 %cond, ptr %p.a, ptr %p.b, i64 %a) {
; CHECK-LABEL: @load_with_non_sunk_gep_right(
-; CHECK-NEXT: br i1 [[COND:%.*]], label [[IF:%.*]], label [[JOIN:%.*]]
+; CHECK-NEXT: br i1 [[COND:%.*]], label [[IF:%.*]], label [[ELSE:%.*]]
; CHECK: if:
; CHECK-NEXT: call void @dummy()
; CHECK-NEXT: [[GEP_A:%.*]] = getelementptr i8, ptr [[P_A:%.*]], i64 [[A:%.*]]
+; CHECK-NEXT: [[V_A:%.*]] = load i64, ptr [[GEP_A]], align 8
+; CHECK-NEXT: br label [[JOIN:%.*]]
+; CHECK: else:
+; CHECK-NEXT: [[V_B:%.*]] = load i64, ptr [[P_B:%.*]], align 8
; CHECK-NEXT: br label [[JOIN]]
; CHECK: join:
-; CHECK-NEXT: [[P_B_SINK:%.*]] = phi ptr [ [[GEP_A]], [[IF]] ], [ [[P_B:%.*]], [[TMP0:%.*]] ]
-; CHECK-NEXT: [[V_B:%.*]] = load i64, ptr [[P_B_SINK]], align 8
-; CHECK-NEXT: ret i64 [[V_B]]
+; CHECK-NEXT: [[V:%.*]] = phi i64 [ [[V_A]], [[IF]] ], [ [[V_B]], [[ELSE]] ]
+; CHECK-NEXT: ret i64 [[V]]
;
br i1 %cond, label %if, label %else
@@ -1966,13 +1975,13 @@ define void @store_with_non_sunk_gep(i1 %cond, ptr %p.a, ptr %p.b, i64 %a, i64 %
; CHECK: if:
; CHECK-NEXT: call void @dummy()
; CHECK-NEXT: [[GEP_A:%.*]] = getelementptr i8, ptr [[P_A:%.*]], i64 [[A:%.*]]
+; CHECK-NEXT: store i64 0, ptr [[GEP_A]], align 8
; CHECK-NEXT: br label [[JOIN:%.*]]
; CHECK: else:
; CHECK-NEXT: [[GEP_B:%.*]] = getelementptr i8, ptr [[P_B:%.*]], i64 [[B:%.*]]
+; CHECK-NEXT: store i64 0, ptr [[GEP_B]], align 8
; CHECK-NEXT: br label [[JOIN]]
; CHECK: join:
-; CHECK-NEXT: [[GEP_B_SINK:%.*]] = phi ptr [ [[GEP_B]], [[ELSE]] ], [ [[GEP_A]], [[IF]] ]
-; CHECK-NEXT: store i64 0, ptr [[GEP_B_SINK]], align 8
; CHECK-NEXT: ret void
;
br i1 %cond, label %if, label %else
>From 42e0e28aa57ffb1b11593eebc1537e8ed195ea77 Mon Sep 17 00:00:00 2001
From: Nikita Popov <npopov at redhat.com>
Date: Fri, 20 Sep 2024 09:42:54 +0200
Subject: [PATCH 2/2] Expand comment
---
llvm/lib/Transforms/Utils/SimplifyCFG.cpp | 3 +++
1 file changed, 3 insertions(+)
diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
index d2927a516d25a3..4ea8cc9a72c19d 100644
--- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -2365,6 +2365,9 @@ static bool sinkCommonCodeFromPredecessors(BasicBlock *BB,
})) {
++NumPHIInsts;
// Do not separate a load/store from the gep producing the address.
+ // The gep can likely be folded into the load/store as an addressing
+ // mode. Additionally, a load of a gep is easier to analyze than a
+ // load of a phi.
if (IsMemOperand(U) &&
any_of(It->second, [](Value *V) { return isa<GEPOperator>(V); }))
return false;
More information about the llvm-commits
mailing list