[PATCH] D144274: [InstCombine] use loop info when running the pass after loop vectorization

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

I've commented with my understanding of the current behavior (excluding cases where analyses are "accidentally" preserved). TBH I'm not entirely sure what to do here. I don't think the LoopVectorize tests had any particular intention (just used whatever the behavior was).

For D144045 <https://reviews.llvm.org/D144045>, I guess we only need LoopInfo for the very last InstCombine run after LICM. But for the GEP swapping fold, we need it before LICM. Probably to be robust, that part shouldn't be performed by InstCombine at all and done by LICM itself (i.e. reassociate if it enables LICM).



================
Comment at: llvm/lib/Passes/PassBuilderPipelines.cpp:1116
   // Cleanup after the loop optimization passes.
-  FPM.addPass(InstCombinePass());
+  FPM.addPass(InstCombinePass(InstCombineOptions().setUseLoopInfo(true)));
 
----------------
Looks right: LoopVectorize requires LoopInfo and LoopUnroll, SROA will preserve it.


================
Comment at: llvm/lib/Passes/PassBuilderPipelines.cpp:1129
+    ExtraPasses.addPass(
+        InstCombinePass(InstCombineOptions().setUseLoopInfo(true)));
     LoopPassManager LPM;
----------------
Looks right: EarlyCSE and CVP are CFG-preserving, and we still have LoopInfo from above.


================
Comment at: llvm/lib/Passes/PassBuilderPipelines.cpp:1143
+    ExtraPasses.addPass(
+        InstCombinePass(InstCombineOptions().setUseLoopInfo(true)));
     FPM.addPass(std::move(ExtraPasses));
----------------
Doesn't look right: SimplifyCFG invalidates LoopInfo.


================
Comment at: llvm/lib/Passes/PassBuilderPipelines.cpp:1166
     FPM.addPass(SCCPPass());
-    FPM.addPass(InstCombinePass());
+    FPM.addPass(InstCombinePass(InstCombineOptions().setUseLoopInfo(true)));
     FPM.addPass(BDCEPass());
----------------
Doesn't look right: SimplifyCFG invalides LoopInfo and SCCP doesn't require it.


================
Comment at: llvm/lib/Passes/PassBuilderPipelines.cpp:1181
   if (!IsFullLTO) {
-    FPM.addPass(InstCombinePass());
+    FPM.addPass(InstCombinePass(InstCombineOptions().setUseLoopInfo(true)));
     // Unroll small loops to hide loop backedge latency and saturate any
----------------
Same here, nothing has requested LoopInfo since.


================
Comment at: llvm/lib/Passes/PassBuilderPipelines.cpp:1204
     FPM.addPass(SROAPass(SROAOptions::PreserveCFG));
-    FPM.addPass(InstCombinePass());
+    FPM.addPass(InstCombinePass(InstCombineOptions().setUseLoopInfo(true)));
     FPM.addPass(
----------------
Looks right, LoopUnroll requested LoopInfo, SROA preserved.


================
Comment at: llvm/lib/Passes/PassBuilderPipelines.cpp:1218
   if (IsFullLTO)
-    FPM.addPass(InstCombinePass());
+    FPM.addPass(InstCombinePass(InstCombineOptions().setUseLoopInfo(true)));
 }
----------------
Looks right, AlignmentFromAssumptions requires LoopInfo.


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

https://reviews.llvm.org/D144274