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

[Florian Hahn via Phabricator via llvm-commits]

fhahn marked an inline comment as done.
fhahn added inline comments.


================
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()))
----------------
andwar wrote:
> Commented out code is most likely to become stale - I'd remove it.
Yes, ideally it should be also uncommented, but I am not entirely sure yet how to best integrate such small variations. I'd like to leave it in, with a todo for now.


================
Comment at: llvm/examples/IRTransforms/TutorialSimplifyCFG.cpp:375
+
+  if (!doSimplify_v2(F, DT))
+    return PreservedAnalyses::all();
----------------
paquette wrote:
> Is there any reason that we don't check the version like in the legacy `runOnFunction`?
Nope, I just missed it! Updated. Although the pass still needs to be hooked up properly so the new PM actually knows about it. I've added a TODO


================
Comment at: llvm/examples/IRTransforms/TutorialSimplifyCFG.h:17
+
+void initializeTutorialSimplifyCFGLegacyPassPass(PassRegistry &);
+
----------------
andwar wrote:
> [nit] Did you mean `initializeTutorialSimplifyCFGLegacyPMPass` instead?
Nope, the INITIALIZE_PASS macros will add a `Pass` at the end of the pass class name.


================
Comment at: llvm/tools/opt/opt.cpp:543-545
+#ifdef BUILD_EXAMPLES
+  initializeExampleIRTransforms(Registry);
+#endif
----------------
Meinersbur wrote:
> I'd prefer if the production code stays distinct from examples. In this case D61446 would allow a solution.
I think we should move to the mechanism, once it lands. In the meantime, I think having that in opt should be low risk, as opt is mostly used for testing anyways and the example passes are completely separated from the transforms library.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69416