[llvm] r286976 - Revert "[JumpThreading] Unfold selects that depend on the same condition"

Pablo Barrio via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 15 07:42:23 PST 2016


Author: pabbar01
Date: Tue Nov 15 09:42:23 2016
New Revision: 286976

URL: http://llvm.org/viewvc/llvm-project?rev=286976&view=rev
Log:
Revert "[JumpThreading] Unfold selects that depend on the same condition"

This reverts commit ac54d0066c478a09c7cd28d15d0f9ff8af984afc.

Removed:
    llvm/trunk/test/Transforms/JumpThreading/unfold-selects-same-cond.ll
Modified:
    llvm/trunk/include/llvm/Transforms/Scalar/JumpThreading.h
    llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp

Modified: llvm/trunk/include/llvm/Transforms/Scalar/JumpThreading.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Transforms/Scalar/JumpThreading.h?rev=286976&r1=286975&r2=286976&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Transforms/Scalar/JumpThreading.h (original)
+++ llvm/trunk/include/llvm/Transforms/Scalar/JumpThreading.h Tue Nov 15 09:42:23 2016
@@ -129,8 +129,6 @@ private:
                                     BasicBlock *NewBB, BasicBlock *SuccBB);
   /// Check if the block has profile metadata for its outgoing edges.
   bool doesBlockHaveProfileData(BasicBlock *BB);
-  SelectInst *getSelectFedByPhi(PHINode *PN);
-  void expandSelect(SelectInst *SI);
 };
 
 } // end namespace llvm

Modified: llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp?rev=286976&r1=286975&r2=286976&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp Tue Nov 15 09:42:23 2016
@@ -1963,100 +1963,61 @@ bool JumpThreadingPass::TryToUnfoldSelec
   return false;
 }
 
-/// GetSelectFedByPhi - Look for PHI/Select in the same BB of the form
+/// TryToUnfoldSelectInCurrBB - Look for PHI/Select in the same BB of the form
 /// bb:
 ///   %p = phi [false, %bb1], [true, %bb2], [false, %bb3], [true, %bb4], ...
 ///   %s = select p, trueval, falseval
 ///
-/// And return the select. Unfolding it into a branch structure later enables
+/// And expand the select into a branch structure. This later enables
 /// jump-threading over bb in this pass.
 ///
-/// Using the similar approach of SimplifyCFG::FoldCondBranchOnPHI(), return
-/// select if the associated PHI has at least one constant.
-SelectInst *JumpThreadingPass::getSelectFedByPhi(PHINode *PN) {
-
-  unsigned NumPHIValues = PN->getNumIncomingValues();
-  if (NumPHIValues == 0 || !PN->hasOneUse())
-    return nullptr;
-
-  SelectInst *SI = dyn_cast<SelectInst>(PN->user_back());
-  BasicBlock *BB = PN->getParent();
-  if (!SI || SI->getParent() != BB)
-    return nullptr;
-
-  Value *Cond = SI->getCondition();
-  if (!Cond || Cond != PN || !Cond->getType()->isIntegerTy(1))
-    return nullptr;
-
-  for (unsigned i = 0; i != NumPHIValues; ++i) {
-    if (PN->getIncomingBlock(i) == BB)
-      return nullptr;
-    if (isa<ConstantInt>(PN->getIncomingValue(i)))
-      return SI;
-  }
-
-  return nullptr;
-}
-
-/// ExpandSelect - Expand a select into an if-then-else construct.
-void JumpThreadingPass::expandSelect(SelectInst *SI) {
-
-  BasicBlock *BB = SI->getParent();
-  TerminatorInst *Term =
-      SplitBlockAndInsertIfThen(SI->getCondition(), SI, false);
-  PHINode *NewPN = PHINode::Create(SI->getType(), 2, "", SI);
-  NewPN->addIncoming(SI->getTrueValue(), Term->getParent());
-  NewPN->addIncoming(SI->getFalseValue(), BB);
-  SI->replaceAllUsesWith(NewPN);
-  SI->eraseFromParent();
-}
-
-/// TryToUnfoldSelectInCurrBB - Unfold selects that could be jump-threaded were
-/// they if-then-elses. If the unfolded selects are not jump-threaded, it will
-/// be folded again in the later optimizations.
+/// Using the similar approach of SimplifyCFG::FoldCondBranchOnPHI(), unfold
+/// select if the associated PHI has at least one constant.  If the unfolded
+/// select is not jump-threaded, it will be folded again in the later
+/// optimizations.
 bool JumpThreadingPass::TryToUnfoldSelectInCurrBB(BasicBlock *BB) {
-
   // If threading this would thread across a loop header, don't thread the edge.
   // See the comments above FindLoopHeaders for justifications and caveats.
   if (LoopHeaders.count(BB))
     return false;
 
-  bool Changed = false;
-  for (auto &I : *BB) {
+  // Look for a Phi/Select pair in the same basic block.  The Phi feeds the
+  // condition of the Select and at least one of the incoming values is a
+  // constant.
+  for (BasicBlock::iterator BI = BB->begin();
+       PHINode *PN = dyn_cast<PHINode>(BI); ++BI) {
+    unsigned NumPHIValues = PN->getNumIncomingValues();
+    if (NumPHIValues == 0 || !PN->hasOneUse())
+      continue;
 
-    // Look for a Phi/Select pair in the same basic block.  The Phi feeds the
-    // condition of the Select and at least one of the incoming values is a
-    // constant.
-    PHINode *PN;
-    SelectInst *SI;
-    if ((PN = dyn_cast<PHINode>(&I)) && (SI = getSelectFedByPhi(PN))) {
-      expandSelect(SI);
-      Changed = true;
+    SelectInst *SI = dyn_cast<SelectInst>(PN->user_back());
+    if (!SI || SI->getParent() != BB)
       continue;
-    }
 
-    if (I.getType()->isIntegerTy(1)) {
+    Value *Cond = SI->getCondition();
+    if (!Cond || Cond != PN || !Cond->getType()->isIntegerTy(1))
+      continue;
 
-      SmallVector<SelectInst *, 4> Selects;
+    bool HasConst = false;
+    for (unsigned i = 0; i != NumPHIValues; ++i) {
+      if (PN->getIncomingBlock(i) == BB)
+        return false;
+      if (isa<ConstantInt>(PN->getIncomingValue(i)))
+        HasConst = true;
+    }
 
-      // Look for scalar booleans used in selects as conditions. If there are
-      // several selects that use the same boolean, they are candidates for jump
-      // threading and therefore we should unfold them.
-      for (Value *U : I.users())
-        if (auto *SI = dyn_cast<SelectInst>(U))
-          Selects.push_back(SI);
-      if (Selects.size() <= 1)
-        continue;
-
-      // Remove duplicates
-      std::sort(Selects.begin(), Selects.end());
-      auto NewEnd = std::unique(Selects.begin(), Selects.end());
-
-      Changed = true;
-      for (auto SI = Selects.begin(); SI != NewEnd; ++SI)
-        expandSelect(*SI);
+    if (HasConst) {
+      // Expand the select.
+      TerminatorInst *Term =
+          SplitBlockAndInsertIfThen(SI->getCondition(), SI, false);
+      PHINode *NewPN = PHINode::Create(SI->getType(), 2, "", SI);
+      NewPN->addIncoming(SI->getTrueValue(), Term->getParent());
+      NewPN->addIncoming(SI->getFalseValue(), BB);
+      SI->replaceAllUsesWith(NewPN);
+      SI->eraseFromParent();
+      return true;
     }
   }
-
-  return Changed;
+  
+  return false;
 }

Removed: llvm/trunk/test/Transforms/JumpThreading/unfold-selects-same-cond.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/JumpThreading/unfold-selects-same-cond.ll?rev=286975&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/JumpThreading/unfold-selects-same-cond.ll (original)
+++ llvm/trunk/test/Transforms/JumpThreading/unfold-selects-same-cond.ll (removed)
@@ -1,45 +0,0 @@
-; RUN: opt < %s -jump-threading -instcombine -simplifycfg  -S | FileCheck %s
-
-; The three selects are jump-threaded so that instcombine can optimize, and
-; simplifycfg should turn the result into a single select.
-define i32 @f(i32 %a, i32 %b) {
-; CHECK: select
-; CHECK-NOT: select
-entry:
-  %0 = and i32 %a, 1
-  %1 = and i32 %b, 1
-  %xor = xor i32 %1, %a
-  %shr32 = lshr i32 %a, 1
-  %cmp10 = icmp eq i32 %xor, 1
-  %2 = xor i32 %b, 12345
-  %b.addr.1 = select i1 %cmp10, i32 %2, i32 %b
-  %shr1633 = lshr i32 %b.addr.1, 1
-  %3 = or i32 %shr1633, 54321
-  %b.addr.2 = select i1 %cmp10, i32 %3, i32 %shr1633
-  %shr1634 = lshr i32 %b.addr.2, 2
-  %4 = or i32 %shr1634, 54320
-  %b.addr.3 = select i1 %cmp10, i32 %4, i32 %shr1634
-  ret i32 %b.addr.3
-}
-
-; Case where the condition is not only used as condition but also as the
-; true or false value in at least one of the selects.
-define i1 @g(i32 %a, i32 %b) {
-; CHECK: select
-; CHECK-NOT: select
-entry:
-  %0 = and i32 %a, 1
-  %1 = and i32 %b, 1
-  %xor = xor i32 %1, %a
-  %shr32 = lshr i32 %a, 1
-  %cmp10 = icmp eq i32 %xor, 1
-  %2 = xor i32 %b, 12345
-  %b.addr.1 = select i1 %cmp10, i32 %2, i32 %b
-  %shr1633 = lshr i32 %b.addr.1, 1
-  %3 = or i32 %shr1633, 54321
-  %b.addr.2 = select i1 %cmp10, i32 %3, i32 %shr1633
-  %shr1634 = lshr i32 %b.addr.2, 2
-  %4 = icmp eq i32 %shr1634, 54320
-  %b.addr.3 = select i1 %cmp10, i1 %4, i1 %cmp10
-  ret i1 %b.addr.3
-}




More information about the llvm-commits mailing list