<html><head><meta http-equiv="Content-Type" content="text/html charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">Hi Nick<div><br></div><div>I’m getting a crash on a buildbot with this change. Here’s a reduced test case. Would you mind checking if the failure here is related to this patch.</div><div><br></div><div>Thanks,</div><div>Pete</div><div><br></div><div><div>define <2 x float> @main(float %x, float %z) {</div><div> %resext = insertelement <4 x float> undef, float %x, i32 0</div><div> %resext1 = insertelement <4 x float> %resext, float %z, i32 2</div><div> %xy = shufflevector <4 x float> %resext1, <4 x float> undef, <2 x i32> <i32 0, i32 2></div><div> ret <2 x float> %xy</div><div>}</div><div><div>On May 30, 2013, at 5:59 PM, Nick Lewycky <<a href="mailto:nicholas@mxc.ca">nicholas@mxc.ca</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">Author: nicholas<br>Date: Thu May 30 19:59:42 2013<br>New Revision: 182976<br><br>URL:<span class="Apple-converted-space"> </span><a href="http://llvm.org/viewvc/llvm-project?rev=182976&view=rev">http://llvm.org/viewvc/llvm-project?rev=182976&view=rev</a><br>Log:<br>Reapply with r182909 with a fix to the calculation of the new indices for<br>insertelement instructions.<br><br>Modified:<br> llvm/trunk/lib/Transforms/InstCombine/InstCombine.h<br> llvm/trunk/lib/Transforms/InstCombine/InstCombineVectorOps.cpp<br> llvm/trunk/test/Transforms/InstCombine/vec_shuffle.ll<br><br>Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombine.h<br>URL:<span class="Apple-converted-space"> </span><a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombine.h?rev=182976&r1=182975&r2=182976&view=diff">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombine.h?rev=182976&r1=182975&r2=182976&view=diff</a><br>==============================================================================<br>--- llvm/trunk/lib/Transforms/InstCombine/InstCombine.h (original)<br>+++ llvm/trunk/lib/Transforms/InstCombine/InstCombine.h Thu May 30 19:59:42 2013<br>@@ -234,6 +234,7 @@ private:<br> bool WillNotOverflowSignedAdd(Value *LHS, Value *RHS);<br> Value *EmitGEPOffset(User *GEP);<br> Instruction *scalarizePHI(ExtractElementInst &EI, PHINode *PN);<br>+ Value *EvaluateInDifferentElementOrder(Value *V, ArrayRef<int> Mask);<br><br>public:<br> // InsertNewInstBefore - insert an instruction New before instruction Old<br><br>Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineVectorOps.cpp<br>URL:<span class="Apple-converted-space"> </span><a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineVectorOps.cpp?rev=182976&r1=182975&r2=182976&view=diff">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineVectorOps.cpp?rev=182976&r1=182975&r2=182976&view=diff</a><br>==============================================================================<br>--- llvm/trunk/lib/Transforms/InstCombine/InstCombineVectorOps.cpp (original)<br>+++ llvm/trunk/lib/Transforms/InstCombine/InstCombineVectorOps.cpp Thu May 30 19:59:42 2013<br>@@ -494,6 +494,250 @@ Instruction *InstCombiner::visitInsertEl<br> return 0;<br>}<br><br>+/// Return true if we can evaluate the specified expression tree if the vector<br>+/// elements were shuffled in a different order.<br>+static bool CanEvaluateShuffled(Value *V, ArrayRef<int> Mask,<br>+ unsigned Depth = 100) {<br>+ // We can always reorder the elements of a constant.<br>+ if (isa<Constant>(V))<br>+ return true;<br>+<br>+ // We won't reorder vector arguments. No IPO here.<br>+ Instruction *I = dyn_cast<Instruction>(V);<br>+ if (!I) return false;<br>+<br>+ // Two users may expect different orders of the elements. Don't try it.<br>+ if (!I->hasOneUse())<br>+ return false;<br>+<br>+ if (Depth == 0) return false;<br>+<br>+ switch (I->getOpcode()) {<br>+ case Instruction::Add:<br>+ case Instruction::FAdd:<br>+ case Instruction::Sub:<br>+ case Instruction::FSub:<br>+ case Instruction::Mul:<br>+ case Instruction::FMul:<br>+ case Instruction::UDiv:<br>+ case Instruction::SDiv:<br>+ case Instruction::FDiv:<br>+ case Instruction::URem:<br>+ case Instruction::SRem:<br>+ case Instruction::FRem:<br>+ case Instruction::Shl:<br>+ case Instruction::LShr:<br>+ case Instruction::AShr:<br>+ case Instruction::And:<br>+ case Instruction::Or:<br>+ case Instruction::Xor:<br>+ case Instruction::ICmp:<br>+ case Instruction::FCmp:<br>+ case Instruction::Trunc:<br>+ case Instruction::ZExt:<br>+ case Instruction::SExt:<br>+ case Instruction::FPToUI:<br>+ case Instruction::FPToSI:<br>+ case Instruction::UIToFP:<br>+ case Instruction::SIToFP:<br>+ case Instruction::FPTrunc:<br>+ case Instruction::FPExt:<br>+ case Instruction::GetElementPtr: {<br>+ for (int i = 0, e = I->getNumOperands(); i != e; ++i) {<br>+ if (!CanEvaluateShuffled(I->getOperand(i), Mask, Depth-1))<br>+ return false;<br>+ }<br>+ return true;<br>+ }<br>+ case Instruction::InsertElement: {<br>+ ConstantInt *CI = dyn_cast<ConstantInt>(I->getOperand(2));<br>+ if (!CI) return false;<br>+ int ElementNumber = CI->getLimitedValue();<br>+<br>+ // Verify that 'CI' does not occur twice in Mask. A single 'insertelement'<br>+ // can't put an element into multiple indices.<br>+ bool SeenOnce = false;<br>+ for (int i = 0, e = Mask.size(); i != e; ++i) {<br>+ if (Mask[i] == ElementNumber) {<br>+ if (SeenOnce)<br>+ return false;<br>+ SeenOnce = true;<br>+ }<br>+ }<br>+ return CanEvaluateShuffled(I->getOperand(0), Mask, Depth-1);<br>+ }<br>+ }<br>+ return false;<br>+}<br>+<br>+/// Rebuild a new instruction just like 'I' but with the new operands given.<br>+/// In the event of type mismatch, the type of the operands is correct.<br>+static Value *BuildNew(Instruction *I, ArrayRef<Value*> NewOps) {<br>+ // We don't want to use the IRBuilder here because we want the replacement<br>+ // instructions to appear next to 'I', not the builder's insertion point.<br>+ switch (I->getOpcode()) {<br>+ case Instruction::Add:<br>+ case Instruction::FAdd:<br>+ case Instruction::Sub:<br>+ case Instruction::FSub:<br>+ case Instruction::Mul:<br>+ case Instruction::FMul:<br>+ case Instruction::UDiv:<br>+ case Instruction::SDiv:<br>+ case Instruction::FDiv:<br>+ case Instruction::URem:<br>+ case Instruction::SRem:<br>+ case Instruction::FRem:<br>+ case Instruction::Shl:<br>+ case Instruction::LShr:<br>+ case Instruction::AShr:<br>+ case Instruction::And:<br>+ case Instruction::Or:<br>+ case Instruction::Xor: {<br>+ BinaryOperator *BO = cast<BinaryOperator>(I);<br>+ assert(NewOps.size() == 2 && "binary operator with #ops != 2");<br>+ BinaryOperator *New =<br>+ BinaryOperator::Create(cast<BinaryOperator>(I)->getOpcode(),<br>+ NewOps[0], NewOps[1], "", BO);<br>+ if (isa<OverflowingBinaryOperator>(BO)) {<br>+ New->setHasNoUnsignedWrap(BO->hasNoUnsignedWrap());<br>+ New->setHasNoSignedWrap(BO->hasNoSignedWrap());<br>+ }<br>+ if (isa<PossiblyExactOperator>(BO)) {<br>+ New->setIsExact(BO->isExact());<br>+ }<br>+ return New;<br>+ }<br>+ case Instruction::ICmp:<br>+ assert(NewOps.size() == 2 && "icmp with #ops != 2");<br>+ return new ICmpInst(I, cast<ICmpInst>(I)->getPredicate(),<br>+ NewOps[0], NewOps[1]);<br>+ case Instruction::FCmp:<br>+ assert(NewOps.size() == 2 && "fcmp with #ops != 2");<br>+ return new FCmpInst(I, cast<FCmpInst>(I)->getPredicate(),<br>+ NewOps[0], NewOps[1]);<br>+ case Instruction::Trunc:<br>+ case Instruction::ZExt:<br>+ case Instruction::SExt:<br>+ case Instruction::FPToUI:<br>+ case Instruction::FPToSI:<br>+ case Instruction::UIToFP:<br>+ case Instruction::SIToFP:<br>+ case Instruction::FPTrunc:<br>+ case Instruction::FPExt: {<br>+ // It's possible that the mask has a different number of elements from<br>+ // the original cast. We recompute the destination type to match the mask.<br>+ Type *DestTy =<br>+ VectorType::get(I->getType()->getScalarType(),<br>+ NewOps[0]->getType()->getVectorNumElements());<br>+ assert(NewOps.size() == 1 && "cast with #ops != 1");<br>+ return CastInst::Create(cast<CastInst>(I)->getOpcode(), NewOps[0], DestTy,<br>+ "", I);<br>+ }<br>+ case Instruction::GetElementPtr: {<br>+ Value *Ptr = NewOps[0];<br>+ ArrayRef<Value*> Idx = NewOps.slice(1);<br>+ GetElementPtrInst *GEP = GetElementPtrInst::Create(Ptr, Idx, "", I);<br>+ GEP->setIsInBounds(cast<GetElementPtrInst>(I)->isInBounds());<br>+ return GEP;<br>+ }<br>+ }<br>+ llvm_unreachable("failed to rebuild vector instructions");<br>+}<br>+<br>+Value *<br>+InstCombiner::EvaluateInDifferentElementOrder(Value *V, ArrayRef<int> Mask) {<br>+ // Mask.size() does not need to be equal to the number of vector elements.<br>+<br>+ assert(V->getType()->isVectorTy() && "can't reorder non-vector elements");<br>+ if (isa<UndefValue>(V)) {<br>+ return UndefValue::get(VectorType::get(V->getType()->getScalarType(),<br>+ Mask.size()));<br>+ }<br>+ if (isa<ConstantAggregateZero>(V)) {<br>+ return ConstantAggregateZero::get(<br>+ VectorType::get(V->getType()->getScalarType(),<br>+ Mask.size()));<br>+ }<br>+ if (Constant *C = dyn_cast<Constant>(V)) {<br>+ SmallVector<Constant *, 16> MaskValues;<br>+ for (int i = 0, e = Mask.size(); i != e; ++i) {<br>+ if (Mask[i] == -1)<br>+ MaskValues.push_back(UndefValue::get(Builder->getInt32Ty()));<br>+ else<br>+ MaskValues.push_back(Builder->getInt32(Mask[i]));<br>+ }<br>+ return ConstantExpr::getShuffleVector(C, UndefValue::get(C->getType()),<br>+ ConstantVector::get(MaskValues));<br>+ }<br>+<br>+ Instruction *I = cast<Instruction>(V);<br>+ switch (I->getOpcode()) {<br>+ case Instruction::Add:<br>+ case Instruction::FAdd:<br>+ case Instruction::Sub:<br>+ case Instruction::FSub:<br>+ case Instruction::Mul:<br>+ case Instruction::FMul:<br>+ case Instruction::UDiv:<br>+ case Instruction::SDiv:<br>+ case Instruction::FDiv:<br>+ case Instruction::URem:<br>+ case Instruction::SRem:<br>+ case Instruction::FRem:<br>+ case Instruction::Shl:<br>+ case Instruction::LShr:<br>+ case Instruction::AShr:<br>+ case Instruction::And:<br>+ case Instruction::Or:<br>+ case Instruction::Xor:<br>+ case Instruction::ICmp:<br>+ case Instruction::FCmp:<br>+ case Instruction::Trunc:<br>+ case Instruction::ZExt:<br>+ case Instruction::SExt:<br>+ case Instruction::FPToUI:<br>+ case Instruction::FPToSI:<br>+ case Instruction::UIToFP:<br>+ case Instruction::SIToFP:<br>+ case Instruction::FPTrunc:<br>+ case Instruction::FPExt:<br>+ case Instruction::Select:<br>+ case Instruction::GetElementPtr: {<br>+ SmallVector<Value*, 8> NewOps;<br>+ bool NeedsRebuild = (Mask.size() != I->getType()->getVectorNumElements());<br>+ for (int i = 0, e = I->getNumOperands(); i != e; ++i) {<br>+ Value *V = EvaluateInDifferentElementOrder(I->getOperand(i), Mask);<br>+ NewOps.push_back(V);<br>+ NeedsRebuild |= (V != I->getOperand(i));<br>+ }<br>+ if (NeedsRebuild) {<br>+ return BuildNew(I, NewOps);<br>+ }<br>+ return I;<br>+ }<br>+ case Instruction::InsertElement: {<br>+ int Element = cast<ConstantInt>(I->getOperand(2))->getLimitedValue();<br>+ if (Element < 0 || Element >= (int)Mask.size()) {<br>+ // Such instructions are valid and exhibit undefined behaviour.<br>+ return UndefValue::get(I->getType());<br>+ }<br>+<br>+ // The insertelement was inserting at Element. Figure out which element<br>+ // that becomes after shuffling. The answer is guaranteed to be unique<br>+ // by CanEvaluateShuffled.<br>+ int Index = 0;<br>+ for (int e = Mask.size(); Index != e; ++Index)<br>+ if (Mask[Index] == Element)<br>+ break;<br>+<br>+ Value *V = EvaluateInDifferentElementOrder(I->getOperand(0), Mask);<br>+ return InsertElementInst::Create(V, I->getOperand(1),<br>+ Builder->getInt32(Index), "", I);<br>+ }<br>+ }<br>+ llvm_unreachable("failed to reorder elements of vector instruction!");<br>+}<br><br>Instruction *InstCombiner::visitShuffleVectorInst(ShuffleVectorInst &SVI) {<br> Value *LHS = SVI.getOperand(0);<br>@@ -525,9 +769,9 @@ Instruction *InstCombiner::visitShuffleV<br> if (LHS == RHS || isa<UndefValue>(LHS)) {<br> if (isa<UndefValue>(LHS) && LHS == RHS) {<br> // shuffle(undef,undef,mask) -> undef.<br>- Value* result = (VWidth == LHSWidth)<br>+ Value *Result = (VWidth == LHSWidth)<br> ? LHS : UndefValue::get(SVI.getType());<br>- return ReplaceInstUsesWith(SVI, result);<br>+ return ReplaceInstUsesWith(SVI, Result);<br> }<br><br> // Remap any references to RHS to use LHS.<br>@@ -574,6 +818,16 @@ Instruction *InstCombiner::visitShuffleV<br> if (isRHSID) return ReplaceInstUsesWith(SVI, RHS);<br> }<br><br>+ if (isa<UndefValue>(RHS) &&<br>+ // This isn't necessary for correctness, but the comment block below<br>+ // claims that there are cases where folding two shuffles into one would<br>+ // cause worse codegen on some targets.<br>+ !isa<ShuffleVectorInst>(LHS) &&<br>+ CanEvaluateShuffled(LHS, Mask)) {<br>+ Value *V = EvaluateInDifferentElementOrder(LHS, Mask);<br>+ return ReplaceInstUsesWith(SVI, V);<br>+ }<br>+<br> // If the LHS is a shufflevector itself, see if we can combine it with this<br> // one without producing an unusual shuffle.<br> // Cases that might be simplified:<br><br>Modified: llvm/trunk/test/Transforms/InstCombine/vec_shuffle.ll<br>URL:<span class="Apple-converted-space"> </span><a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/vec_shuffle.ll?rev=182976&r1=182975&r2=182976&view=diff">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/vec_shuffle.ll?rev=182976&r1=182975&r2=182976&view=diff</a><br>==============================================================================<br>--- llvm/trunk/test/Transforms/InstCombine/vec_shuffle.ll (original)<br>+++ llvm/trunk/test/Transforms/InstCombine/vec_shuffle.ll Thu May 30 19:59:42 2013<br>@@ -153,3 +153,24 @@ define <8 x i8> @test12a(<8 x i8> %tmp6,<br> ret <8 x i8> %tmp3<br>}<br><br>+define <2 x i8> @test13a(i8 %x1, i8 %x2) {<br>+; CHECK: @test13a<br>+; CHECK-NEXT: insertelement {{.*}} undef, i8 %x1, i32 1<br>+; CHECK-NEXT: insertelement {{.*}} i8 %x2, i32 0<br>+; CHECK-NEXT: add {{.*}} <i8 7, i8 5><br>+; CHECK-NEXT: ret<br>+ %A = insertelement <2 x i8> undef, i8 %x1, i32 0<br>+ %B = insertelement <2 x i8> %A, i8 %x2, i32 1<br>+ %C = add <2 x i8> %B, <i8 5, i8 7><br>+ %D = shufflevector <2 x i8> %C, <2 x i8> undef, <2 x i32> <i32 1, i32 0><br>+ ret <2 x i8> %D<br>+}<br>+<br>+define <2 x i8> @test13b(i8 %x) {<br>+; CHECK: @test13b<br>+; CHECK-NEXT: insertelement <2 x i8> undef, i8 %x, i32 1<br>+; CHECK-NEXT: ret<br>+ %A = insertelement <2 x i8> undef, i8 %x, i32 0<br>+ %B = shufflevector <2 x i8> %A, <2 x i8> undef, <2 x i32> <i32 undef, i32 0><br>+ ret <2 x i8> %B<br>+}<br><br><br>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a></div></blockquote></div><br></div></body></html>