[llvm-commits] [llvm] r84799 - in /llvm/trunk: lib/Transforms/Scalar/InstructionCombining.cpp test/Transforms/InstCombine/fold-intophi.ll

Chris Lattner sabre at nondot.org
Wed Oct 21 16:41:58 PDT 2009


Author: lattner
Date: Wed Oct 21 18:41:58 2009
New Revision: 84799

URL: http://llvm.org/viewvc/llvm-project?rev=84799&view=rev
Log:
revert r84754, it isn't the right approach.  Edwin, please propose 
patches for fixes like this instead of committing them directly.


Removed:
    llvm/trunk/test/Transforms/InstCombine/fold-intophi.ll
Modified:
    llvm/trunk/lib/Transforms/Scalar/InstructionCombining.cpp

Modified: llvm/trunk/lib/Transforms/Scalar/InstructionCombining.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/InstructionCombining.cpp?rev=84799&r1=84798&r2=84799&view=diff

==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/InstructionCombining.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/InstructionCombining.cpp Wed Oct 21 18:41:58 2009
@@ -1985,42 +1985,6 @@
   return 0;
 }
 
-// Check whether all the operands of the PHI dominate the PHI node,
-// knowing that the PHI's operands either dominate BB, or are defined in the BB.
-static bool checkPHI(PHINode *PN, BasicBlock *BB)
-{
-  BasicBlock *phiBB = PN->getParent();
-  for (unsigned i=0;i<PN->getNumIncomingValues();i++) {
-    Instruction *I = dyn_cast<Instruction>(PN->getIncomingValue(i));
-    if (!I)
-      continue;
-    BasicBlock *pBB = I->getParent();
-    if (pBB == BB) {
-      // another PHI in same BB is always before PN (PN is last phi).
-      if (isa<PHINode>(I))
-        continue;
-      // An instruction in the same BB, and not a phi, this is after PN.
-      return false;
-    }
-    if (phiBB == BB)
-      continue;
-    // We don't have dominator info, so just check that the instruction
-    // is not defined in on of the BB on the unique path between BB and phiBB.
-    // If there is no such unique path, or pBB equals to one of the BBs on that
-    // path we know that this operand doesn't dominate the PHI node.
-    BasicBlock *B = PN->getIncomingBlock(i);
-    while ((B = B->getUniquePredecessor())) {
-      if (pBB == B)
-        return false;
-      if (B == phiBB)
-        break;
-    }
-    // No unique path -> doesn't dominate
-    if (!B)
-      return false;
-  }
-  return true;
-}
 
 /// FoldOpIntoPhi - Given a binary operator, cast instruction, or select which
 /// has a PHI node as operand #0, see if we can fold the instruction into the
@@ -2072,13 +2036,9 @@
   // Okay, we can do the transformation: create the new PHI node.
   PHINode *NewPN = PHINode::Create(I.getType(), "");
   NewPN->reserveOperandSpace(PN->getNumOperands()/2);
-  // We must the PHI node as the last PHI, because it may use one of the other
-  // PHIs.
-  BasicBlock::iterator BBIt = PN;
-  while (isa<PHINode>(BBIt)) ++BBIt;
-  InsertNewInstBefore(NewPN, *BBIt);
+  InsertNewInstBefore(NewPN, *PN);
+  NewPN->takeName(PN);
 
-  SmallVector<Instruction*, 2> tmpWorklist;
   // Next, add all of the operands to the PHI.
   if (SelectInst *SI = dyn_cast<SelectInst>(&I)) {
     // We only currently try to fold the condition of a select when it is a phi,
@@ -2098,7 +2058,7 @@
         InV = SelectInst::Create(PN->getIncomingValue(i), TrueVInPred,
                                  FalseVInPred,
                                  "phitmp", NonConstBB->getTerminator());
-        tmpWorklist.push_back(cast<Instruction>(InV));
+        Worklist.Add(cast<Instruction>(InV));
       }
       NewPN->addIncoming(InV, ThisBB);
     }
@@ -2124,7 +2084,8 @@
                                 NonConstBB->getTerminator());
         else
           llvm_unreachable("Unknown binop!");
-        tmpWorklist.push_back(cast<Instruction>(InV));
+        
+        Worklist.Add(cast<Instruction>(InV));
       }
       NewPN->addIncoming(InV, PN->getIncomingBlock(i));
     }
@@ -2140,26 +2101,11 @@
         InV = CastInst::Create(CI->getOpcode(), PN->getIncomingValue(i), 
                                I.getType(), "phitmp", 
                                NonConstBB->getTerminator());
-        tmpWorklist.push_back(cast<Instruction>(InV));
+        Worklist.Add(cast<Instruction>(InV));
       }
       NewPN->addIncoming(InV, PN->getIncomingBlock(i));
     }
   }
-  // The PHI's operands must dominate the PHI, if we can't prove that
-  // undo this transformation.
-  if (!checkPHI(NewPN, I.getParent())) {
-    Worklist.Remove(NewPN);
-    NewPN->eraseFromParent();
-    while (!tmpWorklist.empty()) {
-      tmpWorklist.pop_back_val()->eraseFromParent();
-    }
-    return 0;
-  }
-  while (!tmpWorklist.empty()) {
-    Worklist.Add(tmpWorklist.pop_back_val());
-  }
-  NewPN->takeName(PN);
-  Worklist.Add(PN);
   return ReplaceInstUsesWith(I, NewPN);
 }
 

Removed: llvm/trunk/test/Transforms/InstCombine/fold-intophi.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/fold-intophi.ll?rev=84798&view=auto

==============================================================================
--- llvm/trunk/test/Transforms/InstCombine/fold-intophi.ll (original)
+++ llvm/trunk/test/Transforms/InstCombine/fold-intophi.ll (removed)
@@ -1,54 +0,0 @@
-; RUN: opt -instcombine -S <%s | FileCheck %s
-; PR5262
-; Make sure the PHI node gets put in a place where all of its operands dominate it
-target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128"
-target triple = "x86_64-unknown-linux-gnu"
-
- at tmp2 = global i64 0                              ; <i64*> [#uses=1]
-
-declare void @use(i64) nounwind
-
-define void @foo(i1) nounwind align 2 {
-; <label>:1
-  br i1 %0, label %2, label %3
-
-; <label>:2                                       ; preds = %1
-  br label %3
-
-; <label>:3                                       ; preds = %2, %1
-; CHECK: <label>:3
-; CHECK-NEXT: %v4 = phi i1 [ false, %2 ], [ true, %1 ]
-; CHECK-NEXT: %v4_ =  phi i1 [ true, %2 ], [ false, %1 ]
-; CHECK-NEXT: br label %l8
-  %v4 = phi i8 [ 1, %2 ], [ 0, %1 ]                ; <i8> [#uses=1]
-  %v4_ = phi i8 [ 0, %2 ], [ 1, %1 ]                ; <i8> [#uses=1]
-  %v5 = icmp eq i8 %v4, 0                           ; <i1> [#uses=1]
-  %v5_ = icmp eq i8 %v4_, 0                           ; <i1> [#uses=1]
-  %v6 = load i64* @tmp2, align 8                   ; <i64> [#uses=1]
-  %v7 = select i1 %v5, i64 0, i64 %v6                ; <i64> [#uses=1]
-  br label %l8
-
-l8:
-  %v9 = load i64* @tmp2, align 8
-  call void @use(i64 %v7)
-  br label %l10
-l10:
-  %v11 = select i1 %v5_, i64 0, i64 %v9                ; <i64> [#uses=1]
-  call void @use(i64 %v11)
-  br label %l11
-l11:
-  %v12 = load i64* @tmp2, align 8
-  %v13 = select i1 %v5_, i64 0, i64 %v12                ; <i64> [#uses=1]
-  call void @use(i64 %v13)
-  br i1 %0, label %l12, label %l13
-l12:
-  br label %l13
-l13:
-;CHECK: l13:
-;CHECK-NEXT: %v14 = phi i64 [ %v12, %l12 ], [ 0, %l11 ]
-;CHECK-NEXT: call void @use(i64 %v14)
-  %v14 = phi i1 [0, %l12], [1, %l11]
-  %v16 = select i1 %v14, i64 0, i64 %v12                ; <i64> [#uses=1]
-  call void @use(i64 %v16)
-  ret void
-}





More information about the llvm-commits mailing list