nikic added a comment.

In D134410#3895253 <https://reviews.llvm.org/D134410#3895253>, @nlopes wrote:

> In D134410#3895077 <https://reviews.llvm.org/D134410#3895077>, @nikic wrote:
>
>> In D134410#3894995 <https://reviews.llvm.org/D134410#3894995>, @nlopes wrote:
>>
>>> We wanted this patch to make us switch uninitialized loads to poison at 
>>> will, since they become UB. In practice, this helps us fixing bugs in SROA 
>>> and etc without perf degradation.
>>
>> Can you elaborate on this? I don't see how this is necessary for switching 
>> uninitialized loads to poison.
>
> It's not mandatory, it's a simple way of achieving it as !noundef already 
> exists.
>
> We cannot change the default behavior of load as it would break BC.

FWIW, I don't agree with this. It's fine to change load semantics, as long as 
bitcode autoupgrade is handled correctly. I'd go even further and say that at 
least long term, any solution that does not have a plain `load` instruction 
return `poison` for uninitialized memory will be a maintenance mess.

I think the core problem that prevents us from moving in this direction is that 
we have no way to represent a frozen load at all (as `freeze(load)` will 
propagate poison before freezing). If I understand correctly, you're trying to 
solve this by making *all* loads frozen loads, and use `load !noundef` to allow 
dropping the freeze. I think this would be a very bad outcome, especially as 
we're going to lose that `!noundef` on most load speculations, and I don't 
think we want to be introducing freezes when speculating loads (e.g. during 
LICM).

I expect that the path of least resistance is going to be to support bitwise 
poison for load/store/phi/select and promote it to full-value poison on any 
other operation, allowing a freezing load to be expressed as `freeze(load)`.

Please let me know if I completely misunderstood the scheme you have in mind...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134410

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to