[PATCH] D134410: [clang][CodeGen] Add noundef metadata to load instructions (preliminary 1 or 5)

[Nuno Lopes via Phabricator via cfe-commits]

nlopes added a comment.

In D134410#3983684 <https://reviews.llvm.org/D134410#3983684>, @nikic wrote:

> The other problem I see here is that we still need to do something about the memcpy -> load/store fold. As soon as we have poison from uninit values (either directly or via `!uninit_is_poison`) this will start causing miscompiles very quickly. The only way to do this right now is again with an `<N x i8>` vector load/store, which still optimizes terribly. This needs either load+store of integer to preserve poison, or again some form of byte type.
>
> Something I've only recently realized is that we also run into this problem when inserting spurious load/store pairs, e.g. as done by LICM scalar promotion. If we're promoting say an i32 value to scalar that previously used a conditional store, then promotion will now introduce an unconditional load and store, which will spread poison from individual bytes. So that means that technically scalar promotion (and SimplifyCFG store speculation) also need to do some special dance to preserve poison. And without preservation of poison across load/store/phi in plain ints, this is going to be a bad optimization outcome either way: We'd have to use either a vector of i8 or a byte type for the inserted phi nodes and only cast to integer and back when manipulating the value, which would probably defeat the optimization. At that point probably best to freeze the first load (which again needs a freezing load).

For this stuff, I think the ideal solution is the byte type. That's the only way to do this kind of raw byte copies. Doesn't spread poison between bits nor do you need to know if you are reading a pointer or something else. Nor does it require a freeze, which is annoying.

John will submit a proposal (next week?) using poison for uninit loads. I think we have converged on something cool. Thanks for insisting on that one!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134410