[PATCH] D82293: [CodeMoverUtils][WIP] Move code motion related checks from LICM to CodeMoverUtils

[Whitney Tsang via Phabricator via llvm-commits]

Whitney added inline comments.


================
Comment at: llvm/include/llvm/Analysis/LoopAnalysisManager.h:48
+class PostDominatorTree;
+class DependenceInfo;
 
----------------
nit: follow alphabetic order. 


================
Comment at: llvm/include/llvm/Analysis/LoopAnalysisManager.h:163
 PreservedAnalyses getLoopPassPreservedAnalyses();
-}
+} // namespace llvm
 
----------------
recommend not to do formatting changes on lines not changed by you. 
This is a general comment to all your changes, this can make your patch smaller.


================
Comment at: llvm/lib/Transforms/Scalar/LICM.cpp:263
   LoopInvariantCodeMotion LICM(LicmMssaOptCap, LicmMssaNoAccForPromotionCap);
   if (!LICM.runOnLoop(&L, &AR.AA, &AR.LI, &AR.DT, &AR.TLI, &AR.TTI, &AR.SE,
+                      AR.MSSA, &ORE, &AR.PDT, &AR.DI))
----------------
RithikSharma wrote:
> @Whitney I'm not sure is it okay to do all these deep changes but they all exist because this LICM invocation requires analysis from AnalysisResult. I request your comment on this. 
Can you please elaborate what you mean by `deep changes`? Do you mean adding adding PDT and  DI in AR? 
We should consider extending CodeMover utilities to take in more analyses, where some of them can be optional, and depending on the available analysis do the best judgement. 
Or if MSSA can be used to cover all cases currently covered on CodeMover, then we should change CodeMover to use MSSA. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82293