[PATCH] D113888: [SDAG] Use UnknownSize for masked load/store MMO size

[Dave Green via Phabricator via llvm-commits]

dmgreen added a comment.

> It's basically "masked" load/store operations.  At the IR level, those are rare, sure.  At the machine code level, there are operations other than llvm.masked.load that end up masked, though.  For example, the ARM instruction "strne".  Not sure how many places can end up with instructions like that off the top of my head.

I don't believe that is how it works though, not exactly.  In this case the backend is constructing a query of the form "does `(%1, 16)` alias `(%arrayidx4, 1)`". And because the underlying object of `%arrayidx4` is only 10 bytes large, the answer is no as the alternative would be UB. But we are using the aliasing information at the IR level, not what the mir has become. If the mir has been altered to the point that the llvm-lr level aliasing info no longer applies then yes that would be invalid, but I haven't seen that happen anywhere. A predicated store (strne) still uses the llvm-ir level aliasing info which will be valid if the original store was in an if block (or however it was predicated). And so long as that aliasing query remains valid at the ir level, the size of the predicated store's memory location should be OK.

At least - I don't have an example of that going wrong. If you do know of a way to make it act incorrectly, let me know.

> The type legalization for llvm.masked.load also has the same issue as SelectionDAGBuilder.

What do you mean by "type legalization" here exactly? I don't think global-isel handles masked loads/stores yet, and most of the illegal masked operation lowering is performed pre-isel. I wasn't sure what other type legalization you were referring to.


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

https://reviews.llvm.org/D113888