[PATCH] D28137: [Devirtualization] MemDep returns non-local !invariant.group dependencies

Philip Reames via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Dec 30 12:34:37 PST 2016


reames added a comment.

In https://reviews.llvm.org/D28137#632801, @Prazek wrote:

> In https://reviews.llvm.org/D28137#632782, @reames wrote:
>
> > The use of a side cache here appears unnecessary.  Why can't you simply return the non-local result to the immediate caller and let it be kept in a local unless needed?
>
>
> What do you mean by "kept in local"? Do you mean storing one def as a member of MemDep?
>
> > Also, you have a problem with the quality of your results.  You're returning the first non-local found, not the best non-local found.  This makes the results use list order dependent.  We generally try not to have use list order sensitivities in the optimizer.
>
> But this doesn't matter to invariant.group. Let say that I have 2 loads with invariant group, one local and second non-local
>
>     %0 load %a !invariant.group !0
>     call void foo(%a)
>     br b1
>   
>   b1:
>     %1 = load %a !invariant.group !0
>     call void foo(%a)
>     %2 = load %a !invariant.group !0
>   
>   
>
> There is guarantee that %0, %1 and %2 will load the same value. If I will ask for dependency about %2, then whenever I will get
>  def(%1) or def(%0), they will be transformed into either %0 or %1, but assuming we will query %1, then it will be changed into %0,
>  which means that we will end up with %0 everywhere.
>  This is assuming that GVN is sane.


I understand your argument that after optimizing to a fixed point, we should get the same result.  The problem is that the *order* of iteration will differ.  This can mean differences in naming, memory allocation patterns, or even output (I don't believe GVNPRE actually iterates to a fixed point.)

> I can continue looping and either try hard to find local dependency or 'the best' non-local dependency (which probably won't be found because GVN will already remove it)
>  and return it, or even collect list of all non local dependencies.

The key point is that we should return a consistent result, regardless of which query path we see.  Returning the most dominating non-local would be one reasonable scheme.

> I am also not sure if I understand what 'the best' non local dependency means.

Don't get too caught up on "best" here.  The ordering point above is the primary concern.  A secondary concern is reducing the number of iterations required to reach the fixed point.  Why do multiple full scans if we can bypass all but one with a small amount of extra work?  Particularly work that we know only happens when we have found a useful result and are just trying to find a better one?  (i.e. we're not burning time when we're not making progress.)



================
Comment at: lib/Analysis/MemoryDependenceAnalysis.cpp:417
+
+        // Def(U) can't be returned here because it is non-local. If local
+        // dependency won't be found then return nonLocal counting that the
----------------
Ok, my comment on the previous round was unclear and probably misleading.  My primary concern is the use of a side structure to propagate results from one call to the next.  This introduces a bunch of complexity (do we need cache invalidation?, are we leaking memory?) which I'd like to avoid if we can. 

Can we structure this such that we split this function and call it once for local and once for non-local?  I know this will do strictly more work, but I'd prefer that over the code complexity.


https://reviews.llvm.org/D28137





More information about the llvm-commits mailing list