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

Piotr Padlewski via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Dec 30 13:02:29 PST 2016


Prazek added a comment.

Hmm, shouldn't it be the least dominating instead? Then we will prefer local dependencies instead of non-local.
I never faced the problem with iterations (in GVN or anywhere else), so I will trust you that it will be valuable to fix it. 
I am not sure if it is valuable to fix the second issue.



================
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
----------------
reames wrote:
> 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.
Yes, I totally agree that this is hacky solution. I don't see a simple solution to solve it expect to do high refactor in MemDep. The main problem here is that all the users of memdep expect to get local results first, and then ask for nonlocal dependencies. 

I firstly tried to change Dep into LocalDep and add NonLocalDep, but then I found that I would have to see all the usages of MemDep, understand all the algorithms to make sure everything is still valid.

Other way would be to run the same algorithm second time in nonlocal, but this would make even less sense than the solution we have today.

There is also strong argument imho to leave it as it - MemDep will is being deprecated, so there is plan to remove it in 6-12 months. Because NewGVN is still not there, I would like to make this small hack so clang-4.0 will be able to do much better devirtualization.

Then I am planning to implement handling of invariant.group in MSSE and assume in NewGVN, but this will probably take me while.


https://reviews.llvm.org/D28137





More information about the llvm-commits mailing list