[PATCH] D108244: [LICM] Remove AST-based implementation

Alina Sbirlea via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 18 12:36:44 PDT 2021


asbirlea added a comment.

First, let's wait to make sure this cleanup sticks (I'm thinking of potential out of tree users that may be affected).

Next, I'd suggest trying to measure performance for the following changes:

- change legacy LoopSink to be a Function pass, processing loops in the same order (preorder, same logic as NPM and as the legacy loop PM)
- make MSSA not a required analysis, but make it preserved in AU.
- get the analysis if available, if yes, use it and preserve it (do not compute the AST).
- if MSSA is not present, before the actual loop processing check if any of the loops has profile data. If so, compute a MemorySSA instantiation for the whole function (`new MemorySSA(F, &AA, &DT)`) and use that. MemorySSA will not be available for the next passes with this approach. There's currently no option to compute MemorySSA for a single loop and it's not something we plan to support at this time.

This testing would be a good data point on the type of benchmarks in the test-suite. The regressions will remain for functions with few and small loops with profile info.

The options to move forward are either wait for the removal of the legacy PM, or do the cleanup anyway and accept the regression earlier.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108244



More information about the llvm-commits mailing list