[llvm] 8293f74 - Further cleanup manipulation of widenable branches [NFC]

Philip Reames via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 21 15:07:36 PST 2019


Author: Philip Reames
Date: 2019-11-21T15:07:30-08:00
New Revision: 8293f7434577e23a07284686f5b54079e22e6a91

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

LOG: Further cleanup manipulation of widenable branches [NFC]

This is a follow on to aaea24802bf5.  In post commit discussion, Artur and I realized we could cleanup the code using Uses; this patch does so.

Added: 
    

Modified: 
    llvm/include/llvm/Analysis/GuardUtils.h
    llvm/lib/Analysis/GuardUtils.cpp
    llvm/lib/Transforms/Utils/GuardUtils.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/Analysis/GuardUtils.h b/llvm/include/llvm/Analysis/GuardUtils.h
index f4492c72ce63..b83211535ec2 100644
--- a/llvm/include/llvm/Analysis/GuardUtils.h
+++ b/llvm/include/llvm/Analysis/GuardUtils.h
@@ -15,6 +15,7 @@
 namespace llvm {
 
 class BasicBlock;
+class Use;
 class User;
 class Value;
 
@@ -43,6 +44,11 @@ bool parseWidenableBranch(const User *U, Value *&Condition,
                           Value *&WidenableCondition, BasicBlock *&IfTrueBB,
                           BasicBlock *&IfFalseBB);
 
+/// Analgous to the above, but return the Uses so that that they can be
+/// modified. Unlike previous version, Condition is optional and may be null.
+bool parseWidenableBranch(User *U, Use *&Cond, Use *&WC, BasicBlock *&IfTrueBB,
+                          BasicBlock *&IfFalseBB);
+  
 } // llvm
 
 #endif // LLVM_ANALYSIS_GUARDUTILS_H

diff  --git a/llvm/lib/Analysis/GuardUtils.cpp b/llvm/lib/Analysis/GuardUtils.cpp
index d4a86e18997e..18141069f6a4 100644
--- a/llvm/lib/Analysis/GuardUtils.cpp
+++ b/llvm/lib/Analysis/GuardUtils.cpp
@@ -44,11 +44,35 @@ bool llvm::isGuardAsWidenableBranch(const User *U) {
 bool llvm::parseWidenableBranch(const User *U, Value *&Condition,
                                 Value *&WidenableCondition,
                                 BasicBlock *&IfTrueBB, BasicBlock *&IfFalseBB) {
-  if (match(U, m_Br(m_Intrinsic<Intrinsic::experimental_widenable_condition>(),
-                    IfTrueBB, IfFalseBB)) &&
-      cast<BranchInst>(U)->getCondition()->hasOneUse()) {
-    WidenableCondition = cast<BranchInst>(U)->getCondition();
-    Condition = ConstantInt::getTrue(IfTrueBB->getContext());
+
+  Use *C, *WC;
+  if (parseWidenableBranch(const_cast<User*>(U), C, WC, IfTrueBB, IfFalseBB)) {
+    if (C) 
+      Condition = C->get();
+    else
+      Condition = ConstantInt::getTrue(IfTrueBB->getContext());
+    WidenableCondition = WC->get();
+    return true;
+  }
+  return false;
+}
+
+bool llvm::parseWidenableBranch(User *U, Use *&C,Use *&WC,
+                                BasicBlock *&IfTrueBB, BasicBlock *&IfFalseBB) {
+
+  auto *BI = dyn_cast<BranchInst>(U);
+  if (!BI || !BI->isConditional())
+    return false;
+  auto *Cond = BI->getCondition();
+  if (!Cond->hasOneUse())
+    return false;
+  
+  IfTrueBB = BI->getSuccessor(0);
+  IfFalseBB = BI->getSuccessor(1);
+  
+  if (match(Cond, m_Intrinsic<Intrinsic::experimental_widenable_condition>())) {
+    WC = &BI->getOperandUse(0);
+    C = nullptr;
     return true;
   }
 
@@ -57,19 +81,23 @@ bool llvm::parseWidenableBranch(const User *U, Value *&Condition,
   // 2) br (i1 (and WC(), B)), label %IfTrue, label %IfFalse
   // We do not check for more generalized and trees as we should canonicalize
   // to the form above in instcombine. (TODO)
-  if (!match(U, m_Br(m_And(m_Value(Condition), m_Value(WidenableCondition)),
-                     IfTrueBB, IfFalseBB)))
+  Value *A, *B;
+  if (!match(Cond, m_And(m_Value(A), m_Value(B))))
     return false;
-  if (!match(WidenableCondition,
-             m_Intrinsic<Intrinsic::experimental_widenable_condition>())) {
-    if (!match(Condition,
-               m_Intrinsic<Intrinsic::experimental_widenable_condition>()))
-      return false;
-    std::swap(Condition, WidenableCondition);
+  auto *And = cast<Instruction>(Cond);
+  
+  if (match(A, m_Intrinsic<Intrinsic::experimental_widenable_condition>()) &&
+      A->hasOneUse()) {
+    WC = &And->getOperandUse(0);
+    C = &And->getOperandUse(1);
+    return true;
   }
-    
-  // For the branch to be (easily) widenable, it must not correlate with other
-  // branches.  Thus, the widenable condition must have a single use.
-  return (WidenableCondition->hasOneUse() &&
-          cast<BranchInst>(U)->getCondition()->hasOneUse());
+
+  if (match(B, m_Intrinsic<Intrinsic::experimental_widenable_condition>()) &&
+      B->hasOneUse()) {
+    WC = &And->getOperandUse(1);
+    C = &And->getOperandUse(0);
+    return true;
+  }
+  return false;
 }

diff  --git a/llvm/lib/Transforms/Utils/GuardUtils.cpp b/llvm/lib/Transforms/Utils/GuardUtils.cpp
index 241bfbf80bdf..4cfc9358499a 100644
--- a/llvm/lib/Transforms/Utils/GuardUtils.cpp
+++ b/llvm/lib/Transforms/Utils/GuardUtils.cpp
@@ -87,23 +87,20 @@ void llvm::widenWidenableBranch(BranchInst *WidenableBR, Value *NewCond) {
   // condition, but that doesn't match the pattern parseWidenableBranch expects
   // so we have to be more sophisticated.
 
-  if (match(WidenableBR->getCondition(),
-            m_Intrinsic<Intrinsic::experimental_widenable_condition>())) {
+  Use *C, *WC;
+  BasicBlock *IfTrueBB, *IfFalseBB;
+  parseWidenableBranch(WidenableBR, C, WC, IfTrueBB, IfFalseBB);
+  if (!C) {
+    // br (wc()), ... form
     IRBuilder<> B(WidenableBR);
-    WidenableBR->setCondition(B.CreateAnd(NewCond,
-                                          WidenableBR->getCondition()));
+    WidenableBR->setCondition(B.CreateAnd(NewCond, WC->get()));
   } else {
+    // br (wc & C), ... form
+    IRBuilder<> B(WidenableBR);
+    C->set(B.CreateAnd(NewCond, C->get()));
     Instruction *WCAnd = cast<Instruction>(WidenableBR->getCondition());
     // Condition is only guaranteed to dominate branch
-    WCAnd->moveBefore(WidenableBR);
-    IRBuilder<> B(WCAnd);
-    const bool Op0IsWC =
-      match(WCAnd->getOperand(0),
-            m_Intrinsic<Intrinsic::experimental_widenable_condition>());
-    const unsigned CondOpIdx = Op0IsWC ? 1 : 0;
-    Value *OldCond = WCAnd->getOperand(CondOpIdx);
-    NewCond = B.CreateAnd(NewCond, OldCond);
-    WCAnd->setOperand(CondOpIdx, NewCond);
+    WCAnd->moveBefore(WidenableBR);    
   }
   assert(isWidenableBranch(WidenableBR) && "preserve widenabiliy");
 }
@@ -111,20 +108,19 @@ void llvm::widenWidenableBranch(BranchInst *WidenableBR, Value *NewCond) {
 void llvm::setWidenableBranchCond(BranchInst *WidenableBR, Value *NewCond) {
   assert(isWidenableBranch(WidenableBR) && "precondition");
 
-  if (match(WidenableBR->getCondition(),
-            m_Intrinsic<Intrinsic::experimental_widenable_condition>())) {
+  Use *C, *WC;
+  BasicBlock *IfTrueBB, *IfFalseBB;
+  parseWidenableBranch(WidenableBR, C, WC, IfTrueBB, IfFalseBB);
+  if (!C) {
+    // br (wc()), ... form
     IRBuilder<> B(WidenableBR);
-    WidenableBR->setCondition(B.CreateAnd(NewCond,
-                                          WidenableBR->getCondition()));
+    WidenableBR->setCondition(B.CreateAnd(NewCond, WC->get()));
   } else {
+    // br (wc & C), ... form
     Instruction *WCAnd = cast<Instruction>(WidenableBR->getCondition());
     // Condition is only guaranteed to dominate branch
     WCAnd->moveBefore(WidenableBR);
-    const bool Op0IsWC =
-      match(WCAnd->getOperand(0),
-            m_Intrinsic<Intrinsic::experimental_widenable_condition>());
-    const unsigned CondOpIdx = Op0IsWC ? 1 : 0;
-    WCAnd->setOperand(CondOpIdx, NewCond);
+    C->set(NewCond);
   }
   assert(isWidenableBranch(WidenableBR) && "preserve widenabiliy");
 }


        


More information about the llvm-commits mailing list