[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 a lot for the explanation and the quick response...

>> - Should a GEP instruction keep the `inbounds` flag if after vectorization the computed address is actually out-of-bounds but out-of-bounds elements are masked out by the consumer masked load/store/gather/scatter?
>
> Yep, same as the previous. But as long as the address is not used for a widened load/store.

Let me clarify... The GEP will feed a masked load/store. We won't load the data in that out-of-bounds address (masked out) but that address will be used as base for the masked load/store. It sounds like a case similar to the one exposing the bug. Since the address will be used, I understand we should drop the `inbounds`.

>> What about vector instructions feeding a vector GEP feeding a gather/scatter?
>
> That depends. Was the original program using that same value? If so, no problem. Otherwise I need a concrete example as I can't imagine one right away :)

Gathers/scatters are modeled as a vector of pointers using a vector GEP so if an address is poison it will be masked out, at least, initially. Unfortunately, some backends will turn this vector of pointers into a single base pointer + offsets. If the base pointer is poison, we have the same issue as with masked loads/stores. This is basically a vector variant of that problem. I would suggest dropping the flags also here, following the same logic. Does it make sense?

> The problem with floats with signaling NaNs I guess. It wouldn't be correct to execute those speculatively. But that's a totally different problem.
> If it's just about FMF producing poison, then no worries and that patch doesn't seem necessary (because if something is broken, it's a much bigger problem).

Ok.

>> Maybe we could consider dropping all the flags in instructions involved in address computation. Would that be reasonable?
>
> If by that you mean dropping flags from instructions whose values flow into addresses of widened load/store operations, yes! That's a good way of fixing the 1st issue.
> Unless you know something about the mask. If the 1st lane of every iteration  is not masked out, you don't need to do anything as that value would be dereferenced in the original program.

Ok, I can add a check that looks for a widen load/store in the def-use chain.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111846