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

[Diego Caballero via Phabricator via llvm-commits]

dcaballe added a comment.

Thanks for the feedback, Nuno!

> 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.

That's a great point. One option could be avoiding masking in basic blocks with direct or indirect uses of `poison` and scalarize that code instead. We could even only scalarize the impacted instructions and apply masking to the rest. I think addressing those cases would require much more involved changes. I agree that the current fix is a good step forward. We can address more complex cases incrementally.

> 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).

LV can produce a division of a vector by a scalar by broadcasting the scalar into a vector. Is that what you mean?

I think the key points here are the instructions with the attributes and their guarding predicate. These instructions, *regardless of their uses*, may produce a poison value and UB themselves if the guarding predicate is dropped as a result of vectorization because they will be executed for iterations of the loop that wouldn't be executed in the original scalar loop. For instance, in the motivating example, a `sub` instruction will produce a poison value itself if its guarding predicate is dropped, regardless of whether the following gep/load using the result of the `sub` is widened or kept scalar. There are passes that will optimize away poison-generating instructions (i.e., this `sub`), leading to UB. For example, `InstructionSimplify` performs some of these optimizations (I think it turns this `sub` into a 0). Note that we are not blindly dropping all the poison-generating flags from all the instructions. We make sure that we only drop these flags from instructions were originally guarded by a predicate that has been dropped as a result of vectorization. If those instructions are scalarized (i.e., the predicate is preserved), only interleaved, etc., we are not dropping their flags.

Hopefully, that helps! Please, let me know if you have any other comments.

Thanks,
Diego


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111846