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

[Diego Caballero via Phabricator via llvm-commits]

dcaballe added a comment.

>> 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`.
>
> A masked load of a masked out lane is a NOP essentially, so the address can be poison as it's not actually used. You don't need to drop inbounds.
> LangRef doesn't require the address of masked out lanes to be aligned, as in e.g. memcpy that even with size=0 the address must be properly aligned.

Ok, let me update the patch so that we can discuss on actual examples. I have the impression that we both agree but might be looking at it from a different perspective.

>> 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?
>
> Sounds like the same problem, yes. But just dropping flags isn't enough, because similarly the base pointer could have been poison already.
> Those backends are buggy. Fixing requires either proving the base pointer isn't poison in the fist place, or deriving a base address from a non-masked out lane (probably the easiest solution?).

Deriving a base address from a non-masked out lane makes sense but we don't always have that information at compile time. In any case, this patch is a good starting point to fix the backend problems. We can iterate on it later as we see fit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111846