[PATCH] D81558: [NewPM] Introduce PreserveCFG check

[Jakub Kuderski via Phabricator via llvm-commits]

kuhar added a comment.

This looks very useful. Does it currently report any errors when enabled?



================
Comment at: llvm/include/llvm/Passes/StandardInstrumentations.h:94
+
+  CFG(const Function *F, bool NeedsGuard = false);
+
----------------
nit: `NeedsGruard` sounds like an external property that you need to satisfy for some reason. In this case, it seems to me that it's just an extra level of validation that we may want, right? 
If that's the case, I'd consider renaming this to something like: `UseGuargs`, `GuardBasicBlocks`, `TrackBBLifetime`, etc.


================
Comment at: llvm/lib/Passes/StandardInstrumentations.cpp:43
+    "verify-cfg-preserved", cl::Hidden,
+#ifdef NDEBUG
+    cl::init(false));
----------------
How much overhead does this have? It it's not negligible, we may want to enable it when expensive checks are enabled.


================
Comment at: llvm/lib/Passes/StandardInstrumentations.cpp:396
+  const CFG *FuncGraph = nullptr;
+  if (After.Graph.size())
+    FuncGraph = &After;
----------------
nit: !empty() or size() > 0


================
Comment at: llvm/lib/Passes/StandardInstrumentations.cpp:398
+    FuncGraph = &After;
+  else if (!Before.isPoisoned() && Before.Graph.size())
+    FuncGraph = &Before;
----------------
same as above


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

https://reviews.llvm.org/D81558