yrouban added inline comments.
================
Comment at: llvm/include/llvm/Passes/StandardInstrumentations.h:134
public:
static cl::opt<bool> VerifyPreservedCFG;
----------------
kuhar wrote:
> not necessary anymore
there can bee a need to disabled/enable (e.g. for some tests or for debugging).
================
Comment at: llvm/lib/Passes/StandardInstrumentations.cpp:759
+ CFG::printDiff(dbgs(), GraphBefore, GraphAfter);
+ report_fatal_error(Twine("CFG unexpectedly changed by ", Pass));
+ };
----------------
kuhar wrote:
> I think this will print with `errs()`. Would it make sense to flush `dbgs()`
> ahead of printing with `errs()`?
I believe this comment is true for all callsites of //report_fatal_error()//.
If so it should be done inside.
================
Comment at: llvm/lib/Passes/StandardInstrumentations.cpp:771
+ *const_cast<Function *>(F)))
+ checkCFG(P, F->getName(), *Result, CFG(F, false /* TrackBBLifetime */));
else
----------------
skatkov wrote:
> why do you check before pass runs?
removed
================
Comment at: llvm/lib/Passes/StandardInstrumentations.cpp:778
[this](StringRef P, const PreservedAnalyses &PassPA) {
- auto Before = GraphStackBefore.pop_back_val();
- assert(Before.first == P &&
+ assert(PassStack.pop_back_val() == P &&
"Before and After callbacks must correspond");
----------------
skatkov wrote:
> you should not PassStack.pop_back_val() for product build?
PassStack is used only inside asserts
================
Comment at: llvm/lib/Passes/StandardInstrumentations.cpp:796
+ auto *F = any_cast<const Function *>(IR);
+ if (auto *GraphBefore = FAM.getCachedResult<PreservedCFGCheckerAnalysis>(
+ *const_cast<Function *>(F)))
----------------
to @skatkov
if //AfterPassCallback// is called before //AM.invalidate()// then we check CFG
only if the condition (PassPA.allAnalysesInSetPreserved<CFGAnalyses>() ||
PassPA.allAnalysesInSetPreserved<AllAnalysesOn<Function>>()) holds.
if //AfterPassCallback// is called after //AM.invalidate()// then we check CFG
only if the same condition holds and the PreservedCFGCheckerAnalysis is not
invalidated. But it is invalidated according to the same condition.
All in all it does not matter if //AfterPassCallback// is called before or
after //AM.invalidate()//.
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
