[PATCH] D86669: [ValueTracking] Remove MaxLookup from getUnderlyingObjects

Mon Jun 6 12:19:35 PDT 2022

vitalybuka added a comment.

In D86669#3561225 <https://reviews.llvm.org/D86669#3561225>, @efriedma wrote:

> In D86669#3561125 <https://reviews.llvm.org/D86669#3561125>, @vitalybuka wrote:
>
>> In D86669#3560983 <https://reviews.llvm.org/D86669#3560983>, @efriedma wrote:
>>
>>> If you're seeing a miscompile, something is probably wrong with the caller.  getUnderlyingObjects() is, in general, not guaranteed to produce an identifiable object.  If the caller cares, it should check; for example, GlobalsAAResult::getModRefInfoForArgument checks `all_of(Objects, isIdentifiedObject)`.
>>
>> I am not sure why do don't want to make it guaranty with a patch like this?
>
> How could we possibly guarantee that?  In general, we're going to find some opaque thing that both getUnderlyingObjects() and its caller can't understand.
>
> I mean, I guess we could define an API that narrowly guarantees it looks though all bitcast, addrspacecast, gep, phi, and select operations.  But that would be expensive in general, and it's not obvious to me it solves anything.

Right, I totally forgot the point of the patch :)
I still think removing MaxLookup is good, and simplifies callers side.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86669