[llvm] [SimplifyCFG] Enable sinking of memory operands with separated geps (PR #172342)
via llvm-commits
llvm-commits at lists.llvm.org
Fri Jan 9 06:32:51 PST 2026
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-llvm-transforms
Author: Jessica Del (OutOfCache)
<details>
<summary>Changes</summary>
Not sure if this is the right approach. Open to other suggestions.
Sinking memory instructions is blocked in some cases by not wanting to separate a memory operations from the corresponding getelementptr since https://github.com/llvm/llvm-project/issues/96838.
We encountered a case where the getelementptr is in the entry block of a switch statement, while the loads are in the case blocks themselves. All of the case blocks are very similar and in theory sinkable, but since the geps are not part of the "sinkable instructions", we can't sink the majority of instructions into the merge block (see test cases).
This PR relaxes the criteria for the sinking, so it allows sinking of memory operands that are already separated from the geps.
However, I'll gladly hear any other suggestions for this situation!
---
Full diff: https://github.com/llvm/llvm-project/pull/172342.diff
2 Files Affected:
- (modified) llvm/lib/Transforms/Utils/SimplifyCFG.cpp (+19-2)
- (modified) llvm/test/Transforms/SimplifyCFG/X86/sink-common-code.ll (+143)
``````````diff
diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
index 2a957737697c3..79f9eaeeb3283 100644
--- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -2492,6 +2492,23 @@ static bool sinkCommonCodeFromPredecessors(BasicBlock *BB,
return false;
};
+ // Check whether any of the sinkable instructions share the same parent
+ auto IsGEPInSameParent = [&LRI](Value *V) {
+ auto *VOp = dyn_cast<GEPOperator>(V);
+ if (!VOp)
+ return false;
+
+ auto *VInst = dyn_cast<Instruction>(VOp);
+ if (!VInst) {
+ return false;
+ }
+
+ return any_of(*LRI, [&VInst](Instruction *I) {
+ return is_contained(VInst->users(), I) &&
+ VInst->getParent() == I->getParent();
+ });
+ };
+
// Okay, we *could* sink last ScanIdx instructions. But how many can we
// actually sink before encountering instruction that is unprofitable to
// sink?
@@ -2507,9 +2524,9 @@ static bool sinkCommonCodeFromPredecessors(BasicBlock *BB,
// 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); }))
+ if (IsMemOperand(U) && any_of(It->second, IsGEPInSameParent)) {
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 6129e3b957e17..664c76b6f862e 100644
--- a/llvm/test/Transforms/SimplifyCFG/X86/sink-common-code.ll
+++ b/llvm/test/Transforms/SimplifyCFG/X86/sink-common-code.ll
@@ -2177,6 +2177,149 @@ join:
ret i32 %phi
}
+define i8 @gep_in_same_parent(i32 %cond, ptr %ptr) {
+; CHECK-LABEL: @gep_in_same_parent(
+; CHECK-NEXT: entry:
+; CHECK-NEXT: [[GEP_0:%.*]] = getelementptr i8, ptr [[PTR:%.*]], i64 0
+; CHECK-NEXT: [[GEP_DEFAULT:%.*]] = getelementptr i8, ptr [[PTR]], i64 32
+; CHECK-NEXT: switch i32 [[COND:%.*]], label [[DEFAULT:%.*]] [
+; CHECK-NEXT: i32 0, label [[CASE0:%.*]]
+; CHECK-NEXT: i32 1, label [[CASE1:%.*]]
+; CHECK-NEXT: i32 2, label [[CASE2:%.*]]
+; CHECK-NEXT: i32 3, label [[CASE3:%.*]]
+; CHECK-NEXT: ]
+; CHECK: case0:
+; CHECK-NEXT: [[LOAD0:%.*]] = load i8, ptr [[GEP_0]], align 1
+; CHECK-NEXT: br label [[SWITCH_MERGE:%.*]]
+; CHECK: case1:
+; CHECK-NEXT: [[GEP_1:%.*]] = getelementptr i8, ptr [[PTR]], i64 8
+; CHECK-NEXT: [[LOAD1:%.*]] = load i8, ptr [[GEP_1]], align 1
+; CHECK-NEXT: br label [[SWITCH_MERGE]]
+; CHECK: case2:
+; CHECK-NEXT: [[GEP_2:%.*]] = getelementptr i8, ptr [[PTR]], i64 16
+; CHECK-NEXT: [[LOAD2:%.*]] = load i8, ptr [[GEP_2]], align 1
+; CHECK-NEXT: br label [[SWITCH_MERGE]]
+; CHECK: case3:
+; CHECK-NEXT: [[GEP_3:%.*]] = getelementptr i8, ptr [[PTR]], i64 24
+; CHECK-NEXT: [[LOAD3:%.*]] = load i8, ptr [[GEP_3]], align 1
+; CHECK-NEXT: br label [[SWITCH_MERGE]]
+; CHECK: default:
+; CHECK-NEXT: [[LOAD_DEFAULT:%.*]] = load i8, ptr [[GEP_DEFAULT]], align 1
+; CHECK-NEXT: br label [[SWITCH_MERGE]]
+; CHECK: switch.merge:
+; CHECK-NEXT: [[LOAD_PHI:%.*]] = phi i8 [ [[LOAD0]], [[CASE0]] ], [ [[LOAD1]], [[CASE1]] ], [ [[LOAD2]], [[CASE2]] ], [ [[LOAD3]], [[CASE3]] ], [ [[LOAD_DEFAULT]], [[DEFAULT]] ]
+; CHECK-NEXT: ret i8 [[LOAD_PHI]]
+;
+entry:
+ %gep.0 = getelementptr i8, ptr %ptr, i64 0
+ %gep.default = getelementptr i8, ptr %ptr, i64 32
+ br label %switch.bb
+
+switch.bb:
+ switch i32 %cond, label %default [
+ i32 0, label %case0
+ i32 1, label %case1
+ i32 2, label %case2
+ i32 3, label %case3
+ ]
+
+case0:
+ %load0 = load i8, ptr %gep.0
+ br label %switch.merge
+
+case1:
+ %gep.1 = getelementptr i8, ptr %ptr, i64 8
+ %load1 = load i8, ptr %gep.1
+ br label %switch.merge
+
+case2:
+ %gep.2 = getelementptr i8, ptr %ptr, i64 16
+ %load2 = load i8, ptr %gep.2
+ br label %switch.merge
+
+case3:
+ %gep.3 = getelementptr i8, ptr %ptr, i64 24
+ %load3 = load i8, ptr %gep.3
+ br label %switch.merge
+
+default:
+ %load.default = load i8, ptr %gep.default
+ br label %switch.merge
+
+switch.merge:
+ %load.phi = phi i8 [ %load0, %case0 ], [ %load1, %case1 ], [ %load2, %case2], [ %load3, %case3 ], [ %load.default, %default ]
+ ret i8 %load.phi
+
+}
+
+define i8 @gep_in_entry(i32 %cond, ptr %ptr) {
+; CHECK-LABEL: @gep_in_entry(
+; CHECK-NEXT: entry:
+; CHECK-NEXT: [[GEP_0:%.*]] = getelementptr i8, ptr [[PTR:%.*]], i64 0
+; CHECK-NEXT: [[GEP_1:%.*]] = getelementptr i8, ptr [[PTR]], i64 8
+; CHECK-NEXT: [[GEP_2:%.*]] = getelementptr i8, ptr [[PTR]], i64 16
+; CHECK-NEXT: [[GEP_3:%.*]] = getelementptr i8, ptr [[PTR]], i64 24
+; CHECK-NEXT: [[GEP_DEFAULT:%.*]] = getelementptr i8, ptr [[PTR]], i64 32
+; CHECK-NEXT: switch i32 [[COND:%.*]], label [[DEFAULT:%.*]] [
+; CHECK-NEXT: i32 0, label [[CASE0:%.*]]
+; CHECK-NEXT: i32 1, label [[CASE1:%.*]]
+; CHECK-NEXT: i32 2, label [[CASE2:%.*]]
+; CHECK-NEXT: i32 3, label [[CASE3:%.*]]
+; CHECK-NEXT: ]
+; CHECK: case1:
+; CHECK-NEXT: br label [[CASE0]]
+; CHECK: case2:
+; CHECK-NEXT: br label [[CASE0]]
+; CHECK: case3:
+; CHECK-NEXT: br label [[CASE0]]
+; CHECK: default:
+; CHECK-NEXT: br label [[CASE0]]
+; CHECK: switch.merge:
+; CHECK-NEXT: [[GEP_DEFAULT_SINK:%.*]] = phi ptr [ [[GEP_DEFAULT]], [[DEFAULT]] ], [ [[GEP_3]], [[CASE3]] ], [ [[GEP_2]], [[CASE2]] ], [ [[GEP_1]], [[CASE1]] ], [ [[GEP_0]], [[ENTRY:%.*]] ]
+; CHECK-NEXT: [[GEP_LOAD:%.*]] = load i8, ptr [[GEP_DEFAULT_SINK]], align 1
+; CHECK-NEXT: ret i8 [[GEP_LOAD]]
+;
+entry:
+ %gep.0 = getelementptr i8, ptr %ptr, i64 0
+ %gep.1 = getelementptr i8, ptr %ptr, i64 8
+ %gep.2 = getelementptr i8, ptr %ptr, i64 16
+ %gep.3 = getelementptr i8, ptr %ptr, i64 24
+ %gep.default = getelementptr i8, ptr %ptr, i64 32
+ br label %switch.bb
+
+switch.bb:
+ switch i32 %cond, label %default [
+ i32 0, label %case0
+ i32 1, label %case1
+ i32 2, label %case2
+ i32 3, label %case3
+ ]
+
+case0:
+ %load0 = load i8, ptr %gep.0
+ br label %switch.merge
+
+case1:
+ %load1 = load i8, ptr %gep.1
+ br label %switch.merge
+
+case2:
+ %load2 = load i8, ptr %gep.2
+ br label %switch.merge
+
+case3:
+ %load3 = load i8, ptr %gep.3
+ br label %switch.merge
+
+default:
+ %load.default = load i8, ptr %gep.default
+ br label %switch.merge
+
+switch.merge:
+ %load.phi = phi i8 [ %load0, %case0 ], [ %load1, %case1 ], [ %load2, %case2], [ %load3, %case3 ], [ %load.default, %default ]
+ ret i8 %load.phi
+}
+
declare void @dummy()
declare void @use.ptr(ptr)
``````````
</details>
https://github.com/llvm/llvm-project/pull/172342
More information about the llvm-commits
mailing list