Skip to content

[SimplifyCFG] Fold pairs of entries in multiple-entry phi #73674

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

XChy
Copy link
Member

@XChy XChy commented Nov 28, 2023

Resolves #73380.
In this PR, I've done:

@llvmbot
Copy link
Member

llvmbot commented Nov 28, 2023

@llvm/pr-subscribers-backend-arm

@llvm/pr-subscribers-llvm-transforms

Author: XChy (XChy)

Changes

Resolves #73380.
In this PR, I've done:

  • Add tests for #73380.
  • Refactor logic of FoldTwoEntryPHINode in SimplifyCFG, so that we can easily fold such pattern based on both dominant and merge point of the if-region.
  • Add logic for folding one of entry pair in multiple-entry phi based on both dominant.

Somehow this PR causes some regressions for doing such fold in incorrect order, I don't know where to place it now and it may take some time to find the place to do fold.


Patch is 46.06 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/73674.diff

11 Files Affected:

  • (modified) llvm/include/llvm/Transforms/Utils/BasicBlockUtils.h (+13-1)
  • (modified) llvm/lib/Transforms/Utils/BasicBlockUtils.cpp (+42-1)
  • (modified) llvm/lib/Transforms/Utils/FlattenCFG.cpp (+2-2)
  • (modified) llvm/lib/Transforms/Utils/SimplifyCFG.cpp (+79-41)
  • (modified) llvm/test/Transforms/SimplifyCFG/PhiEliminate2.ll (+18-15)
  • (modified) llvm/test/Transforms/SimplifyCFG/UnreachableEliminate.ll (+4-6)
  • (modified) llvm/test/Transforms/SimplifyCFG/X86/sink-common-code.ll (+13-12)
  • (modified) llvm/test/Transforms/SimplifyCFG/X86/switch_to_lookup_table.ll (+65-74)
  • (added) llvm/test/Transforms/SimplifyCFG/fold-multiple-entry-phi.ll (+212)
  • (modified) llvm/test/Transforms/SimplifyCFG/no-md-sink.ll (+4-3)
  • (modified) llvm/test/Transforms/SimplifyCFG/opaque-ptr.ll (+4-2)
diff --git a/llvm/include/llvm/Transforms/Utils/BasicBlockUtils.h b/llvm/include/llvm/Transforms/Utils/BasicBlockUtils.h
index e6dde450b7df9c8..998acbe9ab75da7 100644
--- a/llvm/include/llvm/Transforms/Utils/BasicBlockUtils.h
+++ b/llvm/include/llvm/Transforms/Utils/BasicBlockUtils.h
@@ -595,9 +595,21 @@ void SplitBlockAndInsertForEachLane(
 ///
 /// This does no checking to see if the true/false blocks have large or unsavory
 /// instructions in them.
-BranchInst *GetIfCondition(BasicBlock *BB, BasicBlock *&IfTrue,
+BranchInst *GetIfConditionFromMergePoint(BasicBlock *BB, BasicBlock *&IfTrue,
                            BasicBlock *&IfFalse);
 
+/// Check whether BB is the dominant point of a if-region.
+/// If so, return the merge point of the if-region. Also, return by references
+/// the block that will be entered from if the condition is true, and the block
+/// that will be entered if the condition is false.
+///
+/// This does no checking to see if the true/false blocks have large or unsavory
+/// instructions in them.
+///
+/// NOTE: we assume that the terminator of DomBB is a conditional branch instruction. 
+BasicBlock *GetIfConditionFromDom(BasicBlock *DomBB, BasicBlock *&IfTrue,
+                                  BasicBlock *&IfFalse);
+
 // Split critical edges where the source of the edge is an indirectbr
 // instruction. This isn't always possible, but we can handle some easy cases.
 // This is useful because MI is unable to split such critical edges,
diff --git a/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp b/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp
index 168998fbee114ab..d43a5f72846f72c 100644
--- a/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp
+++ b/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp
@@ -1727,7 +1727,7 @@ void llvm::SplitBlockAndInsertForEachLane(
   }
 }
 
-BranchInst *llvm::GetIfCondition(BasicBlock *BB, BasicBlock *&IfTrue,
+BranchInst *llvm::GetIfConditionFromMergePoint(BasicBlock *BB, BasicBlock *&IfTrue,
                                  BasicBlock *&IfFalse) {
   PHINode *SomePHI = dyn_cast<PHINode>(BB->begin());
   BasicBlock *Pred1 = nullptr;
@@ -1819,6 +1819,47 @@ BranchInst *llvm::GetIfCondition(BasicBlock *BB, BasicBlock *&IfTrue,
   return BI;
 }
 
+BasicBlock *llvm::GetIfConditionFromDom(BasicBlock *DomBB, BasicBlock *&IfTrue,
+                                        BasicBlock *&IfFalse) {
+  BranchInst* BI = cast<BranchInst>(DomBB->getTerminator());
+  BasicBlock *Succ1 = BI->getSuccessor(0);
+  BasicBlock *Succ2 = BI->getSuccessor(1);
+
+  if (!Succ1->getSinglePredecessor() || !Succ2->getSinglePredecessor() ||
+      Succ1 == Succ2 || Succ1 == DomBB || Succ2 == DomBB)
+    return nullptr;
+
+  // We can only handle branches.  Other control flow will be lowered to
+  // branches if possible anyway.
+  BranchInst *Succ1Br = dyn_cast<BranchInst>(Succ1->getTerminator());
+  BranchInst *Succ2Br = dyn_cast<BranchInst>(Succ2->getTerminator());
+  if (!Succ1Br || !Succ2Br)
+    return nullptr;
+
+  if (Succ1->getSingleSuccessor() == Succ2) {
+    IfTrue = Succ1;
+    IfFalse = DomBB;
+    return Succ2;
+  }
+
+  if (Succ2->getSingleSuccessor() == Succ1) {
+    IfTrue = DomBB;
+    IfFalse = Succ2;
+    return Succ1;
+  }
+
+  auto *CommonDest = Succ1->getSingleSuccessor();
+  if (CommonDest && CommonDest != DomBB &&
+      CommonDest == Succ2->getSingleSuccessor()) {
+    IfTrue = Succ1;
+    IfFalse = Succ2;
+    return CommonDest;
+  }
+
+  // There may be other cases, but we just handle the trivial ones now.
+  return nullptr;
+}
+
 // After creating a control flow hub, the operands of PHINodes in an outgoing
 // block Out no longer match the predecessors of that block. Predecessors of Out
 // that are incoming blocks to the hub are now replaced by just one edge from
diff --git a/llvm/lib/Transforms/Utils/FlattenCFG.cpp b/llvm/lib/Transforms/Utils/FlattenCFG.cpp
index 1925b91c4da7ec1..4480202bae8e7db 100644
--- a/llvm/lib/Transforms/Utils/FlattenCFG.cpp
+++ b/llvm/lib/Transforms/Utils/FlattenCFG.cpp
@@ -408,7 +408,7 @@ bool FlattenCFGOpt::CompareIfRegionBlock(BasicBlock *Block1, BasicBlock *Block2,
 /// approach goes for the opposite case.
 bool FlattenCFGOpt::MergeIfRegion(BasicBlock *BB, IRBuilder<> &Builder) {
   BasicBlock *IfTrue2, *IfFalse2;
-  BranchInst *DomBI2 = GetIfCondition(BB, IfTrue2, IfFalse2);
+  BranchInst *DomBI2 = GetIfConditionFromMergePoint(BB, IfTrue2, IfFalse2);
   if (!DomBI2)
     return false;
   Instruction *CInst2 = dyn_cast<Instruction>(DomBI2->getCondition());
@@ -420,7 +420,7 @@ bool FlattenCFGOpt::MergeIfRegion(BasicBlock *BB, IRBuilder<> &Builder) {
     return false;
 
   BasicBlock *IfTrue1, *IfFalse1;
-  BranchInst *DomBI1 = GetIfCondition(SecondEntryBlock, IfTrue1, IfFalse1);
+  BranchInst *DomBI1 = GetIfConditionFromMergePoint(SecondEntryBlock, IfTrue1, IfFalse1);
   if (!DomBI1)
     return false;
   Instruction *CInst1 = dyn_cast<Instruction>(DomBI1->getCondition());
diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
index c09cf9c2325c405..cc6badab9d19712 100644
--- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -3403,35 +3403,37 @@ static bool FoldCondBranchOnValueKnownInPredecessor(BranchInst *BI,
   return EverChanged;
 }
 
-/// Given a BB that starts with the specified two-entry PHI node,
-/// see if we can eliminate it.
-static bool FoldTwoEntryPHINode(PHINode *PN, const TargetTransformInfo &TTI,
-                                DomTreeUpdater *DTU, const DataLayout &DL) {
-  // Ok, this is a two entry PHI node.  Check to see if this is a simple "if
-  // statement", which has a very simple dominance structure.  Basically, we
-  // are trying to find the condition that is being branched on, which
-  // subsequently causes this merge to happen.  We really want control
-  // dependence information for this check, but simplifycfg can't keep it up
-  // to date, and this catches most of the cases we care about anyway.
-  BasicBlock *BB = PN->getParent();
-
-  BasicBlock *IfTrue, *IfFalse;
-  BranchInst *DomBI = GetIfCondition(BB, IfTrue, IfFalse);
-  if (!DomBI)
+// Fold phis in BB based on if-else information.
+static bool FoldPHIOfIfRegion(BasicBlock *BB, BasicBlock *DomBlock,
+                              BasicBlock *IfTrue, BasicBlock *IfFalse,
+                              const TargetTransformInfo &TTI,
+                              DomTreeUpdater *DTU, const DataLayout &DL) {
+  assert(isa<BranchInst>(DomBlock->getTerminator()));
+
+  if (BB->phis().empty())
     return false;
+
+  PHINode *PN = &*BB->phis().begin();
+
+  BranchInst *DomBI = cast<BranchInst>(DomBlock->getTerminator());
   Value *IfCond = DomBI->getCondition();
   // Don't bother if the branch will be constant folded trivially.
   if (isa<ConstantInt>(IfCond))
     return false;
 
-  BasicBlock *DomBlock = DomBI->getParent();
-  SmallVector<BasicBlock *, 2> IfBlocks;
+  // Track BBs that jumps into phi unconditionally, to handle the cases where
+  // one of {IfTrue, IfFalse} is DomBlock.
+  SmallVector<BasicBlock *, 2> UncondEnterBlocks;
   llvm::copy_if(
-      PN->blocks(), std::back_inserter(IfBlocks), [](BasicBlock *IfBlock) {
+      SmallVector<BasicBlock *, 2>{IfTrue, IfFalse},
+      std::back_inserter(UncondEnterBlocks), [](BasicBlock *IfBlock) {
         return cast<BranchInst>(IfBlock->getTerminator())->isUnconditional();
       });
-  assert((IfBlocks.size() == 1 || IfBlocks.size() == 2) &&
-         "Will have either one or two blocks to speculate.");
+
+  // NOTE: Keep the order of BBs the same as the order in phi.
+  llvm::sort(UncondEnterBlocks, [PN](BasicBlock *B1, BasicBlock *B2) {
+    return PN->getBasicBlockIndex(B1) < PN->getBasicBlockIndex(B2);
+  });
 
   // If the branch is non-unpredictable, see if we either predictably jump to
   // the merge bb (if we have only a single 'then' block), or if we predictably
@@ -3446,7 +3448,7 @@ static bool FoldTwoEntryPHINode(PHINode *PN, const TargetTransformInfo &TTI,
           BranchProbability::getBranchProbability(TWeight, TWeight + FWeight);
       BranchProbability Likely = TTI.getPredictableBranchThreshold();
       BranchProbability BIFalseProb = BITrueProb.getCompl();
-      if (IfBlocks.size() == 1) {
+      if (UncondEnterBlocks.size() == 1) {
         BranchProbability BIBBProb =
             DomBI->getSuccessor(0) == BB ? BITrueProb : BIFalseProb;
         if (BIBBProb >= Likely)
@@ -3492,10 +3494,10 @@ static bool FoldTwoEntryPHINode(PHINode *PN, const TargetTransformInfo &TTI,
       continue;
     }
 
-    if (!dominatesMergePoint(PN->getIncomingValue(0), BB, AggressiveInsts,
-                             Cost, Budget, TTI) ||
-        !dominatesMergePoint(PN->getIncomingValue(1), BB, AggressiveInsts,
-                             Cost, Budget, TTI))
+    if (!dominatesMergePoint(PN->getIncomingValueForBlock(IfTrue), BB,
+                             AggressiveInsts, Cost, Budget, TTI) ||
+        !dominatesMergePoint(PN->getIncomingValueForBlock(IfFalse), BB,
+                             AggressiveInsts, Cost, Budget, TTI))
       return Changed;
   }
 
@@ -3525,18 +3527,21 @@ static bool FoldTwoEntryPHINode(PHINode *PN, const TargetTransformInfo &TTI,
                m_CombineOr(m_Select(m_Value(), m_ImmConstant(), m_Value()),
                            m_Select(m_Value(), m_Value(), m_ImmConstant()))));
   };
+
+  Value *TrueVal = PN->getIncomingValueForBlock(IfTrue);
+  Value *FalseVal = PN->getIncomingValueForBlock(IfFalse);
+
   if (PN->getType()->isIntegerTy(1) &&
-      (IsBinOpOrAnd(PN->getIncomingValue(0)) ||
-       IsBinOpOrAnd(PN->getIncomingValue(1)) || IsBinOpOrAnd(IfCond)) &&
-      !CanHoistNotFromBothValues(PN->getIncomingValue(0),
-                                 PN->getIncomingValue(1)))
+      (IsBinOpOrAnd(TrueVal) || IsBinOpOrAnd(FalseVal) ||
+       IsBinOpOrAnd(IfCond)) &&
+      !CanHoistNotFromBothValues(TrueVal, FalseVal))
     return Changed;
 
   // If all PHI nodes are promotable, check to make sure that all instructions
   // in the predecessor blocks can be promoted as well. If not, we won't be able
   // to get rid of the control flow, so it's not worth promoting to select
   // instructions.
-  for (BasicBlock *IfBlock : IfBlocks)
+  for (BasicBlock *IfBlock : UncondEnterBlocks)
     for (BasicBlock::iterator I = IfBlock->begin(); !I->isTerminator(); ++I)
       if (!AggressiveInsts.count(&*I) && !I->isDebugOrPseudoInst()) {
         // This is not an aggressive instruction that we can promote.
@@ -3546,7 +3551,7 @@ static bool FoldTwoEntryPHINode(PHINode *PN, const TargetTransformInfo &TTI,
       }
 
   // If either of the blocks has it's address taken, we can't do this fold.
-  if (any_of(IfBlocks,
+  if (any_of(UncondEnterBlocks,
              [](BasicBlock *IfBlock) { return IfBlock->hasAddressTaken(); }))
     return Changed;
 
@@ -3559,27 +3564,31 @@ static bool FoldTwoEntryPHINode(PHINode *PN, const TargetTransformInfo &TTI,
 
   // Move all 'aggressive' instructions, which are defined in the
   // conditional parts of the if's up to the dominating block.
-  for (BasicBlock *IfBlock : IfBlocks)
-      hoistAllInstructionsInto(DomBlock, DomBI, IfBlock);
+  for (BasicBlock *IfBlock : UncondEnterBlocks)
+    hoistAllInstructionsInto(DomBlock, DomBI, IfBlock);
 
   IRBuilder<NoFolder> Builder(DomBI);
   // Propagate fast-math-flags from phi nodes to replacement selects.
   IRBuilder<>::FastMathFlagGuard FMFGuard(Builder);
-  while (PHINode *PN = dyn_cast<PHINode>(BB->begin())) {
+  for (PHINode &PN : make_early_inc_range(BB->phis())) {
     if (isa<FPMathOperator>(PN))
-      Builder.setFastMathFlags(PN->getFastMathFlags());
+      Builder.setFastMathFlags(PN.getFastMathFlags());
 
     // Change the PHI node into a select instruction.
-    Value *TrueVal = PN->getIncomingValueForBlock(IfTrue);
-    Value *FalseVal = PN->getIncomingValueForBlock(IfFalse);
+    Value *TrueVal = PN.getIncomingValueForBlock(IfTrue);
+    Value *FalseVal = PN.getIncomingValueForBlock(IfFalse);
 
     Value *Sel = Builder.CreateSelect(IfCond, TrueVal, FalseVal, "", DomBI);
-    PN->replaceAllUsesWith(Sel);
-    Sel->takeName(PN);
-    PN->eraseFromParent();
+    if (PN.getNumIncomingValues() == 2) {
+      PN.replaceAllUsesWith(Sel);
+      Sel->takeName(&PN);
+      PN.eraseFromParent();
+    } else {
+      PN.addIncoming(Sel, DomBlock);
+    }
   }
 
-  // At this point, all IfBlocks are empty, so our if statement
+  // At this point, all UncondEnterBlocks are empty, so our if statement
   // has been flattened.  Change DomBlock to jump directly to our new block to
   // avoid other simplifycfg's kicking in on the diamond.
   Builder.CreateBr(BB);
@@ -3598,6 +3607,25 @@ static bool FoldTwoEntryPHINode(PHINode *PN, const TargetTransformInfo &TTI,
   return true;
 }
 
+/// Given a BB that starts with the specified two-entry PHI node,
+/// see if we can eliminate it.
+static bool FoldTwoEntryPHINode(PHINode *PN, const TargetTransformInfo &TTI,
+                                DomTreeUpdater *DTU, const DataLayout &DL) {
+  // Ok, this is a two entry PHI node.  Check to see if this is a simple "if
+  // statement", which has a very simple dominance structure.  Basically, we
+  // are trying to find the condition that is being branched on, which
+  // subsequently causes this merge to happen.  We really want control
+  // dependence information for this check, but simplifycfg can't keep it up
+  // to date, and this catches most of the cases we care about anyway.
+  BasicBlock *BB = PN->getParent();
+  BasicBlock *IfTrue, *IfFalse;
+  BranchInst *DomBI = GetIfConditionFromMergePoint(BB, IfTrue, IfFalse);
+  if (!DomBI)
+    return false;
+  BasicBlock *DomBlock = DomBI->getParent();
+  return FoldPHIOfIfRegion(BB, DomBlock, IfTrue, IfFalse, TTI, DTU, DL);
+}
+
 static Value *createLogicalOp(IRBuilderBase &Builder,
                               Instruction::BinaryOps Opc, Value *LHS,
                               Value *RHS, const Twine &Name = "") {
@@ -7315,6 +7343,7 @@ bool SimplifyCFGOpt::simplifyCondBranch(BranchInst *BI, IRBuilder<> &Builder) {
       if (HoistCommon && hoistCommonCodeFromSuccessors(
                              BI->getParent(), !Options.HoistCommonInsts))
         return requestResimplify();
+
     } else {
       // If Successor #1 has multiple preds, we may be able to conditionally
       // execute Successor #0 if it branches to Successor #1.
@@ -7355,6 +7384,15 @@ bool SimplifyCFGOpt::simplifyCondBranch(BranchInst *BI, IRBuilder<> &Builder) {
           if (mergeConditionalStores(PBI, BI, DTU, DL, TTI))
             return requestResimplify();
 
+  // Fold the merge-point phi of if-region.
+  BasicBlock *IfTrue;
+  BasicBlock *IfFalse;
+  if (Options.SpeculateBlocks &&
+      !BB->getParent()->hasFnAttribute(Attribute::OptForFuzzing))
+    if (BasicBlock *CommonDest = GetIfConditionFromDom(BB, IfTrue, IfFalse))
+      if (FoldPHIOfIfRegion(CommonDest, BB, IfTrue, IfFalse, TTI, DTU, DL))
+        return requestResimplify();
+
   return false;
 }
 
diff --git a/llvm/test/Transforms/SimplifyCFG/PhiEliminate2.ll b/llvm/test/Transforms/SimplifyCFG/PhiEliminate2.ll
index 3b7cbcf1534ae42..ea2f2f1491c5289 100644
--- a/llvm/test/Transforms/SimplifyCFG/PhiEliminate2.ll
+++ b/llvm/test/Transforms/SimplifyCFG/PhiEliminate2.ll
@@ -1,29 +1,32 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 3
 ; RUN: opt < %s -passes=simplifycfg -simplifycfg-require-and-preserve-domtree=1 -S | FileCheck %s
 
 ; Use a select to make this a single BB.
 ; Also, make sure the profile metadata is propagated to the select (PR26636).
 
 define i32 @FoldTwoEntryPHINode(i1 %C, i32 %V1, i32 %V2, i16 %V3) {
+; CHECK-LABEL: define i32 @FoldTwoEntryPHINode(
+; CHECK-SAME: i1 [[C:%.*]], i32 [[V1:%.*]], i32 [[V2:%.*]], i16 [[V3:%.*]]) {
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[V5:%.*]] = sext i16 [[V3]] to i32
+; CHECK-NEXT:    [[V4:%.*]] = or i32 [[V2]], [[V1]]
+; CHECK-NEXT:    [[V6:%.*]] = select i1 [[C]], i32 [[V4]], i32 [[V5]], !prof [[PROF0:![0-9]+]], !unpredictable !1
+; CHECK-NEXT:    [[TMP0:%.*]] = call i32 @FoldTwoEntryPHINode(i1 false, i32 0, i32 0, i16 0)
+; CHECK-NEXT:    ret i32 [[V1]]
+;
 entry:
-        br i1 %C, label %then, label %else, !prof !0, !unpredictable !1
+  br i1 %C, label %then, label %else, !prof !0, !unpredictable !1
 then:
-        %V4 = or i32 %V2, %V1
-        br label %Cont
+  %V4 = or i32 %V2, %V1
+  br label %Cont
 else:
-        %V5 = sext i16 %V3 to i32
-        br label %Cont
+  %V5 = sext i16 %V3 to i32
+  br label %Cont
 Cont:
-        %V6 = phi i32 [ %V5, %else ], [ %V4, %then ]
-        call i32 @FoldTwoEntryPHINode( i1 false, i32 0, i32 0, i16 0 )
-        ret i32 %V1
+  %V6 = phi i32 [ %V5, %else ], [ %V4, %then ]
+  call i32 @FoldTwoEntryPHINode( i1 false, i32 0, i32 0, i16 0 )
+  ret i32 %V1
 
-; CHECK-LABEL: @FoldTwoEntryPHINode(
-; CHECK-NEXT:  entry:
-; CHECK-NEXT:  %V5 = sext i16 %V3 to i32
-; CHECK-NEXT:  %V4 = or i32 %V2, %V1
-; CHECK-NEXT:  %V6 = select i1 %C, i32 %V4, i32 %V5, !prof !0, !unpredictable !1
-; CHECK-NEXT:  %0 = call i32 @FoldTwoEntryPHINode(i1 false, i32 0, i32 0, i16 0)
-; CHECK-NEXT:  ret i32 %V1
 }
 
 !0 = !{!"branch_weights", i32 3, i32 5}
diff --git a/llvm/test/Transforms/SimplifyCFG/UnreachableEliminate.ll b/llvm/test/Transforms/SimplifyCFG/UnreachableEliminate.ll
index 5ba43c055f9be12..2710f8426eeab96 100644
--- a/llvm/test/Transforms/SimplifyCFG/UnreachableEliminate.ll
+++ b/llvm/test/Transforms/SimplifyCFG/UnreachableEliminate.ll
@@ -64,9 +64,8 @@ T:
 define void @test5(i1 %cond, ptr %ptr) {
 ; CHECK-LABEL: @test5(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[TMP0:%.*]] = xor i1 [[COND:%.*]], true
-; CHECK-NEXT:    call void @llvm.assume(i1 [[TMP0]])
-; CHECK-NEXT:    store i8 2, ptr [[PTR:%.*]], align 8
+; CHECK-NEXT:    [[PTR_2:%.*]] = select i1 [[COND:%.*]], ptr null, ptr [[PTR:%.*]]
+; CHECK-NEXT:    store i8 2, ptr [[PTR_2]], align 8
 ; CHECK-NEXT:    ret void
 ;
 entry:
@@ -93,11 +92,10 @@ declare i1 @llvm.type.test(ptr, metadata) nounwind readnone
 define void @test5_type_test_assume(i1 %cond, ptr %ptr, ptr %vtable) {
 ; CHECK-LABEL: @test5_type_test_assume(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[TMP0:%.*]] = xor i1 [[COND:%.*]], true
-; CHECK-NEXT:    call void @llvm.assume(i1 [[TMP0]])
+; CHECK-NEXT:    [[PTR_2:%.*]] = select i1 [[COND:%.*]], ptr null, ptr [[PTR:%.*]]
 ; CHECK-NEXT:    [[P:%.*]] = call i1 @llvm.type.test(ptr [[VTABLE:%.*]], metadata !"foo")
 ; CHECK-NEXT:    tail call void @llvm.assume(i1 [[P]])
-; CHECK-NEXT:    store i8 2, ptr [[PTR:%.*]], align 8
+; CHECK-NEXT:    store i8 2, ptr [[PTR_2]], align 8
 ; CHECK-NEXT:    ret void
 ;
 entry:
diff --git a/llvm/test/Transforms/SimplifyCFG/X86/sink-common-code.ll b/llvm/test/Transforms/SimplifyCFG/X86/sink-common-code.ll
index 26107965a1a8b6d..b54b55725fd8425 100644
--- a/llvm/test/Transforms/SimplifyCFG/X86/sink-common-code.ll
+++ b/llvm/test/Transforms/SimplifyCFG/X86/sink-common-code.ll
@@ -693,8 +693,8 @@ define zeroext i1 @test17(i32 %flag, i32 %blksA, i32 %blksB, i32 %nblks) {
 ; CHECK-LABEL: @test17(
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    switch i32 [[FLAG:%.*]], label [[IF_END:%.*]] [
-; CHECK-NEXT:    i32 0, label [[IF_THEN:%.*]]
-; CHECK-NEXT:    i32 1, label [[IF_THEN2:%.*]]
+; CHECK-NEXT:      i32 0, label [[IF_THEN:%.*]]
+; CHECK-NEXT:      i32 1, label [[IF_THEN2:%.*]]
 ; CHECK-NEXT:    ]
 ; CHECK:       if.then:
 ; CHECK-NEXT:    [[CMP:%.*]] = icmp uge i32 [[BLKSA:%.*]], [[NBLKS:%.*]]
@@ -744,8 +744,8 @@ define zeroext i1 @test18(i32 %flag, i32 %blksA, i32 %blksB, i32 %nblks) {
 ; CHECK-LABEL: @test18(
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    switch i32 [[FLAG:%.*]], label [[IF_THEN3:%.*]] [
-; CHECK-NEXT:    i32 0, label [[IF_THEN:%.*]]
-; CHECK-NEXT:    i32 1, label [[IF_THEN2:%.*]]
+; CHECK-NEXT:      i32 0, label [[IF_THEN:%.*]]
+; CHECK-NEXT:      i32 1, label [[IF_THEN2:%.*]]
 ; CHECK-NEXT:    ]
 ; CHECK:       if.then:
 ; CHECK-NEXT:    [[CMP:%.*]] = icmp uge i32 [[BLKSA:%.*]], [[NBLKS:%.*]]
@@ -1257,8 +1257,9 @@ merge:
 define i32 @test_insertvalue(i1 zeroext %flag, %TP %P) {
 ; CHECK-LABEL: @test_insertvalue(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[DOT:%.*]] = select i1 [[FLAG:%.*]], i32 0, i32 1
-; CHECK-NEXT:    [[I2:%.*]] = insertvalue [[TP:%.*]] [[P:%.*]], i32 [[DOT]], 0
+; CHECK-NEXT:    [[I1:%.*]] = insertvalue [[TP...
[truncated]

Copy link

github-actions bot commented Nov 28, 2023

✅ With the latest revision this PR passed the C/C++ code formatter.

@XChy
Copy link
Member Author

XChy commented Nov 29, 2023

TODO:
Make other folds on merge-point phi done ahead of this fold on dominant basic block.

@XChy
Copy link
Member Author

XChy commented Nov 29, 2023

Now this patch calls simplifyOnce(MergePoint) early to keep original optimization order.

@XChy
Copy link
Member Author

XChy commented Dec 4, 2023

ping.

@TiborGY
Copy link
Contributor

TiborGY commented Feb 18, 2025

@XChy Do you still intend to get this PR merged?

@XChy
Copy link
Member Author

XChy commented Feb 22, 2025

@XChy Do you still intend to get this PR merged?

@TiborGY I am not working on this PR now. If you are interested in this problem, feel free to open a new PR or replacement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[SimplifyCFG] Missed optimization: Fail to merge part of phi into one select
3 participants