Crash in SLP for vector data type as function argument.

Hi Arnold, Shahid,

Thanks for reply and suggestions.

I would go with Arnold's suggestion not to cast VectorizedValue to Instruction and use it as it is.

I can commit patch on this (trivial change in assignment to TempVec).

However, this issue hits only when we have http://reviews.llvm.org/D6818 properly committed.
It will take some time for me to improve that patch.

This crash issue doesn't have anything to do with above patch code, so I am not getting how to put a test case for this.

Can I simply commit it without test case? If yes, should I go ahead with the commit after check-all without putting it for approval?
Please suggest a proper way to do this.

Regards,
Suyog

Sender : Arnold Schwaighofer<aschwaighofer@apple.com>
Title : Re: [LLVMdev] Crash in SLP for vector data type as function argument.

The code in emitReduction has to be fixed. As your example shows it is not safe to assume we will always have an instruction as a result of vectorizeTree(). It seems to me that we can just remove the line that performs the cast. All subsequent uses of the value ‘ValToReduce’ actually are uses of “Value *TmpVec”. The IRBuilder in the variable “Builder” carries the insertion point for all operations in this function (inserting after the instruction “ValToReduce” would be a reason why we need an “Instruction”).

  /// \brief Emit a horizontal reduction of the vectorized value.
  Value *emitReduction(Value *VectorizedValue, IRBuilder<> &Builder) {
    assert(VectorizedValue && "Need to have a vectorized tree node");
    Instruction *ValToReduce = dyn_cast(VectorizedValue);
    assert(isPowerOf2_32(ReduxWidth) &&
           "We only handle power-of-two reductions for now");

    Value *TmpVec = ValToReduce;
    for (unsigned i = ReduxWidth / 2; i != 0; i >>= 1) {
      if (IsPairwiseReduction) {
        Value *LeftMask =
          createRdxShuffleMask(ReduxWidth, i, true, true, Builder);
        Value *RightMask =
          createRdxShuffleMask(ReduxWidth, i, true, false, Builder);

        Value *LeftShuf = Builder.CreateShuffleVector(
          TmpVec, UndefValue::get(TmpVec->getType()), LeftMask, "rdx.shuf.l");
        Value *RightShuf = Builder.CreateShuffleVector(
          TmpVec, UndefValue::get(TmpVec->getType()), (RightMask),
          "rdx.shuf.r");
        TmpVec = createBinOp(Builder, ReductionOpcode, LeftShuf, RightShuf,
                             "bin.rdx");
      } else {
        Value *UpperHalf =
          createRdxShuffleMask(ReduxWidth, i, false, false, Builder);
        Value *Shuf = Builder.CreateShuffleVector(
          TmpVec, UndefValue::get(TmpVec->getType()), UpperHalf, "rdx.shuf");
        TmpVec = createBinOp(Builder, ReductionOpcode, TmpVec, Shuf, "bin.rdx");
      }
    }

Thanks,
Arnold

I am fine with this going in without a test case. This code is currently not enabled. Just mention in the commit message that the assumption that "VectorizedValue" is always an instruction is not correct because we could get a vectorized tree consisting of just a Constant or an Argument vector.

Thanks