[llvm] 76e4070 - Revert "[GuardUtils] Add asserts about loop varying widenable conditions"

[Anna Thomas via llvm-commits]

Author: Anna Thomas
Date: 2023-04-12T10:58:45-04:00
New Revision: 76e4070843308ea5774929a2ac57aa22d580c674

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

LOG: Revert "[GuardUtils] Add asserts about loop varying widenable conditions"

This reverts commit 5675757f5fc6e27ce01b3b12bdfd04044df53aa3.

Assert maybe too strict. revert and investigate why assert fires.

Added: 
    

Modified: 
    llvm/include/llvm/Transforms/Utils/GuardUtils.h
    llvm/lib/Transforms/Scalar/GuardWidening.cpp
    llvm/lib/Transforms/Scalar/LoopPredication.cpp
    llvm/lib/Transforms/Utils/GuardUtils.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/Transforms/Utils/GuardUtils.h b/llvm/include/llvm/Transforms/Utils/GuardUtils.h
index c4e5db4b6642..7ab5d9ef4f23 100644
--- a/llvm/include/llvm/Transforms/Utils/GuardUtils.h
+++ b/llvm/include/llvm/Transforms/Utils/GuardUtils.h
@@ -17,7 +17,6 @@ namespace llvm {
 class BranchInst;
 class CallInst;
 class Function;
-class LoopInfo;
 class Value;
 
 /// Splits control flow at point of \p Guard, replacing it with explicit branch
@@ -33,14 +32,12 @@ void makeGuardControlFlowExplicit(Function *DeoptIntrinsic, CallInst *Guard,
 /// Given a branch we know is widenable (defined per Analysis/GuardUtils.h),
 /// widen it such that condition 'NewCond' is also known to hold on the taken
 /// path.  Branch remains widenable after transform.
-void widenWidenableBranch(BranchInst *WidenableBR, Value *NewCond,
-                          const LoopInfo &LI);
+void widenWidenableBranch(BranchInst *WidenableBR, Value *NewCond);
 
 /// Given a branch we know is widenable (defined per Analysis/GuardUtils.h),
 /// *set* it's condition such that (only) 'Cond' is known to hold on the taken
 /// path and that the branch remains widenable after transform.
-void setWidenableBranchCond(BranchInst *WidenableBR, Value *Cond,
-                            const LoopInfo &LI);
+void setWidenableBranchCond(BranchInst *WidenableBR, Value *Cond);
 
 } // llvm
 

diff  --git a/llvm/lib/Transforms/Scalar/GuardWidening.cpp b/llvm/lib/Transforms/Scalar/GuardWidening.cpp
index c7c5921b781b..62b40a23e38c 100644
--- a/llvm/lib/Transforms/Scalar/GuardWidening.cpp
+++ b/llvm/lib/Transforms/Scalar/GuardWidening.cpp
@@ -289,7 +289,7 @@ class GuardWideningImpl {
     widenCondCommon(getCondition(ToWiden), NewCondition, InsertPt, Result,
                     InvertCondition);
     if (isGuardAsWidenableBranch(ToWiden)) {
-      setWidenableBranchCond(cast<BranchInst>(ToWiden), Result, LI);
+      setWidenableBranchCond(cast<BranchInst>(ToWiden), Result);
       return;
     }
     setCondition(ToWiden, Result);

diff  --git a/llvm/lib/Transforms/Scalar/LoopPredication.cpp b/llvm/lib/Transforms/Scalar/LoopPredication.cpp
index 78d3991d166e..12852ae5c460 100644
--- a/llvm/lib/Transforms/Scalar/LoopPredication.cpp
+++ b/llvm/lib/Transforms/Scalar/LoopPredication.cpp
@@ -1248,7 +1248,7 @@ bool LoopPredication::predicateLoopExits(Loop *L, SCEVExpander &Rewriter) {
     // context.
     NewCond = B.CreateFreeze(NewCond);
 
-    widenWidenableBranch(WidenableBR, NewCond, *LI);
+    widenWidenableBranch(WidenableBR, NewCond);
 
     Value *OldCond = BI->getCondition();
     BI->setCondition(ConstantInt::get(OldCond->getType(), !ExitIfTrue));

diff  --git a/llvm/lib/Transforms/Utils/GuardUtils.cpp b/llvm/lib/Transforms/Utils/GuardUtils.cpp
index 6f0a66579659..7c310f16d46e 100644
--- a/llvm/lib/Transforms/Utils/GuardUtils.cpp
+++ b/llvm/lib/Transforms/Utils/GuardUtils.cpp
@@ -11,7 +11,6 @@
 
 #include "llvm/Transforms/Utils/GuardUtils.h"
 #include "llvm/Analysis/GuardUtils.h"
-#include "llvm/Analysis/LoopInfo.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/Instructions.h"
@@ -79,18 +78,8 @@ void llvm::makeGuardControlFlowExplicit(Function *DeoptIntrinsic,
   }
 }
 
-#ifndef NDEBUG
-static bool isLoopInvariantWidenableCond(Value *WCCond, const Loop &L) {
-  bool IsLoopInv = L.isLoopInvariant(WCCond);
-  if (IsLoopInv || !isa<Instruction>(WCCond))
-    return IsLoopInv;
 
-  return L.hasLoopInvariantOperands(cast<Instruction>(WCCond));
-}
-#endif
-
-void llvm::widenWidenableBranch(BranchInst *WidenableBR, Value *NewCond,
-                                const LoopInfo &LI) {
+void llvm::widenWidenableBranch(BranchInst *WidenableBR, Value *NewCond) {
   assert(isWidenableBranch(WidenableBR) && "precondition");
 
   // The tempting trivially option is to produce something like this:
@@ -101,13 +90,6 @@ void llvm::widenWidenableBranch(BranchInst *WidenableBR, Value *NewCond,
   Use *C, *WC;
   BasicBlock *IfTrueBB, *IfFalseBB;
   parseWidenableBranch(WidenableBR, C, WC, IfTrueBB, IfFalseBB);
-#ifndef NDEBUG
-  bool IsLoopInvariantWidenableBR = false;
-  auto *L = LI.getLoopFor(WidenableBR->getParent());
-  if (L)
-    IsLoopInvariantWidenableBR =
-        isLoopInvariantWidenableCond(WidenableBR->getCondition(), *L);
-#endif
   if (!C) {
     // br (wc()), ... form
     IRBuilder<> B(WidenableBR);
@@ -120,33 +102,15 @@ void llvm::widenWidenableBranch(BranchInst *WidenableBR, Value *NewCond,
     // Condition is only guaranteed to dominate branch
     WCAnd->moveBefore(WidenableBR);    
   }
-#ifndef NDEBUG
-  if (IsLoopInvariantWidenableBR) {
-    auto *L = LI.getLoopFor(WidenableBR->getParent());
-    // NB! This can also be loop-varying if we missed hoisting the IR out, which
-    // was infact loop-invariant. This would imply a pass-ordering issue and the
-    // assert should be dealt with as such.
-    assert(isLoopInvariantWidenableCond(WidenableBR->getCondition(), *L) &&
-           "Loop invariant widenable condition made loop variant!");
-  }
-#endif
   assert(isWidenableBranch(WidenableBR) && "preserve widenabiliy");
 }
 
-void llvm::setWidenableBranchCond(BranchInst *WidenableBR, Value *NewCond,
-                                  const LoopInfo &LI) {
+void llvm::setWidenableBranchCond(BranchInst *WidenableBR, Value *NewCond) {
   assert(isWidenableBranch(WidenableBR) && "precondition");
 
   Use *C, *WC;
   BasicBlock *IfTrueBB, *IfFalseBB;
   parseWidenableBranch(WidenableBR, C, WC, IfTrueBB, IfFalseBB);
-#ifndef NDEBUG
-  bool IsLoopInvariantWidenableBR = false;
-  auto *L = LI.getLoopFor(WidenableBR->getParent());
-  if (L)
-    IsLoopInvariantWidenableBR =
-        isLoopInvariantWidenableCond(WidenableBR->getCondition(), *L);
-#endif
   if (!C) {
     // br (wc()), ... form
     IRBuilder<> B(WidenableBR);
@@ -158,15 +122,5 @@ void llvm::setWidenableBranchCond(BranchInst *WidenableBR, Value *NewCond,
     WCAnd->moveBefore(WidenableBR);
     C->set(NewCond);
   }
-#ifndef NDEBUG
-  if (IsLoopInvariantWidenableBR) {
-    auto *L = LI.getLoopFor(WidenableBR->getParent());
-    // NB! This can also be loop-varying if we missed hoisting the IR out, which
-    // was infact loop-invariant. This would imply a pass-ordering issue and the
-    // assert should be dealt with as such.
-    assert(isLoopInvariantWidenableCond(WidenableBR->getCondition(), *L) &&
-           "Loop invariant widenable condition made loop variant!");
-  }
-#endif
   assert(isWidenableBranch(WidenableBR) && "preserve widenabiliy");
 }