[PATCH] D64405: [DependenceAnalysis] Dependecies for loads marked with "ivnariant.load" should not be shared with general accesses. Fix for https://bugs.llvm.org/show_bug.cgi?id=42151

Johannes Doerfert via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jul 9 22:01:40 PDT 2019


jdoerfert added a comment.

In D64405#1577614 <https://reviews.llvm.org/D64405#1577614>, @ebrevnov wrote:

> In D64405#1576608 <https://reviews.llvm.org/D64405#1576608>, @jdoerfert wrote:
>
> > Why do we need both changes, reading the cache and writing it, assuming I understand the code correctly?
> >  I would have assumed not caching "inv-load" results would fix the problem. And I can see we do not want to reuse
> >  non-inv-load results for inv-loads. However, I would argue one could always check for a cached result and use
> >  it if the result is "good". Do I miss something here?
>
>
> Yes, you are reading the change correctly. The thing is that both results (one for inv-load case another for non inv-load) are "good". They are just different. Each result should be used for corresponding case only and not for the other. In other words using result from non-inv case for inv-case is not correct as well. Does it make sense?


No, but I'm tired. I would have assumed that I can drop the `inv-load` metadata without semantic change. If that is the case, I should be able to use the `non-inv` result if it is cached and already good enough. That would mean we can keep the lookup but do only return the cached result if it is "the best". Maybe my problem is that I do not know the lattice of potential values.

Either way, is it possible and useful to test for the number of (un)cached queries?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D64405





More information about the llvm-commits mailing list