[PATCH] D41689: [SCEVAA] Don't crash on pointers with no dominance relationship.

[Hal Finkel via Phabricator via llvm-commits]

hfinkel added a comment.

In https://reviews.llvm.org/D41689#970310, @efriedma wrote:

> > If we wanted to handle this somehow in SCEVAAResult::alias, can't we just put a dominance check on the values directly instead of trying to examine the SCEVs?
>
> Oh.  Yes, that would be simpler. :)  Maybe slightly less powerful, depending on the structure of the code.
>
> > I think that what AliasSetPrinter is doing is just not well defined
>
> In this case, it's probably worth noting that I have a testcase which doesn't involve aa-eval; BasicAA also makes queries like this (see BasicAAResult::aliasPHI).  I can reduce it if that would be interesting.


I'd find that interesting.

In general, I think that we need to decide what the interface contract here is. I had thought that we required a dominance relationship because there had to be at least on well-defined point where both values could be simultaneously evaluated in order to produce a well-defined result. This might be too strict. I can imagine saying something along these lines but allowing for some kind of hypothetical PHI translation (i.e. saying that the values can be compared if there could exist some series of well-defined PHIs that would bring the values together under at least one valid path through the CFG (albeit under different names). I get a little worried here in defining how this works in the face of backedges (because I need alias (%a, %b) to, say, compare in the current loop iteration, not %b from some loop iteration against %a hypothetically PHI-translated into some other iteration). Our BasicAA::aliasPHI, as I believe we discovered when investigating some bug involving a use of ValueTracking, is somewhat-fundamentally broken in some related sense, so I find your comment unsurprising in that regard.

In any case, I'd be fine with doing a simple dominance check in SCEVAA and working on a separate patch to clean up the docs about what AA means and then trying to clean up everything else.


Repository:
  rL LLVM

https://reviews.llvm.org/D41689