From cddc8aa1b8a5fa7ca72e16af4ad35a79892e31c6 Mon Sep 17 00:00:00 2001 From: Chris Lattner Date: Sun, 8 Nov 2009 08:21:13 +0000 Subject: [PATCH] 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). llvm-svn: 86443 --- .../Scalar/InstructionCombining.cpp | 159 +++++++++++++++++- test/Transforms/InstCombine/phi.ll | 2 + test/Transforms/InstCombine/sext-misc.ll | 2 + 3 files changed, 155 insertions(+), 8 deletions(-) diff --git a/lib/Transforms/Scalar/InstructionCombining.cpp b/lib/Transforms/Scalar/InstructionCombining.cpp index 74d0971b422..0d23e1521c1 100644 --- a/lib/Transforms/Scalar/InstructionCombining.cpp +++ b/lib/Transforms/Scalar/InstructionCombining.cpp @@ -283,6 +283,8 @@ namespace { 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 @@ bool InstCombiner::CanEvaluateInDifferentType(Value *V, const Type *Ty, Value *InstCombiner::EvaluateInDifferentType(Value *V, const Type *Ty, bool isSigned) { if (Constant *C = dyn_cast(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(V); @@ -8117,8 +8118,7 @@ Value *InstCombiner::EvaluateInDifferentType(Value *V, const Type *Ty, return I->getOperand(0); // Otherwise, must be the same type of cast, so just reinsert a new one. - Res = CastInst::Create(cast(I)->getOpcode(), I->getOperand(0), - Ty); + Res = CastInst::Create(cast(I)->getOpcode(), I->getOperand(0),Ty); break; case Instruction::Select: { Value *True = EvaluateInDifferentType(I->getOperand(1), Ty, isSigned); @@ -8167,9 +8167,17 @@ Instruction *InstCombiner::commonCastTransforms(CastInst &CI) { return NV; // If we are casting a PHI then fold the cast into the PHI - if (isa(Src)) - if (Instruction *NV = FoldOpIntoPhi(CI)) - return NV; + if (isa(Src)) { + // We don't do this if this would create a PHI node with an illegal type if + // it is currently legal. + if (!isa(Src->getType()) || + !isa(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 @@ Instruction *InstCombiner::visitTrunc(TruncInst &CI) { return BinaryOperator::CreateLShr(V1, V2); } } - + return 0; } @@ -10886,6 +10894,15 @@ Instruction *InstCombiner::FoldPHIArgOpIntoPHI(PHINode &PN) { if (isa(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(PN.getType()) && + isa(CastSrcTy) && + TD->isLegalInteger(PN.getType()->getPrimitiveSizeInBits()) && + !TD->isLegalInteger(CastSrcTy->getPrimitiveSizeInBits()))) + return 0; } else if (isa(FirstInst) || isa(FirstInst)) { // Can fold binop, compare or shift here if the RHS is a constant, // otherwise call FoldPHIArgBinOpIntoPHI. @@ -10998,6 +11015,123 @@ static bool PHIsEqualValue(PHINode *PN, Value *NonPhiInVal, } +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 PHIUsers; + + for (Value::use_iterator UI = PN.use_begin(), E = PN.use_end(); + UI != E; ++UI) { + Instruction *User = cast(*UI); + + // The PHI can use itself. + if (User == &PN) + continue; + + // Truncates are always ok. + if (isa(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(User->use_back()) || + !isa(User->getOperand(1))) + return 0; + + unsigned Shift = cast(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 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 @@ Instruction *InstCombiner::visitPHINode(PHINode &PN) { } } + // 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(PN.getType()) && TD && + !TD->isLegalInteger(PN.getType()->getPrimitiveSizeInBits())) + if (Instruction *Res = SliceUpIllegalIntegerPHI(PN)) + return Res; + return 0; } diff --git a/test/Transforms/InstCombine/phi.ll b/test/Transforms/InstCombine/phi.ll index b73ce3f9867..23dec4a1b0e 100644 --- a/test/Transforms/InstCombine/phi.ll +++ b/test/Transforms/InstCombine/phi.ll @@ -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 diff --git a/test/Transforms/InstCombine/sext-misc.ll b/test/Transforms/InstCombine/sext-misc.ll index 107bba6e843..3346ff87add 100644 --- a/test/Transforms/InstCombine/sext-misc.ll +++ b/test/Transforms/InstCombine/sext-misc.ll @@ -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)