[PATCH] D136055: [ValueTracking] Make !range metadata imply noundef for load & call results

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev added a comment.

In D136055#3865543 <https://reviews.llvm.org/D136055#3865543>, @nikic wrote:

> In D136055#3865454 <https://reviews.llvm.org/D136055#3865454>, @mkazantsev wrote:
>
>> In some sense, "range implies noundef" is the same thing as "dereferenceable implies noundef". There is no conceptual difference between "integer in specified range" and "pointer in a set of valid pointers". If we do it for dereferenceable pointers, why impede this for ranges?
>
> The relevant difference in how this metadata gets used: APIs using range metadata generally only prove facts modulo poison. If computeKnownBits() says that the top bits of the value are zero, what this really means is that either the top bits are zero or the value is poison. The same is not true for dereferenceable metadata: This metadata is used to speculate loads, and having a "dereferenceable or poison" pointer would be useless in that context.

I'm still not getting this. The code I'm modifying says "dereferenceable pointers don't have to be marked as noundef, but we can still infer noundef from the fact they are dereferenceable". But at the same time we say "loads with range *have* to be marked as noundef explicitly, and we don't want to imply it". This approach is self-contradictory. If you insist that frontend must mark every load with range as !noundef, I'm fine with that. But in this case let's delete

    I->hasMetadata(LLVMContext::MD_dereferenceable) ||
    I->hasMetadata(LLVMContext::MD_dereferenceable_or_null))
  return true;

and demand that they are also marked as !noundef explicitly. It should be same for them.


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

https://reviews.llvm.org/D136055