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

[Vitaly Buka via Phabricator via llvm-commits]

vitalybuka added a comment.

In D86669#2836130 <https://reviews.llvm.org/D86669#2836130>, @fhahn wrote:

>> getUnderlyingObjects already traverse large set on instructions so
>> there is no reason to limit some sequences by MaxLookup.
>
> I am not sure I completely follow this reasoning. Even if it already traverses a large number of instructions, without the depth limit it will visit even more? Do you have any estimate on the impact in terms of extra work?

On real program almost always both version will do the same work and default MaxLookup will not be reached.

However there are possible two edge-cases with large number of nodes:
a) Long chain of N values:

  a = b, b = c, c = d .... = underlying_object

b) Wide graph with total N values:

  a = (a, b, c ....) , a = (a1, a2, ...), b = (b1, b2, ...)...

Existing algorithm will fail on "a" but exits in constant time, it may be able discover all objects on b (if it's not too deep), and but in O(N) time.
Patched algorithm discover all objects, in O(N) in both cases.

So my point is if we accept O(N) for b) why not accept O(N) for a) and let callers avoid thinking about MaxLookup.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86669