[llvm-commits] [llvm] r86443 - in /llvm/trunk: lib/Transforms/Scalar/InstructionCombining.cpp test/Transforms/InstCombine/phi.ll test/Transforms/InstCombine/sext-misc.ll

Chris Lattner sabre at nondot.org
Sun Nov 8 00:21:14 PST 2009


Author: lattner
Date: Sun Nov  8 02:21:13 2009
New Revision: 86443

URL: http://llvm.org/viewvc/llvm-project?rev=86443&view=rev
Log:
teach a couple of instcombine transformations involving PHIs to
not turn a PHI in a legal type into a PHI of an illegal type, and
add a new optimization that breaks up insane integer PHI nodes into
small pieces (PR3451).

Modified:
    llvm/trunk/lib/Transforms/Scalar/InstructionCombining.cpp
    llvm/trunk/test/Transforms/InstCombine/phi.ll
    llvm/trunk/test/Transforms/InstCombine/sext-misc.ll

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

==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/InstructionCombining.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/InstructionCombining.cpp Sun Nov  8 02:21:13 2009
@@ -283,6 +283,8 @@
     Instruction *visitSelectInstWithICmp(SelectInst &SI, ICmpInst *ICI);
     Instruction *visitCallInst(CallInst &CI);
     Instruction *visitInvokeInst(InvokeInst &II);
+
+    Instruction *SliceUpIllegalIntegerPHI(PHINode &PN);
     Instruction *visitPHINode(PHINode &PN);
     Instruction *visitGetElementPtrInst(GetElementPtrInst &GEP);
     Instruction *visitAllocaInst(AllocaInst &AI);
@@ -8083,8 +8085,7 @@
 Value *InstCombiner::EvaluateInDifferentType(Value *V, const Type *Ty, 
                                              bool isSigned) {
   if (Constant *C = dyn_cast<Constant>(V))
-    return ConstantExpr::getIntegerCast(C, Ty,
-                                               isSigned /*Sext or ZExt*/);
+    return ConstantExpr::getIntegerCast(C, Ty, isSigned /*Sext or ZExt*/);
 
   // Otherwise, it must be an instruction.
   Instruction *I = cast<Instruction>(V);
@@ -8117,8 +8118,7 @@
       return I->getOperand(0);
     
     // Otherwise, must be the same type of cast, so just reinsert a new one.
-    Res = CastInst::Create(cast<CastInst>(I)->getOpcode(), I->getOperand(0),
-                           Ty);
+    Res = CastInst::Create(cast<CastInst>(I)->getOpcode(), I->getOperand(0),Ty);
     break;
   case Instruction::Select: {
     Value *True = EvaluateInDifferentType(I->getOperand(1), Ty, isSigned);
@@ -8167,9 +8167,17 @@
       return NV;
 
   // If we are casting a PHI then fold the cast into the PHI
-  if (isa<PHINode>(Src))
-    if (Instruction *NV = FoldOpIntoPhi(CI))
-      return NV;
+  if (isa<PHINode>(Src)) {
+    // We don't do this if this would create a PHI node with an illegal type if
+    // it is currently legal.
+    if (!isa<IntegerType>(Src->getType()) ||
+        !isa<IntegerType>(CI.getType()) ||
+        (TD && TD->isLegalInteger(CI.getType()->getPrimitiveSizeInBits())) ||
+        (TD && !TD->isLegalInteger(Src->getType()->getPrimitiveSizeInBits())))
+      if (Instruction *NV = FoldOpIntoPhi(CI))
+        return NV;
+    
+  }
   
   return 0;
 }
@@ -8508,7 +8516,7 @@
       return BinaryOperator::CreateLShr(V1, V2);
     }
   }
-  
+ 
   return 0;
 }
 
@@ -10886,6 +10894,15 @@
   
   if (isa<CastInst>(FirstInst)) {
     CastSrcTy = FirstInst->getOperand(0)->getType();
+    
+    // If this is a legal integer PHI node, and pulling the operation through
+    // would cause it to be an illegal integer PHI, don't do the transformation.
+    if (!TD ||
+        (isa<IntegerType>(PN.getType()) &&
+         isa<IntegerType>(CastSrcTy) &&
+         TD->isLegalInteger(PN.getType()->getPrimitiveSizeInBits()) &&
+         !TD->isLegalInteger(CastSrcTy->getPrimitiveSizeInBits())))
+      return 0;
   } else if (isa<BinaryOperator>(FirstInst) || isa<CmpInst>(FirstInst)) {
     // Can fold binop, compare or shift here if the RHS is a constant, 
     // otherwise call FoldPHIArgBinOpIntoPHI.
@@ -10998,6 +11015,123 @@
 }
 
 
+namespace {
+struct PHIUsageRecord {
+  unsigned Shift;     // The amount shifted.
+  Instruction *Inst;  // The trunc instruction.
+  
+  PHIUsageRecord(unsigned Sh, Instruction *User) : Shift(Sh), Inst(User) {}
+  
+  bool operator<(const PHIUsageRecord &RHS) const {
+    if (Shift < RHS.Shift) return true;
+    return Shift == RHS.Shift &&
+           Inst->getType()->getPrimitiveSizeInBits() <
+           RHS.Inst->getType()->getPrimitiveSizeInBits();
+  }
+};
+}
+
+
+/// SliceUpIllegalIntegerPHI - This is an integer PHI and we know that it has an
+/// illegal type: see if it is only used by trunc or trunc(lshr) operations.  If
+/// so, we split the PHI into the various pieces being extracted.  This sort of
+/// thing is introduced when SROA promotes an aggregate to large integer values.
+///
+/// TODO: The user of the trunc may be an bitcast to float/double/vector or an
+/// inttoptr.  We should produce new PHIs in the right type.
+///
+Instruction *InstCombiner::SliceUpIllegalIntegerPHI(PHINode &PN) {
+  SmallVector<PHIUsageRecord, 16> PHIUsers;
+  
+  for (Value::use_iterator UI = PN.use_begin(), E = PN.use_end();
+       UI != E; ++UI) {
+    Instruction *User = cast<Instruction>(*UI);
+    
+    // The PHI can use itself.
+    if (User == &PN)
+      continue;
+    
+    // Truncates are always ok.
+    if (isa<TruncInst>(User)) {
+      PHIUsers.push_back(PHIUsageRecord(0, User));
+      continue;
+    }
+    
+    // Otherwise it must be a lshr which can only be used by one trunc.
+    if (User->getOpcode() != Instruction::LShr ||
+        !User->hasOneUse() || !isa<TruncInst>(User->use_back()) ||
+        !isa<ConstantInt>(User->getOperand(1)))
+      return 0;
+    
+    unsigned Shift = cast<ConstantInt>(User->getOperand(1))->getZExtValue();
+    PHIUsers.push_back(PHIUsageRecord(Shift, User->use_back()));
+  }
+  
+  // If we have no users, they must be all self uses, just nuke the PHI.
+  if (PHIUsers.empty())
+    return ReplaceInstUsesWith(PN, UndefValue::get(PN.getType()));
+  
+  // If this phi node is transformable, create new PHIs for all the pieces
+  // extracted out of it.  First, sort the users by their offset and size.
+  array_pod_sort(PHIUsers.begin(), PHIUsers.end());
+  
+  
+  DenseMap<BasicBlock*, Value*> PredValues;
+  
+  unsigned UserI = 0, UserE = PHIUsers.size();
+  while (1) {
+    assert(UserI != UserE && "Iteration fail, loop below should catch this");
+    
+    unsigned Offset = PHIUsers[UserI].Shift;
+    const Type *Ty = PHIUsers[UserI].Inst->getType();
+
+    // Create the new PHI node for this user.
+    PHINode *EltPHI =
+      PHINode::Create(Ty, PN.getName()+".off"+Twine(Offset), &PN);
+    
+    for (unsigned i = 0, e = PN.getNumIncomingValues(); i != e; ++i) {
+      BasicBlock *Pred = PN.getIncomingBlock(i);
+      Value *&PredVal = PredValues[Pred];
+      
+      // If we already have a value for this predecessor, reuse it.
+      if (PredVal) {
+        EltPHI->addIncoming(PredVal, Pred);
+        continue;
+      }
+
+      // Handle the PHI self-reuse case.
+      Value *InVal = PN.getIncomingValue(i);
+      if (InVal == &PN) {
+        PredVal = EltPHI;
+        EltPHI->addIncoming(PredVal, Pred);
+        continue;
+      }
+      
+      // Otherwise, do an extract in the predecessor.
+      Builder->SetInsertPoint(Pred, Pred->getTerminator());
+      if (Offset)
+        InVal = Builder->CreateLShr(InVal, ConstantInt::get(InVal->getType(),
+                                                            Offset), "extract");
+      InVal = Builder->CreateTrunc(InVal, Ty, "extract.t");
+      PredVal = InVal;
+      EltPHI->addIncoming(PredVal, Pred);
+    }
+    PredValues.clear();
+    
+    // Now that we have a new PHI node, replace all uses of this piece of the
+    // PHI with the one new PHI.
+    while (PHIUsers[UserI].Shift == Offset &&
+           PHIUsers[UserI].Inst->getType() == Ty) {
+      ReplaceInstUsesWith(*PHIUsers[UserI].Inst, EltPHI);
+
+      // If we replaced the last PHI user, we're done.  Just replace all the
+      // remaining uses of the PHI (self uses and the lshrs with undefs.
+      if (++UserI == UserE)
+        return ReplaceInstUsesWith(PN, UndefValue::get(PN.getType()));
+    }
+  }
+}
+
 // PHINode simplification
 //
 Instruction *InstCombiner::visitPHINode(PHINode &PN) {
@@ -11103,6 +11237,15 @@
       }
     }
 
+  // If this is an integer PHI and we know that it has an illegal type, see if
+  // it is only used by trunc or trunc(lshr) operations.  If so, we split the
+  // PHI into the various pieces being extracted.  This sort of thing is
+  // introduced when SROA promotes an aggregate to a single large integer type.
+  if (isa<IntegerType>(PN.getType()) && TD &&
+      !TD->isLegalInteger(PN.getType()->getPrimitiveSizeInBits()))
+    if (Instruction *Res = SliceUpIllegalIntegerPHI(PN))
+      return Res;
+  
   return 0;
 }
 

Modified: llvm/trunk/test/Transforms/InstCombine/phi.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/phi.ll?rev=86443&r1=86442&r2=86443&view=diff

==============================================================================
--- llvm/trunk/test/Transforms/InstCombine/phi.ll (original)
+++ llvm/trunk/test/Transforms/InstCombine/phi.ll Sun Nov  8 02:21:13 2009
@@ -2,6 +2,8 @@
 ;
 ; RUN: opt < %s -instcombine -S | FileCheck %s
 
+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"
+
 define i32 @test1(i32 %A, i1 %b) {
 BB0:
         br i1 %b, label %BB1, label %BB2

Modified: llvm/trunk/test/Transforms/InstCombine/sext-misc.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/sext-misc.ll?rev=86443&r1=86442&r2=86443&view=diff

==============================================================================
--- llvm/trunk/test/Transforms/InstCombine/sext-misc.ll (original)
+++ llvm/trunk/test/Transforms/InstCombine/sext-misc.ll Sun Nov  8 02:21:13 2009
@@ -1,5 +1,7 @@
 ; RUN: opt < %s -instcombine -S | not grep sext
 
+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"
+
 declare i32 @llvm.ctpop.i32(i32)
 declare i32 @llvm.ctlz.i32(i32)
 declare i32 @llvm.cttz.i32(i32)





More information about the llvm-commits mailing list