[PATCH] D91327: [NewPM] Redesign of PreserveCFG Checker

[Jakub Kuderski via Phabricator via cfe-commits]

kuhar added a comment.

Just two nits from me. I think it looks fine, but I'm not familiar with the new pass manager and don't feel confident enough to approve it.



================
Comment at: llvm/include/llvm/Passes/StandardInstrumentations.h:134
 
 public:
   static cl::opt<bool> VerifyPreservedCFG;
----------------
yrouban wrote:
> kuhar wrote:
> > yrouban wrote:
> > > kuhar wrote:
> > > > not necessary anymore
> > > there can bee a need to disabled/enable (e.g. for some tests or for debugging).
> > I meant the 'public:'. You made everything public at the very top of the class.
> sure
I don't think this got fixed.


================
Comment at: llvm/lib/Passes/StandardInstrumentations.cpp:1096
+      checkCFG(P, F->getName(), *GraphBefore,
+               CFG(F, false /* TrackBBLifetime */));
   });
----------------
nit: can you put the commented argument name before the argument?


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

https://reviews.llvm.org/D91327