Let CorrelatedValuePropagation preserve LazyValueInfo

[aqjune]

Summary

This patch makes CorrelatedValuePropagation preserve LazyValueInfo by adding LazyValueInfo::eraseValue & calling it whenever an instruction is erased.

Passes make check , test-suite, and SPECrate 2017.

Patch by aqjune (Juneyoung Lee)

[reames]

lib/Transforms/Scalar/CorrelatedValuePropagation.cpp

122 ↗

(On Diff #190573)

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?

815 ↗

(On Diff #190573)

Have you checked the history? I believe that CVP used to preserve LVI. Why was that removed?

[reames]

Marking to remove from review queue. When replying to questions, please mark as ready for review.

[aqjune]

Removes eraseValue() calls

[aqjune]

lib/Transforms/Scalar/CorrelatedValuePropagation.cpp

122 ↗

(On Diff #190573)

You were right, I checked that these items were automatically removed from LazyValueInfoCache. I removed eraseValue() calls.

815 ↗

(On Diff #190573)

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 .

[aqjune]

I couldn't find ready for review option from anywhere :( 'Add Action...' dropdown menu contains Plan Changes and Abandon Revision only.

[reames]

LGTM. This may be slightly imprecise - for instance, when we fold an operand which allows further LVI analysis - but the preservation is conservatively correct.

p.s. Updating a review resets the status.

[aqjune]

Thanks! :)
I don't have commit access; if you don't mind, could you commit this patch instead please?

[xbolva00]

Please rebase

[aqjune]

Sorry, I was pretty busy last week.
@xbolva00 Rebased

[xbolva00]

In D59349#1595877, @aqjune wrote:

Sorry, I was pretty busy last week.
@xbolva00 Rebased

Commited, r366942.

llvm/trunk/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp

public:
 
     bool runOnFunction(Function &F) override;
 
     void getAnalysisUsage(AnalysisUsage &AU) const override {
       AU.addRequired<DominatorTreeWrapperPass>();
       AU.addRequired<LazyValueInfoWrapperPass>();
       AU.addPreserved<GlobalsAAWrapperPass>();
       AU.addPreserved<DominatorTreeWrapperPass>();
+      AU.addPreserved<LazyValueInfoWrapperPass>();
     }
   };
 
 } // end anonymous namespace
 
 char CorrelatedValuePropagation::ID = 0;
 
 INITIALIZE_PASS_BEGIN(CorrelatedValuePropagation, "correlated-propagation",
CorrelatedValuePropagationPass::run(Function &F, FunctionAnalysisManager &AM) {
 
   bool Changed = runImpl(F, LVI, DT, getBestSimplifyQuery(AM, F));
 
   if (!Changed)
     return PreservedAnalyses::all();
   PreservedAnalyses PA;
   PA.preserve<GlobalsAA>();
   PA.preserve<DominatorTreeAnalysis>();
+  PA.preserve<LazyValueAnalysis>();
   return PA;
 }