[PATCH] D91324: [NewPM] Disable PreserveCFGChecker and add regression unit tests

[Yevgeny Rouban via Phabricator via llvm-commits]

yrouban created this revision.
yrouban added reviewers: skatkov, kuhar, asbirlea, fedor.sergeev.
Herald added a subscriber: hiraditya.
Herald added a project: LLVM.
yrouban requested review of this revision.

The design of the PreserveCFG Checker (landed as D81558 <https://reviews.llvm.org/D81558> [NewPM] Introduce PreserveCFG check) has a fundamental flaw which makes it incorrect. The checker is based on the //PreservedAnalyses// result returned by functional passes: if //CFGAnalyses// is in the returned //PreservedAnalyses// set, then the checker asserts that the CFG snapshot saved before the pass is equal to the CFG snapshot taken after the the pass. The problem is in passes that change CFG and invalidate CFGAnalyses on their own. Such passes do not return //CFGanalyses// in the returned //PreservedAnalyses//. So the checker mistakenly expects CFG unchanged. As an example see the class //TestSimplifyCFGInvalidatingAnalysisPass// in the new tests.

It is interesting that the bug was not found in LLVM. That is because the CFG checker ran only if //CFGAnalyses// was checked incorrectly:

  if (!PassPA.allAnalysesInSetPreserved<CFGAnalyses>())
    return;

but must be checked as follows:

  auto PAC = PA.getChecker<PreservedCFGCheckerAnalysis>();
  if (!(PAC.preserved() || PAC.preservedSet<AllAnalysesOn<Function>>() || PAC.preservedSet<CFGAnalyses>())
    return;

This fix and a fully redesigned checker will be sent as a separate follow-up patch.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D91324

Files:
  llvm/lib/Passes/StandardInstrumentations.cpp
  llvm/unittests/IR/PassManagerTest.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D91324.304738.patch
Type: text/x-patch
Size: 7320 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20201112/06ff18fd/attachment.bin>