[SimplifyCFG] Hoist out implied conditions from successor #158773

VedantParanjape · 2025-09-16T04:16:38Z

In some cases successor can have conditions that imply the current branching condition. If we instead evaluate these implied condition before the current branching condition. This will result in more fine tuned branching condition. We skip this optimization if the true path branch has side effects.

Fixes #155986

Validity of updated testcases

SimplifyCFG/switch_create.ll -> https://alive2.llvm.org/ce/z/NxmWg5
SimplifyCFG/switch_create-custom-dl.ll -> https://alive2.llvm.org/ce/z/pS3E2i

llvmbot · 2025-09-16T04:17:13Z

@llvm/pr-subscribers-llvm-transforms

Author: Vedant Paranjape (VedantParanjape)

Changes

In some cases successor can have conditions that imply the current branching condition. If we instead evaluate these implied condition before the current branching condition. This will result in more fine tuned branching condition. We skip this optimization if the true path branch has side effects.

Fixes #155986

Full diff: https://github.com/llvm/llvm-project/pull/158773.diff

2 Files Affected:

(modified) llvm/lib/Transforms/Utils/SimplifyCFG.cpp (+44)
(added) llvm/test/Transforms/SimplifyCFG/hoist-implied-condition.ll (+78)

diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
index a1f759dd1df83..06fe27be550cc 100644
--- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -1853,6 +1853,47 @@ static void hoistConditionalLoadsStores(
   }
 }
 
+static bool hoistImplyingConditions(BranchInst *BI, IRBuilder<> &Builder,
+                                    const DataLayout &DL) {
+  if (!isa<ICmpInst>(BI->getCondition()))
+    return false;
+
+  ICmpInst *branchCond = cast<ICmpInst>(BI->getCondition());
+  BasicBlock *truePathBB = BI->getSuccessor(0);
+
+  for (auto &I : *truePathBB)
+    if (I.mayHaveSideEffects())
+      return false;
+
+  for (auto &I : *truePathBB) {
+    if (isa<ICmpInst>(I)) {
+      ICmpInst *impliedICmp = cast<ICmpInst>(&I);
+      if (impliedICmp->getPredicate() == branchCond->getPredicate() &&
+          impliedICmp->getOperand(0) == branchCond->getOperand(0) &&
+          impliedICmp->getOperand(1) == branchCond->getOperand(1)) {
+        // found the same condition, so we can skip processing this.
+        continue;
+      }
+
+      std::optional<bool> Imp = isImpliedCondition(impliedICmp, branchCond, DL);
+      if (Imp == true) {
+        Builder.SetInsertPoint(BI);
+        Value *newBranchCond = Builder.CreateICmp(impliedICmp->getPredicate(),
+                                                  impliedICmp->getOperand(0),
+                                                  impliedICmp->getOperand(1));
+
+        BI->setCondition(newBranchCond);
+        branchCond->eraseFromParent();
+        impliedICmp->replaceAllUsesWith(
+            ConstantInt::getTrue(truePathBB->getContext()));
+        return true;
+      }
+    }
+  }
+
+  return false;
+}
+
 static bool isSafeCheapLoadStore(const Instruction *I,
                                  const TargetTransformInfo &TTI) {
   // Not handle volatile or atomic.
@@ -8121,6 +8162,9 @@ bool SimplifyCFGOpt::simplifyCondBranch(BranchInst *BI, IRBuilder<> &Builder) {
   if (simplifyBranchOnICmpChain(BI, Builder, DL))
     return true;
 
+  if (hoistImplyingConditions(BI, Builder, DL))
+    return requestResimplify();
+
   // If this basic block has dominating predecessor blocks and the dominating
   // blocks' conditions imply BI's condition, we know the direction of BI.
   std::optional<bool> Imp = isImpliedByDomCondition(BI->getCondition(), BI, DL);
diff --git a/llvm/test/Transforms/SimplifyCFG/hoist-implied-condition.ll b/llvm/test/Transforms/SimplifyCFG/hoist-implied-condition.ll
new file mode 100644
index 0000000000000..ad5fe7ca32ca8
--- /dev/null
+++ b/llvm/test/Transforms/SimplifyCFG/hoist-implied-condition.ll
@@ -0,0 +1,78 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 6
+; RUN: opt < %s -passes=simplifycfg -S | FileCheck %s
+
+define i32 @src(ptr %contents.0, i64 %contents.1) {
+; CHECK-LABEL: define i32 @src(
+; CHECK-SAME: ptr [[CONTENTS_0:%.*]], i64 [[CONTENTS_1:%.*]]) {
+; CHECK-NEXT:  [[START:.*:]]
+; CHECK-NEXT:    [[TMP0:%.*]] = icmp eq i64 [[CONTENTS_1]], 16
+; CHECK-NEXT:    br i1 [[TMP0]], label %[[IF:.*]], label %[[EXIT:.*]]
+; CHECK:       [[IF]]:
+; CHECK-NEXT:    [[LOAD:%.*]] = load i64, ptr [[CONTENTS_0]], align 4
+; CHECK-NEXT:    [[CMP2:%.*]] = icmp eq i64 [[LOAD]], 123
+; CHECK-NEXT:    [[CMP3:%.*]] = icmp eq i64 [[CONTENTS_1]], 16
+; CHECK-NEXT:    [[AND:%.*]] = and i1 [[CMP2]], true
+; CHECK-NEXT:    br i1 [[AND]], label %[[COMMON_RET:.*]], label %[[EXIT]]
+; CHECK:       [[COMMON_RET]]:
+; CHECK-NEXT:    [[COMMON_RET_OP:%.*]] = phi i32 [ 0, %[[EXIT]] ], [ 1, %[[IF]] ]
+; CHECK-NEXT:    ret i32 [[COMMON_RET_OP]]
+; CHECK:       [[EXIT]]:
+; CHECK-NEXT:    br label %[[COMMON_RET]]
+;
+start:
+  %cmp1 = icmp ugt i64 %contents.1, 7
+  br i1 %cmp1, label %if, label %exit
+
+if:
+  %load = load i64, ptr %contents.0
+  %cmp2 = icmp eq i64 %load, 123
+  %cmp3 = icmp eq i64 %contents.1, 16
+  %and = and i1 %cmp2, %cmp3
+  br i1 %and, label %if2, label %exit
+
+if2:
+  ret i32 1
+
+exit:
+  ret i32 0
+}
+
+define i32 @src-sideeffects(ptr %contents.0, i64 %contents.1) {
+; CHECK-LABEL: define i32 @src-sideeffects(
+; CHECK-SAME: ptr [[CONTENTS_0:%.*]], i64 [[CONTENTS_1:%.*]]) {
+; CHECK-NEXT:  [[START:.*:]]
+; CHECK-NEXT:    [[CMP1:%.*]] = icmp ugt i64 [[CONTENTS_1]], 7
+; CHECK-NEXT:    br i1 [[CMP1]], label %[[IF:.*]], label %[[EXIT:.*]]
+; CHECK:       [[IF]]:
+; CHECK-NEXT:    [[LOAD:%.*]] = load i64, ptr [[CONTENTS_0]], align 4
+; CHECK-NEXT:    [[CMP2:%.*]] = icmp eq i64 [[LOAD]], 123
+; CHECK-NEXT:    [[CMP3:%.*]] = icmp eq i64 [[CONTENTS_1]], 16
+; CHECK-NEXT:    [[AND:%.*]] = and i1 [[CMP2]], [[CMP3]]
+; CHECK-NEXT:    [[ADD:%.*]] = add i64 [[LOAD]], [[CONTENTS_1]]
+; CHECK-NEXT:    store i64 [[ADD]], ptr [[CONTENTS_0]], align 4
+; CHECK-NEXT:    br i1 [[AND]], label %[[COMMON_RET:.*]], label %[[EXIT]]
+; CHECK:       [[COMMON_RET]]:
+; CHECK-NEXT:    [[COMMON_RET_OP:%.*]] = phi i32 [ 0, %[[EXIT]] ], [ 1, %[[IF]] ]
+; CHECK-NEXT:    ret i32 [[COMMON_RET_OP]]
+; CHECK:       [[EXIT]]:
+; CHECK-NEXT:    br label %[[COMMON_RET]]
+;
+start:
+  %cmp1 = icmp ugt i64 %contents.1, 7
+  br i1 %cmp1, label %if, label %exit
+
+if:
+  %load = load i64, ptr %contents.0
+  %cmp2 = icmp eq i64 %load, 123
+  %cmp3 = icmp eq i64 %contents.1, 16
+  %and = and i1 %cmp2, %cmp3
+  %add = add i64 %load, %contents.1
+  store i64 %add, ptr %contents.0
+  br i1 %and, label %if2, label %exit
+
+if2:
+  ret i32 1
+
+exit:
+  ret i32 0
+}

In some cases successor can have conditions that imply the current branching condition. If we instead evaluate these implied condition before the current branching condition. This will result in more fine tuned branching condition. We skip this optimization if the true path branch has side effects. Fixes llvm#155986 Validity of updated testcases ----------------------------- 1) SimplifyCFG/pr55765.ll -> https://alive2.llvm.org/ce/z/N_LZoE 2) SimplifyCFG/fold-branch-to-common-dest.ll -> https://alive2.llvm.org/ce/z/7-LL0Y 3) LoopVectorize/float-induction.ll -> https://alive2.llvm.org/ce/z/vuMNEJ

nikic

It looks like currently you are just looking for any icmp inside the block and hoisting it. This is not correct: It needs to be connected to the control flow in a specific way. Hoisting the condition means that the branch is executed in fewer cases, and you need to make sure that's fine.

(As a trivial counter example, consider the case where the block just contains a completely unused icmp.)

VedantParanjape · 2025-09-16T09:54:11Z

Formalizing this more, we want to hoist only if the set of possible values in the outer condition is much bigger than set of possible values in the implied condition. So the outer predicate should be relational and the implied predicate should be an equality?

It looks like currently you are just looking for any icmp inside the

block and hoisting it. Should I only check for icmps if they share a common operand?

…

On Tue, Sep 16, 2025, 05:21 Nikita Popov ***@***.***> wrote: ***@***.**** requested changes on this pull request. It looks like currently you are just looking for any icmp inside the block and hoisting it. This is not correct: It needs to be connected to the control flow in a specific way. Hoisting the condition means that the branch is executed in fewer cases, and you need to make sure that's fine. (As a trivial counter example, consider the case where the block just contains a completely unused icmp.) — Reply to this email directly, view it on GitHub <#158773 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AFMU6VCW3KN4JSTYIY5XZOT3S7JDFAVCNFSM6AAAAACGTNLMZ2VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTEMRYGY2TENZYGY> . You are receiving this because you authored the thread.Message ID: ***@***.***>

nikic · 2025-09-16T10:17:44Z

The problem is not with the icmps, it's with the control flow. Note that in the test case from #155986 the icmp in the inner block controls a branch, and note that the "else" labels for both branches go to the same block. You need this kind of control flow for the transform to be correct.

VedantParanjape · 2025-09-16T14:56:31Z

The problem is not with the icmps, it's with the control flow. Note that in the test case from #155986 the icmp in the inner block controls a branch, and note that the "else" labels for both branches go to the same block. You need this kind of control flow for the transform to be correct.

It should work on CFGs like this you mean?

A-> B, C
B -> D, C

nikic · 2025-09-16T15:19:14Z

The problem is not with the icmps, it's with the control flow. Note that in the test case from #155986 the icmp in the inner block controls a branch, and note that the "else" labels for both branches go to the same block. You need this kind of control flow for the transform to be correct.

It should work on CFGs like this you mean?
A-> B, C
B -> D, C

Yes, that looks right.

(Having this order of successors is not required -- there are variants of this transform that would also work with other orders. But maybe for the sake of simplicity we can start by handling only this case.)

VedantParanjape · 2025-09-17T20:41:51Z

@nikic I reimplemented based on the review, the code is implemented in such a way that it would be trivial to extend it to pull out a tree of conditions that imply the base condition. But as of now it just looks at leaf conditions and will only hoist a single condition. Makes it easier to complicated cases in the future.

nikic

(not a full review, just some quick notes)

llvm/lib/Transforms/Utils/SimplifyCFG.cpp

nikic · 2025-09-17T21:04:52Z

llvm/lib/Transforms/Utils/SimplifyCFG.cpp

+  if (Imp != true)
+    return std::nullopt;
+
+  std::optional<bool> LHS = visitConditions(I->getOperand(0), baseCond,


I think this code is assuming that the instruction is an and without actually checking that?

Please add a test with e.g. or.

It doesn't assume that as isImpliedCondition handles and/or etc. This should work with 'or' out of the box

Sorry, it seems I was wrong. I am not sure why it fails to find an implication relation if instead there is an or in the testcase?

I had a wrong understanding of what implication relation means, I have fixed it now and will work on adding more testcases.

llvm/lib/Transforms/Utils/SimplifyCFG.cpp

nikic · 2025-09-17T21:06:03Z

llvm/test/Transforms/SimplifyCFG/hoist-implied-condition.ll

This needs a lot more test cases. Especially negative tests where the transform is invalid (e.g. due to incorrect control flow, incorrect impliciations, etc. Generally there needs to be a test for every possible failing precondition).

VedantParanjape · 2025-09-18T21:20:28Z

@nikic looking at the failing testcases, should I stop this optimization if basic blocks are part of a loop?

llvmbot added the llvm:transforms label Sep 16, 2025

VedantParanjape requested a review from nikic September 16, 2025 04:16

VedantParanjape requested a review from dtcxzyw September 16, 2025 04:19

VedantParanjape added the missed-optimization label Sep 16, 2025

VedantParanjape force-pushed the main branch from 48d1bed to 88fc208 Compare September 16, 2025 05:53

nikic requested changes Sep 16, 2025

View reviewed changes

Reimplement based on feedback

80a57b8

VedantParanjape added 4 commits September 17, 2025 16:58

revert some testcases

cc19af7

nitpick

ed0a2a8

revert LV testcases

c96605b

nit

2c588fd

nikic reviewed Sep 17, 2025

View reviewed changes

VedantParanjape added 4 commits September 17, 2025 18:13

refactor

fb6e142

fixed issue with handling or case

f0ca7b9

fix testcase crashes

4b77812

updated tests

5bbe066

[SimplifyCFG] Hoist out implied conditions from successor #158773

Are you sure you want to change the base?

[SimplifyCFG] Hoist out implied conditions from successor #158773

Uh oh!

Conversation

VedantParanjape commented Sep 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Validity of updated testcases

Uh oh!

llvmbot commented Sep 16, 2025

Uh oh!

nikic left a comment

Choose a reason for hiding this comment

Uh oh!

VedantParanjape commented Sep 16, 2025 via email

Uh oh!

nikic commented Sep 16, 2025

Uh oh!

VedantParanjape commented Sep 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nikic commented Sep 16, 2025

Uh oh!

VedantParanjape commented Sep 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nikic left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nikic Sep 17, 2025

Choose a reason for hiding this comment

Uh oh!

VedantParanjape Sep 17, 2025

Choose a reason for hiding this comment

Uh oh!

VedantParanjape Sep 17, 2025

Choose a reason for hiding this comment

Uh oh!

VedantParanjape Sep 18, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nikic Sep 17, 2025

Choose a reason for hiding this comment

Uh oh!

VedantParanjape Sep 18, 2025

Choose a reason for hiding this comment

Uh oh!

VedantParanjape commented Sep 18, 2025

Uh oh!

Uh oh!

VedantParanjape commented Sep 16, 2025 •

edited

Loading

VedantParanjape commented Sep 16, 2025 •

edited

Loading

VedantParanjape commented Sep 17, 2025 •

edited

Loading