[PATCH] D59349: Let CorrelatedValuePropagation preserve LazyValueInfo

Juneyoung Lee via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Mar 23 21:47:34 PDT 2019


aqjune marked 3 inline comments as done.
aqjune added inline comments.


================
Comment at: lib/Transforms/Scalar/CorrelatedValuePropagation.cpp:122
   S->replaceAllUsesWith(ReplaceWith);
+  LVI->eraseValue(S);
   S->eraseFromParent();
----------------
reames wrote:
> I'm confused why we need this.  LVI uses ValueHandles, which should delete the entry from the cache once it's destroyed.  Do you have a test case where that is not happening?
You were right, I checked that these items were automatically removed from LazyValueInfoCache. I removed `eraseValue()` calls.


================
Comment at: lib/Transforms/Scalar/CorrelatedValuePropagation.cpp:815
   PA.preserve<DominatorTreeAnalysis>();
+  PA.preserve<LazyValueAnalysis>();
   return PA;
----------------
reames wrote:
> Have you checked the history?  I believe that CVP used to preserve LVI.  Why was that removed?
I checked that `LazyValueAnalysis` had not been added in the past. `DominatorTreeAnalysis` was added at https://reviews.llvm.org/D48059 , GlobalsAA was added at https://llvm.org/svn/llvm-project/llvm/trunk@274705 .


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

https://reviews.llvm.org/D59349





More information about the llvm-commits mailing list