kuhar added a comment.
Found some cosmetics, but I'm not familiar enough with the NPM to do a full
review.
================
Comment at: llvm/include/llvm/Passes/StandardInstrumentations.h:99
struct CFG {
struct BBGuard final : public CallbackVH {
BBGuard(const BasicBlock *BB) : CallbackVH(BB) {}
----------------
Consider pulling this out of CFG. I don't see many reasons for having 3 levels
of class nesting.
================
Comment at: llvm/include/llvm/Passes/StandardInstrumentations.h:134
public:
static cl::opt<bool> VerifyPreservedCFG;
----------------
not necessary anymore
================
Comment at: llvm/lib/Passes/StandardInstrumentations.cpp:728
+ Result run(Function &F, FunctionAnalysisManager &AM) {
+ return Result(&F, true /* TrackBBLifetime */);
+ }
----------------
nit: move parameter name to the left?
================
Comment at: llvm/lib/Passes/StandardInstrumentations.cpp:759
+ CFG::printDiff(dbgs(), GraphBefore, GraphAfter);
+ report_fatal_error(Twine("CFG unexpectedly changed by ", Pass));
+ };
----------------
I think this will print with `errs()`. Would it make sense to flush `dbgs()`
ahead of printing with `errs()`?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D91327/new/
https://reviews.llvm.org/D91327
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
