[PATCH] D85109: [NewPM] Clear abandoned analyses set when preserveSet is called

Yuanfang Chen via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Aug 6 16:55:53 PDT 2020


ychen added inline comments.


================
Comment at: llvm/include/llvm/IR/PassManager.h:192
+  /// Regular passes should not call this.
+  /// FIXME: rename this to `preserveSetAndClearAbandonedAnalyses`?
   template <typename AnalysisSetT> void preserveSet() {
----------------
asbirlea wrote:
> asbirlea wrote:
> > I'd be fine with renaming this with this mouthful name. Not just because it's more informative, but it's more likely to remain internally used, or easy to catch if it's not. I don't see how we can assert/enforce this easily.
> Actually this doesn't look quite right. It's clearing all analyses, for all IR units.
> After talking with chandlerc@, a better approach would be clearing in `invalidate`. I'm looking into the alternative approach.
Agree. Doing it in `invalidate` works too. Any reason clearing all analyses, for all IR units is wrong?

The scenario I was thinking are

  /// - (abandoned inner analyses) the invalidation has happened in inner IR
  ///   manager, no need to let the outer manager know and they should not care.
  ///   If abandoned inner analyses were returned, it may trigger another round
  ///   of inner analyses invalidation from outer manager through the proxy
  ///   (InnerAnalysisManagerProxy).
  /// - (abandoned outer analyses) abandoning has the same effect of not
  ///   preserving it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85109



More information about the llvm-commits mailing list