[llvm] r182976 - Reapply with r182909 with a fix to the calculation of the new indices for

Pete Cooper peter_cooper at apple.com
Fri May 31 15:30:55 PDT 2013


Hi Nick

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.

Thanks,
Pete

define <2 x float> @main(float %x, float %z) {
  %resext = insertelement <4 x float> undef, float %x, i32 0
  %resext1 = insertelement <4 x float> %resext, float %z, i32 2
  %xy = shufflevector <4 x float> %resext1, <4 x float> undef, <2 x i32> <i32 0, i32 2>
  ret <2 x float> %xy
}
On May 30, 2013, at 5:59 PM, Nick Lewycky <nicholas at mxc.ca> wrote:

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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130531/effe1a43/attachment.html>


More information about the llvm-commits mailing list