[llvm] bb3763e - Revert "[SimplifyCFG] Allow dropping block that only contains ephemeral values"

Nikita Popov via llvm-commits llvm-commits at lists.llvm.org
Fri Jun 30 12:24:14 PDT 2023


Author: Nikita Popov
Date: 2023-06-30T21:24:05+02:00
New Revision: bb3763e497d94b061aa9d625fc542d16da9cae6d

URL: https://github.com/llvm/llvm-project/commit/bb3763e497d94b061aa9d625fc542d16da9cae6d
DIFF: https://github.com/llvm/llvm-project/commit/bb3763e497d94b061aa9d625fc542d16da9cae6d.diff

LOG: Revert "[SimplifyCFG] Allow dropping block that only contains ephemeral values"

This reverts commit 20f0c68fd83a0147a8ec1722bd2e848180610288.

https://reviews.llvm.org/D153966#4464594 reports an optimization
regression in Rust.

Additionally this change has caused an unexpected 0.3% compile-time
regression.

Added: 
    

Modified: 
    llvm/include/llvm/Transforms/Utils/Local.h
    llvm/lib/Transforms/Utils/Local.cpp
    llvm/lib/Transforms/Utils/SimplifyCFG.cpp
    llvm/test/CodeGen/Thumb2/pr52817.ll
    llvm/test/Transforms/LoopVectorize/ARM/pointer_iv.ll
    llvm/test/Transforms/SimplifyCFG/X86/critedge-assume.ll
    llvm/test/Transforms/SimplifyCFG/assume.ll
    llvm/test/Transforms/SimplifyCFG/gepcost.ll
    llvm/test/Transforms/SimplifyCFG/hoist-common-skip.ll

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/Transforms/Utils/Local.h b/llvm/include/llvm/Transforms/Utils/Local.h
index b2f11cbb84f677..4578af069814d0 100644
--- a/llvm/include/llvm/Transforms/Utils/Local.h
+++ b/llvm/include/llvm/Transforms/Utils/Local.h
@@ -156,11 +156,8 @@ void MergeBasicBlockIntoOnlyPred(BasicBlock *BB, DomTreeUpdater *DTU = nullptr);
 /// other than PHI nodes, potential debug intrinsics and the branch. If
 /// possible, eliminate BB by rewriting all the predecessors to branch to the
 /// successor block and return true. If we can't transform, return false.
-/// If a RemoveInsts set is specified, the block may additionally contain these
-/// instructions, which will be removed as part of the transform.
-bool TryToSimplifyUncondBranchFromEmptyBlock(
-    BasicBlock *BB, DomTreeUpdater *DTU = nullptr,
-    const SmallPtrSetImpl<const Instruction *> *RemoveInsts = nullptr);
+bool TryToSimplifyUncondBranchFromEmptyBlock(BasicBlock *BB,
+                                             DomTreeUpdater *DTU = nullptr);
 
 /// Check for and eliminate duplicate PHI nodes in this block. This doesn't try
 /// to be clever about PHI nodes which 
diff er only in the order of the incoming

diff  --git a/llvm/lib/Transforms/Utils/Local.cpp b/llvm/lib/Transforms/Utils/Local.cpp
index 8618d1e87d7fdd..f153ace5d3fc53 100644
--- a/llvm/lib/Transforms/Utils/Local.cpp
+++ b/llvm/lib/Transforms/Utils/Local.cpp
@@ -1059,9 +1059,8 @@ static void redirectValuesFromPredecessorsToPhi(BasicBlock *BB,
   replaceUndefValuesInPhi(PN, IncomingValues);
 }
 
-bool llvm::TryToSimplifyUncondBranchFromEmptyBlock(
-    BasicBlock *BB, DomTreeUpdater *DTU,
-    const SmallPtrSetImpl<const Instruction *> *RemoveInsts) {
+bool llvm::TryToSimplifyUncondBranchFromEmptyBlock(BasicBlock *BB,
+                                                   DomTreeUpdater *DTU) {
   assert(BB != &BB->getParent()->getEntryBlock() &&
          "TryToSimplifyUncondBranchFromEmptyBlock called on entry block!");
 
@@ -1174,15 +1173,6 @@ bool llvm::TryToSimplifyUncondBranchFromEmptyBlock(
 
   LLVM_DEBUG(dbgs() << "Killing Trivial BB: \n" << *BB);
 
-  if (RemoveInsts) {
-    for (Instruction &I : make_early_inc_range(*BB)) {
-      if (RemoveInsts->contains(&I)) {
-        I.replaceAllUsesWith(PoisonValue::get(I.getType()));
-        I.eraseFromParent();
-      }
-    }
-  }
-
   SmallVector<DominatorTree::UpdateType, 32> Updates;
   if (DTU) {
     // To avoid processing the same predecessor more than once.

diff  --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
index 149eb46e966fac..5d3bc44e93c5e6 100644
--- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -2660,13 +2660,7 @@ class EphemeralValueTracker {
     return false;
   }
 
-  bool empty() const { return EphValues.empty(); }
-
   bool contains(const Instruction *I) const { return EphValues.contains(I); }
-
-  const SmallPtrSetImpl<const Instruction *> &getEphemeralValues() const {
-    return EphValues;
-  }
 };
 } // namespace
 
@@ -6935,22 +6929,6 @@ bool SimplifyCFGOpt::simplifyBranch(BranchInst *Branch, IRBuilder<> &Builder) {
                                    : simplifyCondBranch(Branch, Builder);
 }
 
-static bool isMostlyEmptyBlock(EphemeralValueTracker &EphTracker,
-                               BasicBlock *BB) {
-  for (Instruction &I : reverse(drop_end(BB->instructionsWithoutDebug()))) {
-    // Block can contain phi nodes for this transform.
-    if (isa<PHINode>(I))
-      return true;
-    // Removing EH pads requires additional transforms, even if unused.
-    if (I.isEHPad())
-      return false;
-    if (EphTracker.track(&I))
-      continue;
-    return false;
-  }
-  return true;
-}
-
 bool SimplifyCFGOpt::simplifyUncondBranch(BranchInst *BI,
                                           IRBuilder<> &Builder) {
   BasicBlock *BB = BI->getParent();
@@ -6967,21 +6945,13 @@ bool SimplifyCFGOpt::simplifyUncondBranch(BranchInst *BI,
       Options.NeedCanonicalLoop &&
       (!LoopHeaders.empty() && BB->hasNPredecessorsOrMore(2) &&
        (is_contained(LoopHeaders, BB) || is_contained(LoopHeaders, Succ)));
-  if (!BB->isEntryBlock() && !NeedCanonicalLoop) {
-    EphemeralValueTracker EphTracker;
-    // If there are ephemeral values and we have a single predecessor, let
-    // MergeBlockIntoPredecessor() handle it, as it does not need to drop the
-    // ephemeral values.
-    if (isMostlyEmptyBlock(EphTracker, BB) &&
-        (EphTracker.empty() || !Succ->getSinglePredecessor()) &&
-        TryToSimplifyUncondBranchFromEmptyBlock(
-            BB, DTU, &EphTracker.getEphemeralValues()))
-      return true;
-  }
+  BasicBlock::iterator I = BB->getFirstNonPHIOrDbg(true)->getIterator();
+  if (I->isTerminator() && BB != &BB->getParent()->getEntryBlock() &&
+      !NeedCanonicalLoop && TryToSimplifyUncondBranchFromEmptyBlock(BB, DTU))
+    return true;
 
   // If the only instruction in the block is a seteq/setne comparison against a
   // constant, try to simplify the block.
-  BasicBlock::iterator I = BB->getFirstNonPHIOrDbg(true)->getIterator();
   if (ICmpInst *ICI = dyn_cast<ICmpInst>(I))
     if (ICI->isEquality() && isa<ConstantInt>(ICI->getOperand(1))) {
       for (++I; isa<DbgInfoIntrinsic>(I); ++I)

diff  --git a/llvm/test/CodeGen/Thumb2/pr52817.ll b/llvm/test/CodeGen/Thumb2/pr52817.ll
index 4afe15d49b5b68..87615f0a1f7ef4 100644
--- a/llvm/test/CodeGen/Thumb2/pr52817.ll
+++ b/llvm/test/CodeGen/Thumb2/pr52817.ll
@@ -1,5 +1,5 @@
 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
-; RUN: llc -mtriple=thumbv7-apple-ios9.0.0 -arm-atomic-cfg-tidy=0 -verify-machineinstrs < %s | FileCheck %s
+; RUN: llc -mtriple=thumbv7-apple-ios9.0.0 -verify-machineinstrs < %s | FileCheck %s
 
 ; Make sure machine verification does not fail due to incorrect bundle kill
 ; flags.

diff  --git a/llvm/test/Transforms/LoopVectorize/ARM/pointer_iv.ll b/llvm/test/Transforms/LoopVectorize/ARM/pointer_iv.ll
index 6ce3830b344396..12fb1fa821cd45 100644
--- a/llvm/test/Transforms/LoopVectorize/ARM/pointer_iv.ll
+++ b/llvm/test/Transforms/LoopVectorize/ARM/pointer_iv.ll
@@ -62,8 +62,8 @@ define hidden void @pointer_phi_v4i32_add2(ptr noalias nocapture readonly %A, pt
 ; CHECK-NEXT:    [[TMP2:%.*]] = add nsw <4 x i32> [[STRIDED_VEC]], [[BROADCAST_SPLAT]]
 ; CHECK-NEXT:    store <4 x i32> [[TMP2]], ptr [[NEXT_GEP4]], align 4
 ; CHECK-NEXT:    [[INDEX_NEXT]] = add nuw i32 [[INDEX]], 4
-; CHECK-NEXT:    [[TMP3:%.*]] = icmp eq i32 [[INDEX_NEXT]], 996
-; CHECK-NEXT:    br i1 [[TMP3]], label [[FOR_BODY:%.*]], label [[VECTOR_BODY]], !llvm.loop [[LOOP3:![0-9]+]]
+; CHECK-NEXT:    [[TMP4:%.*]] = icmp eq i32 [[INDEX_NEXT]], 996
+; CHECK-NEXT:    br i1 [[TMP4]], label [[FOR_BODY:%.*]], label [[VECTOR_BODY]], !llvm.loop [[LOOP3:![0-9]+]]
 ; CHECK:       for.body:
 ; CHECK-NEXT:    [[A_ADDR_09:%.*]] = phi ptr [ [[ADD_PTR:%.*]], [[FOR_BODY]] ], [ [[IND_END]], [[VECTOR_BODY]] ]
 ; CHECK-NEXT:    [[I_08:%.*]] = phi i32 [ [[INC:%.*]], [[FOR_BODY]] ], [ 996, [[VECTOR_BODY]] ]
@@ -881,16 +881,20 @@ for.body:
 define hidden void @mult_ptr_iv(ptr noalias nocapture readonly %x, ptr noalias nocapture %z) {
 ; CHECK-LABEL: @mult_ptr_iv(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[SCEVGEP:%.*]] = getelementptr i8, ptr [[Z:%.*]], i32 3000
-; CHECK-NEXT:    [[SCEVGEP1:%.*]] = getelementptr i8, ptr [[X:%.*]], i32 3000
-; CHECK-NEXT:    [[BOUND0:%.*]] = icmp ugt ptr [[SCEVGEP1]], [[Z]]
-; CHECK-NEXT:    [[BOUND1:%.*]] = icmp ugt ptr [[SCEVGEP]], [[X]]
+; CHECK-NEXT:    [[UGLYGEP:%.*]] = getelementptr i8, ptr [[Z:%.*]], i32 3000
+; CHECK-NEXT:    [[UGLYGEP1:%.*]] = getelementptr i8, ptr [[X:%.*]], i32 3000
+; CHECK-NEXT:    [[BOUND0:%.*]] = icmp ugt ptr [[UGLYGEP1]], [[Z]]
+; CHECK-NEXT:    [[BOUND1:%.*]] = icmp ugt ptr [[UGLYGEP]], [[X]]
 ; CHECK-NEXT:    [[FOUND_CONFLICT:%.*]] = and i1 [[BOUND0]], [[BOUND1]]
-; CHECK-NEXT:    br i1 [[FOUND_CONFLICT]], label [[FOR_BODY:%.*]], label [[VECTOR_BODY:%.*]]
+; CHECK-NEXT:    br i1 [[FOUND_CONFLICT]], label [[FOR_BODY:%.*]], label [[VECTOR_PH:%.*]]
+; CHECK:       vector.ph:
+; CHECK-NEXT:    [[IND_END:%.*]] = getelementptr i8, ptr [[X]], i32 3000
+; CHECK-NEXT:    [[IND_END2:%.*]] = getelementptr i8, ptr [[Z]], i32 3000
+; CHECK-NEXT:    br label [[VECTOR_BODY:%.*]]
 ; CHECK:       vector.body:
-; CHECK-NEXT:    [[POINTER_PHI:%.*]] = phi ptr [ [[PTR_IND:%.*]], [[VECTOR_BODY]] ], [ [[X]], [[ENTRY:%.*]] ]
-; CHECK-NEXT:    [[POINTER_PHI5:%.*]] = phi ptr [ [[PTR_IND6:%.*]], [[VECTOR_BODY]] ], [ [[Z]], [[ENTRY]] ]
-; CHECK-NEXT:    [[INDEX:%.*]] = phi i32 [ [[INDEX_NEXT:%.*]], [[VECTOR_BODY]] ], [ 0, [[ENTRY]] ]
+; CHECK-NEXT:    [[POINTER_PHI:%.*]] = phi ptr [ [[X]], [[VECTOR_PH]] ], [ [[PTR_IND:%.*]], [[VECTOR_BODY]] ]
+; CHECK-NEXT:    [[POINTER_PHI5:%.*]] = phi ptr [ [[Z]], [[VECTOR_PH]] ], [ [[PTR_IND6:%.*]], [[VECTOR_BODY]] ]
+; CHECK-NEXT:    [[INDEX:%.*]] = phi i32 [ 0, [[VECTOR_PH]] ], [ [[INDEX_NEXT:%.*]], [[VECTOR_BODY]] ]
 ; CHECK-NEXT:    [[TMP0:%.*]] = getelementptr i8, ptr [[POINTER_PHI]], <4 x i32> <i32 0, i32 3, i32 6, i32 9>
 ; CHECK-NEXT:    [[TMP1:%.*]] = getelementptr i8, ptr [[POINTER_PHI5]], <4 x i32> <i32 0, i32 3, i32 6, i32 9>
 ; CHECK-NEXT:    [[TMP2:%.*]] = getelementptr inbounds i8, <4 x ptr> [[TMP0]], i32 1
@@ -912,7 +916,7 @@ define hidden void @mult_ptr_iv(ptr noalias nocapture readonly %x, ptr noalias n
 ; CHECK-NEXT:    [[TMP9:%.*]] = icmp eq i32 [[INDEX_NEXT]], 1000
 ; CHECK-NEXT:    br i1 [[TMP9]], label [[END:%.*]], label [[VECTOR_BODY]], !llvm.loop [[LOOP33:![0-9]+]]
 ; CHECK:       for.body:
-; CHECK-NEXT:    [[X_ADDR_050:%.*]] = phi ptr [ [[INCDEC_PTR2:%.*]], [[FOR_BODY]] ], [ [[X]], [[ENTRY]] ]
+; CHECK-NEXT:    [[X_ADDR_050:%.*]] = phi ptr [ [[INCDEC_PTR2:%.*]], [[FOR_BODY]] ], [ [[X]], [[ENTRY:%.*]] ]
 ; CHECK-NEXT:    [[Z_ADDR_049:%.*]] = phi ptr [ [[INCDEC_PTR34:%.*]], [[FOR_BODY]] ], [ [[Z]], [[ENTRY]] ]
 ; CHECK-NEXT:    [[I_048:%.*]] = phi i32 [ [[INC:%.*]], [[FOR_BODY]] ], [ 0, [[ENTRY]] ]
 ; CHECK-NEXT:    [[INCDEC_PTR:%.*]] = getelementptr inbounds i8, ptr [[X_ADDR_050]], i32 1

diff  --git a/llvm/test/Transforms/SimplifyCFG/X86/critedge-assume.ll b/llvm/test/Transforms/SimplifyCFG/X86/critedge-assume.ll
index b561f093e314d3..58ca8df8ff6d6e 100644
--- a/llvm/test/Transforms/SimplifyCFG/X86/critedge-assume.ll
+++ b/llvm/test/Transforms/SimplifyCFG/X86/critedge-assume.ll
@@ -45,6 +45,7 @@ _ZN10unique_ptrD2Ev.exit:
   %vtable.i.i = load ptr, ptr %call8, align 8
   %x3 = tail call i1 @llvm.type.test(ptr %vtable.i.i, metadata !"foo")
   ; CHECK: call void @llvm.assume
+  ; CHECK: call void @llvm.assume
   tail call void @llvm.assume(i1 %x3) #5
   br i1 %cmp, label %while.end.loopexit, label %while.body
 

diff  --git a/llvm/test/Transforms/SimplifyCFG/assume.ll b/llvm/test/Transforms/SimplifyCFG/assume.ll
index c85eee8c1b9c43..eaebf376a9d72f 100644
--- a/llvm/test/Transforms/SimplifyCFG/assume.ll
+++ b/llvm/test/Transforms/SimplifyCFG/assume.ll
@@ -133,7 +133,11 @@ join:
 define void @empty_block_with_assume(i1 %c, i32 %x) {
 ; CHECK-LABEL: @empty_block_with_assume(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    br i1 [[C:%.*]], label [[JOIN:%.*]], label [[ELSE:%.*]]
+; CHECK-NEXT:    br i1 [[C:%.*]], label [[IF:%.*]], label [[ELSE:%.*]]
+; CHECK:       if:
+; CHECK-NEXT:    [[CMP:%.*]] = icmp ne i32 [[X:%.*]], 0
+; CHECK-NEXT:    call void @llvm.assume(i1 [[CMP]])
+; CHECK-NEXT:    br label [[JOIN:%.*]]
 ; CHECK:       else:
 ; CHECK-NEXT:    call void @dummy()
 ; CHECK-NEXT:    br label [[JOIN]]

diff  --git a/llvm/test/Transforms/SimplifyCFG/gepcost.ll b/llvm/test/Transforms/SimplifyCFG/gepcost.ll
index 8cb23a461df71a..a4497ae71881e7 100644
--- a/llvm/test/Transforms/SimplifyCFG/gepcost.ll
+++ b/llvm/test/Transforms/SimplifyCFG/gepcost.ll
@@ -9,6 +9,11 @@ target triple = "thumbv7m-none--eabi"
 define void @f(i1 %c) {
 ; CHECK-LABEL: @f(
 ; CHECK-NEXT:  entr:
+; CHECK-NEXT:    br i1 %c, label [[NEXT:%.*]], label [[EXIT:%.*]]
+; CHECK:       next:
+; CHECK-NEXT:    [[PAT:%.*]] = getelementptr [16 x i8], ptr @glob
+; CHECK-NEXT:    br label [[EXIT]]
+; CHECK:       exit:
 ; CHECK-NEXT:    ret void
 ;
 entr:

diff  --git a/llvm/test/Transforms/SimplifyCFG/hoist-common-skip.ll b/llvm/test/Transforms/SimplifyCFG/hoist-common-skip.ll
index 76455c30359d97..5b6eeda7c7847e 100644
--- a/llvm/test/Transforms/SimplifyCFG/hoist-common-skip.ll
+++ b/llvm/test/Transforms/SimplifyCFG/hoist-common-skip.ll
@@ -107,6 +107,14 @@ define void @f3(i1 %c, ptr nocapture noundef %d, ptr nocapture noundef readonly
 ; CHECK-NEXT:    [[ADD_0:%.*]] = add nsw i16 [[TMP0]], 1
 ; CHECK-NEXT:    [[TMP1:%.*]] = load i16, ptr [[M:%.*]], align 2
 ; CHECK-NEXT:    [[U:%.*]] = add i16 [[ADD_0]], [[TMP1]]
+; CHECK-NEXT:    br i1 [[C:%.*]], label [[IF_THEN:%.*]], label [[IF_ELSE:%.*]]
+; CHECK:       if.then:
+; CHECK-NEXT:    call void @no_side_effects0()
+; CHECK-NEXT:    br label [[IF_END:%.*]]
+; CHECK:       if.else:
+; CHECK-NEXT:    call void @no_side_effects1()
+; CHECK-NEXT:    br label [[IF_END]]
+; CHECK:       if.end:
 ; CHECK-NEXT:    store i16 [[U]], ptr [[D:%.*]], align 2
 ; CHECK-NEXT:    ret void
 ;
@@ -229,6 +237,14 @@ define void @f6(i1 %c, ptr nocapture noundef %d, ptr nocapture noundef readonly
 ; CHECK-NEXT:    [[TMP0:%.*]] = load i16, ptr [[B:%.*]], align 2
 ; CHECK-NEXT:    [[DIV_0:%.*]] = sdiv i16 211, [[TMP0]]
 ; CHECK-NEXT:    [[U:%.*]] = add i16 [[DIV_0]], [[TMP0]]
+; CHECK-NEXT:    br i1 [[C:%.*]], label [[IF_THEN:%.*]], label [[IF_ELSE:%.*]]
+; CHECK:       if.then:
+; CHECK-NEXT:    call void @no_side_effects0()
+; CHECK-NEXT:    br label [[IF_END:%.*]]
+; CHECK:       if.else:
+; CHECK-NEXT:    call void @no_side_effects1()
+; CHECK-NEXT:    br label [[IF_END]]
+; CHECK:       if.end:
 ; CHECK-NEXT:    store i16 [[U]], ptr [[D:%.*]], align 2
 ; CHECK-NEXT:    ret void
 ;


        


More information about the llvm-commits mailing list