[PATCH] D86979: [docs][NewPM] Add docs for writing NPM passes

[Yuanfang Chen via Phabricator via llvm-commits]

ychen added a comment.

Thanks for getting this started.



================
Comment at: llvm/docs/WritingAnLLVMNewPMPass.rst:19
+
+Unlike passes under the legacy pass manager where the pass interface was
+defined via inheritance, passes under the new pass manager rely on template
----------------
Not an English grammar expert. But it feels to me the `was` should `is`?


================
Comment at: llvm/docs/WritingAnLLVMNewPMPass.rst:21
+defined via inheritance, passes under the new pass manager rely on template
+metaprogramming. All LLVM passes inherit from the CRTP mix-in
+``PassInfoMixin<PassT>``. The pass should have a ``run()`` method which
----------------
As mentioned in the comments on top of PassManager.h, I think we could say there is no interface for pass since the NPM is adopting concept-based polymorphism where not using inheritance is a goal. Maybe add a reference to https://sean-parent.stlab.cc/papers-and-presentations/#value-semantics-and-concept-based-polymorphism


================
Comment at: llvm/docs/WritingAnLLVMNewPMPass.rst:47
+
+First, configure and build LLVM (just ``opt`` is necessary for now).
+
----------------
Do we need this "(just ``opt`` is necessary for now)"? If I'm new to LLVM, I would find this a little bit distracting. It does not hurt anything though.


================
Comment at: llvm/docs/WritingAnLLVMNewPMPass.rst:90
+  public:
+    HelloWorldPass() {}
+
----------------
Could we delete this?


================
Comment at: llvm/docs/WritingAnLLVMNewPMPass.rst:97
+
+  #endif
+
----------------
#endif // LLVM_TRANSFORMS_HELLO_HELLOWORLD_H


================
Comment at: llvm/docs/WritingAnLLVMNewPMPass.rst:99
+
+This creates the class for the pass with a default constructor, and a
+declaration of the ``run()`` method which actually runs the pass. Inheriting
----------------
Extra comma. Or if we delete the ctor, we could also delete the wording here?


================
Comment at: llvm/docs/WritingAnLLVMNewPMPass.rst:113
+
+... to include the header file we just created.
+
----------------
Is the ... here intentional?

Looking at https://llvm.org/docs/WritingAnLLVMPass.html makes me think it was a typo.


================
Comment at: llvm/docs/WritingAnLLVMNewPMPass.rst:132
+
+... which simply prints out the name of the function to stderr. The pass
+manager will ensure that the pass will be run on every function in a module.
----------------
ditto


================
Comment at: llvm/docs/WritingAnLLVMNewPMPass.rst:145
+
+... which adds the pass under the name "helloworld".
+
----------------
ditto


================
Comment at: llvm/include/llvm/Transforms/HelloNew/HelloWorld.h:9
+
+#ifndef LLVM_TRANSFORMS_HELLO_HELLOWORLD_H
+#define LLVM_TRANSFORMS_HELLO_HELLOWORLD_H
----------------
LLVM_TRANSFORMS_HELLONEW_HELLOWORLD_H


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86979