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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG39e3e3aa51d6: [NewPM] Redesign of PreserveCFG Checker (authored by yrouban). Changed prior to commit: https://reviews.llvm.org/D91327?vs=335221&id

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

skatkov accepted this revision. skatkov added a comment. This revision is now accepted and ready to land. with two nits. Comment at: llvm/include/llvm/Passes/StandardInstrumentations.h:422 + // Register all the standard instrumentation callbacks. If \p FAM is nullptr + // the

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

yrouban updated this revision to Diff 335221. yrouban marked 2 inline comments as done. yrouban added a comment. Addressed comments. Some tests were moved a separate patch (D99878 ). CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91327/new/ https://revie

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

skatkov added a comment. Consider landing tests update for "-verify-cfg-preserved=0" in a separate commit. This will significantly reduce the size of review. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91327/new/ https://reviews.llvm.org/D91327 __

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

skatkov added a comment. First iteration: style comments. Comment at: llvm/include/llvm/Passes/StandardInstrumentations.h:124 if (BBGuards) for (auto &BB : *BBGuards) { if (BB.second.isPoisoned()) redundant {} consider using any_of

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

yrouban added a comment. @skatkov, @fedor.sergeev, could you please review. During the long time this features is on the reconstruction we introduced a new bug in the LoopFlatten pass. See the comment https://reviews.llvm.org/D90940#inline-938253. CHANGES SINCE LAST ACTION https://reviews.ll

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

yrouban added inline comments. Comment at: llvm/include/llvm/Passes/StandardInstrumentations.h:422 + // Register all the standard instrumentation callbacks. If \p FAM is nullptr + // then PreservedCFGChecker is not registeredenabled. + void registerCallbacks(PassInstrumentatio

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

yrouban updated this revision to Diff 334042. yrouban added a comment. fixed the nits CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91327/new/ https://reviews.llvm.org/D91327 Files: clang/lib/CodeGen/BackendUtil.cpp llvm/include/llvm/Passes/StandardInstrumentations.h llvm/lib/LTO

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

yrouban marked 2 inline comments as done. yrouban added a comment. In D91327#2655786 , @kuhar wrote: > 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. Jakub, th

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

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 VerifyPreservedCF

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

yrouban updated this revision to Diff 333825. yrouban added a comment. just rebased. please review CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91327/new/ https://reviews.llvm.org/D91327 Files: clang/lib/CodeGen/BackendUtil.cpp llvm/include/llvm/Passes/StandardInstrumentations.h

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

yrouban added inline comments. Comment at: llvm/include/llvm/Passes/StandardInstrumentations.h:134 public: static cl::opt VerifyPreservedCFG; kuhar wrote: > yrouban wrote: > > kuhar wrote: > > > not necessary anymore > > there can bee a need to disabled/ena

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

kuhar added inline comments. Comment at: llvm/include/llvm/Passes/StandardInstrumentations.h:134 public: static cl::opt VerifyPreservedCFG; yrouban wrote: > kuhar wrote: > > not necessary anymore > there can bee a need to disabled/enable (e.g. for some test

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

yrouban added a comment. In D91327#2408521 , @kuhar wrote: > Looks fine to me, but I'm not confident enough to give an approval. Thanks. I'm working closely with @skatkov and believe he have enough expertise to review fully. The biggest issue with the cu

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

yrouban added inline comments. Comment at: llvm/include/llvm/Passes/StandardInstrumentations.h:134 public: static cl::opt VerifyPreservedCFG; kuhar wrote: > not necessary anymore there can bee a need to disabled/enable (e.g. for some tests or for debugging)

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

kuhar added a comment. Looks fine to me, but I'm not confident enough to give an approval. Comment at: llvm/lib/Passes/StandardInstrumentations.cpp:733-735 + static AnalysisKey Key; + +public: This is already public CHANGES SINCE LAST ACTION https://review

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

yrouban updated this revision to Diff 306604. yrouban marked 4 inline comments as done. yrouban edited the summary of this revision. yrouban added a comment. Addressed comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91327/new/ https://reviews.llvm.org/D91327 Files: clang/lib/

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

skatkov added inline comments. Comment at: llvm/lib/Passes/StandardInstrumentations.cpp:769 +auto *F = any_cast(IR); +if (auto *Result = FAM.getCachedResult( +*const_cast(F))) Result -> GraphBefore Comment at: llvm/lib/Pass

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

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) :

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

yrouban created this revision. yrouban added reviewers: skatkov, kuhar, asbirlea, fedor.sergeev. Herald added subscribers: cfe-commits, wenlei, steven_wu, hiraditya. Herald added projects: clang, LLVM. yrouban requested review of this revision. Please see the reason for this redesign in the patch