[PATCH] D146003: [StandardInstrumentations] Check that the number of instructions doesn't change if analyses are preserved

[Nikita Popov via Phabricator via cfe-commits]

nikic requested changes to this revision.
nikic added a comment.
This revision now requires changes to proceed.

Please split out the fixes to individual passes.



================
Comment at: llvm/lib/Passes/StandardInstrumentations.cpp:48
+                                             cl::Hidden,
 #ifdef NDEBUG
+                                             cl::init(false)
----------------
Better move the default behind EXPENSIVE_CHECKS.


================
Comment at: llvm/lib/Passes/StandardInstrumentations.cpp:1053
+// TODO: detect more changes than just instruction count.
+struct InstructionCountAnalysis : AnalysisInfoMixin<InstructionCountAnalysis> {
+  static AnalysisKey Key;
----------------
Instead of instruction count, it would be better to use StructuralHash. It exists for exactly this purpose.


================
Comment at: llvm/lib/Transforms/Coroutines/CoroCleanup.cpp:133
+    if (L.lower(F)) {
+      FAM.invalidate(F, PreservedAnalyses::none());
       FPM.run(F, FAM);
----------------
Can preserve CFG here.


================
Comment at: llvm/lib/Transforms/Utils/AssumeBundleBuilder.cpp:568
+                         AM.getCachedResult<DominatorTreeAnalysis>(F))
+             ? PreservedAnalyses::none()
+             : PreservedAnalyses::all();
----------------
Can preserve CFG here.


================
Comment at: llvm/lib/Transforms/Utils/AssumeBundleBuilder.cpp:616
   for (Instruction &I : instructions(F))
     salvageKnowledge(&I, AC, DT);
+  return PreservedAnalyses::none();
----------------
Should return from salvageKnowledge whether something was salvaged...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146003