[PATCH] D79385: NFC: Simplify O1 pass pipeline construction.

[Chandler Carruth via Phabricator via llvm-commits]

chandlerc added a comment.

As much as I'm not thrilled about the duplication, the number of comments I have below about the exact O1 <https://reviews.llvm.org/owners/package/1/> pipeline sure make it seem valuable. Unsure how you feel about addressing these here, later, etc.



================
Comment at: llvm/lib/Passes/PassBuilder.cpp:435
+
+  // TODO: Investigate the cost/benefit of tail call elimination on debugging.
+  FPM.addPass(SimplifyCFGPass());
----------------
Mapbe pull to the function comment? Doesn't seem especially relevant here given the other `SimplifyCFGPass` runs.


================
Comment at: llvm/lib/Passes/PassBuilder.cpp:436
+  // TODO: Investigate the cost/benefit of tail call elimination on debugging.
+  FPM.addPass(SimplifyCFGPass());
+
----------------
I find the sequence simplify-cfg -> instcombine -> libcall-shrink-wrap -> simplify-cfg a bit odd... Does the shrink wrap pass require running after instcombine? If not, maybe do libcall-shrink-wrap -> simplify-cfg -> instcombine?


================
Comment at: llvm/lib/Passes/PassBuilder.cpp:444-451
+  // FIXME: Currently this is split into two loop pass pipelines because we run
+  // some function passes in between them. These can and should be removed
+  // and/or replaced by scheduling the loop pass equivalents in the correct
+  // positions. But those equivalent passes aren't powerful enough yet.
+  // Specifically, `SimplifyCFGPass` and `InstCombinePass` are currently still
+  // used. We have `LoopSimplifyCFGPass` which isn't yet powerful enough yet to
+  // fully replace `SimplifyCFGPass`, and the closest to the other we have is
----------------
Maybe O1 is a place to go ahead and addres what we can of this?


================
Comment at: llvm/lib/Passes/PassBuilder.cpp:515-518
+  // Run instcombine after redundancy and dead bit elimination to exploit
+  // opportunities opened up by them.
+  FPM.addPass(InstCombinePass());
+  invokePeepholeEPCallbacks(FPM, Level);
----------------
I'd skip this one at O1, FWIW.

we're going to run it again just after this. Also seems a bit redundant to run BDCE right before ADCE... but you already have a TODO down there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79385