[PATCH] D97861: [LoopVectorize][NFC] Refactor code to use IRBuilder::CreateStepVector

[Christopher Tetreault via Phabricator via llvm-commits]

ctetreau added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:2265
+
+  // Add on StartIdx
+  Value *StartIdxSplat = Builder.CreateVectorSplat(
----------------
It seems like this does the same thing as the original version. However, I don't think this function does what it claims to do.

The docs say that this computes `Val + <StartIdx, StartIdx + Step, StartIdx + 2*Step, ...>`

But what this function does is:
```
StartIdxSplat = <StartIdx, StartIdx, ...>
InitVec = StartIdxSplat + <0, 1, ...>
Step = <InputStep, InputStep, ...>
result = Val + (InitVec * Step)
```

If the input step is 1, this seems to do the right thing:

```
StartIdx = 2 // some non-1 start for illustrative purposes
Val = <a, b, c, d>
InputStep = 1

StartIdxSplat = <2, 2, 2, 2>
InitVec = <2, 2, 2, 2> + <0, 1, 2, 3> = <2, 3, 4, 5>
Step = <1, 1, 1, 1>
result = <a, b, c, d> + (<2, 3, 4, 5> * <1, 1, 1, 1>) = <a, b, c, d> + <2, 3, 4, 5>
```

But if we try a larger input step:

```
StartIdx = 2 // some non-1 start for illustrative purposes
Val = <a, b, c, d>
InputStep = 2

StartIdxSplat = <2, 2, 2, 2>
InitVec = <2, 2, 2, 2> + <0, 1, 2, 3> = <2, 3, 4, 5>
Step = <2, 2, 2, 2>
result = <a, b, c, d> + (<2, 3, 4, 5> * <2, 2, 2, 2>) = <a, b, c, d> + <4, 6, 8, 10>
```

The label on the tin says that the first element of the RHS vector should be equal to `StartIdx`, which it clearly isn't. I haven't scrutinized the floating point codepath, but I assume it has a similar issue.

I believe we need to multiply the step vector by the step before adding the start value.

I feel like we should do something about this. Either assert that the step is 1 and add a fixme, or just fix the function.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97861/new/

https://reviews.llvm.org/D97861