[PATCH] D100912: [docs][NewPM] Add section on analyses

[Arthur Eubanks via Phabricator via llvm-commits]

aeubanks added inline comments.


================
Comment at: llvm/docs/NewPassManager.rst:147
+LLVM provides many analyses that passes can use, such as a dominator tree.
+Calculating these can be expensive, so the new pass manager has
+infrastructure to cache analyses and reuse them when possible.
----------------
mtrofin wrote:
> So does the LPM. We could say "similar to the legacy pass manager, the new pass manager has...". I'd add "there are differences, though, in how passes communicate to the infrastructure to ensure the validity of the cached values"
I'm not explicitly trying to compare the new PM to the legacy PM in this doc. The couple points in this doc about the legacy PM are either about removing it in the future, or quick API differences for people who don't want to learn about the new PM in detail and just want to port things quickly


================
Comment at: llvm/docs/NewPassManager.rst:239
+
+An analysis should implement ``invalidate()`` like below, at least for simple
+cases:
----------------
mtrofin wrote:
> but for simple cases that's not necessary, the default impl does that, no?
There's no default implementation?
Each analysis's `Result` type needs to implement `invalidate()`. Although that does bring up a good point that it's specifically the result that implements it, not the analysis interface. Will clarify.


================
Comment at: llvm/docs/NewPassManager.rst:293
+for more details.
+
 Status of the New and Legacy Pass Managers
----------------
mtrofin wrote:
> I'd recommend adding a subsection with guidelines for analysis authors, and one for consumers of analyses (i.e. pass authors), distilling the design into actionable DO/DON'T statements. Like "don't implement invalidate() and return false, unless you really mean to develop an immutable analysis*)
> 
> *assuming we fix the issues of bulk-invalidation by the likes of cgscc AM, but I digress :)
> 
good point, this is mixing users and implementors of analyses, I'll try to make the distinction better


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100912