[PATCH] D69416: [Examples] Add IRTransformations directory to examples.

[Andrzej Warzynski via Phabricator via llvm-commits]

andwar added a comment.

I think that having a reference to these examples somewhere in the official docs would be very helpful. Maybe somewhere here: http://llvm.org/docs/WritingAnLLVMPass.html?

[nit] I'd keep the names of files and passes shorter and remove 'Tutorial' from names. IMHO it doesn't really add much in terms of self-documentation, yet makes everything a bit harder to read.



================
Comment at: llvm/examples/IRTransforms/TutorialSimplifyCFG.cpp:17
+#include "llvm/IR/PassManager.h"
+
+
----------------
[nit] Unnecessary empty line.


================
Comment at: llvm/examples/IRTransforms/TutorialSimplifyCFG.cpp:54
+      Succ->removePredecessor(&BB);
+    // Alternatively, we can update the PHI nodes manually:
+    // for (PHINode &PN : make_early_inc_range(Succ->phis()))
----------------
Commented out code is most likely to become stale - I'd remove it.


================
Comment at: llvm/examples/IRTransforms/TutorialSimplifyCFG.cpp:58
+
+    // Replace all instructions in BB with a undef constant. The block is
+    // unreachable, so the results of the instructions should never get used.
----------------
[nit] 'a undef' --> 'an undef'


================
Comment at: llvm/examples/IRTransforms/TutorialSimplifyCFG.cpp:95
+      // Alternatively, we can update the PHI nodes manually:
+      // for (PHINode &PN : make_early_inc_range(Succ->phis()))
+      //  PN.removeIncomingValue(&BB);
----------------
Same as above.


================
Comment at: llvm/examples/IRTransforms/TutorialSimplifyCFG.cpp:140
+
+// Eliminate branches with constant conditionals. This is the second first
+// version, which *does* preserve the dominator tree.
----------------
'second first' --> 'first'


================
Comment at: llvm/examples/IRTransforms/TutorialSimplifyCFG.cpp:176
+
+// Eliminate branches with constant conditionals. This is the second third
+// version, which use PatternMatch.h.
----------------
'second third' -> 'third'?


================
Comment at: llvm/examples/IRTransforms/TutorialSimplifyCFG.cpp:177
+// Eliminate branches with constant conditionals. This is the second third
+// version, which use PatternMatch.h.
+static bool eliminateCondBranches_v3(Function &F, DominatorTree &DT) {
----------------
[nit] 'use' -> 'uses'


================
Comment at: llvm/examples/IRTransforms/TutorialSimplifyCFG.h:17
+
+void initializeTutorialSimplifyCFGLegacyPassPass(PassRegistry &);
+
----------------
[nit] Did you mean `initializeTutorialSimplifyCFGLegacyPMPass` instead?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69416