[PATCH] D111846: [LV] Drop integer poison-generating flags from instructions that need predication

[Nuno Lopes via Phabricator via llvm-commits]

nlopes added a comment.

I don't know the code of the loop vectorizer, but I have some 2 concerns.

1. Correctness:

The underlying issue of the bug report is that we widen a load to a masked vector load which takes only the address of the first iteration. We need to ensure this address is not poison if it wouldn't be poison for the non-masked out loads.
Dropping poison-producing flags from any operation that is hoisted of conditional BBs that contribute to the address is a good step forward. But it doesn't seem enough.
Consider this example:
loop:

  %i = phi [poison, %entry], [%i3, %loop]
  %i2 = phi [0, %entry], [%i3, loop]
  if (%i2 > 0) {
     %p = gep %p, %i
     load %p
  }
   %i3 = add %i2, 1
   br %cond, loop, ..

Now vectorize that load to a masked load of %p and we are doomed because %p is poison in the first iteration.
Not sure this example would kick in with the vectorizer as it needs to prove that loads are contiguous, but maybe SCEV will take the poison as 0 and vectorization kicks in. Anyway, just to say that just dropping poison-producing attributes may not be enough.

The second point is that it seems the patch drops attributes from all hoisted instructions, but that's not strictly needed. You only need to drop attributes from instructions that contribute to instructions that are widened and that produce UB if given poison. I don't think LLVM can produce e.g. a division of a vector by a scalar (if the loop always divides by the same value).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111846