[PATCH] D28627: [PM] Introduce an analysis set used to preserve all analyses over a function's CFG when that CFG is unchanged.

[Davide Italiano via Phabricator via llvm-commits]

davide added a comment.

I'm so glad to see this happening =)
First round of comments.



================
Comment at: lib/Transforms/InstCombine/InstructionCombining.cpp:3180-3181
+  auto PA = PreservedAnalyses::allOnFunctionCFG();
+  PA.preserve<AAManager>();
+  PA.preserve<GlobalsAA>();
   return PA;
----------------
The fact that we preserve now `GlobalsAA` and `AAManager` is an unrelated change, I guess? (can be committed separately).


================
Comment at: lib/Transforms/Scalar/DCE.cpp:127-128
 PreservedAnalyses DCEPass::run(Function &F, FunctionAnalysisManager &AM) {
-  if (eliminateDeadCode(F, AM.getCachedResult<TargetLibraryAnalysis>(F)))
-    return PreservedAnalyses::none();
-  return PreservedAnalyses::all();
+  if (!eliminateDeadCode(F, AM.getCachedResult<TargetLibraryAnalysis>(F)))
+    // Nothing changed.
+    return PreservedAnalyses::all();
----------------
this comment is too trivial to be useful.


================
Comment at: lib/Transforms/Scalar/IndVarSimplify.cpp:2497
+  PA.preserveSet<AnalysesOnFunctionCFG>();
+  return std::move(PA);
 }
----------------
Why `std::move()` here?


================
Comment at: lib/Transforms/Scalar/LoopRotation.cpp:637
     return PreservedAnalyses::all();
+
   return getLoopPassPreservedAnalyses();
----------------
Unrelated?


================
Comment at: lib/Transforms/Scalar/LoopSimplifyCFG.cpp:72
     return PreservedAnalyses::all();
+
   return getLoopPassPreservedAnalyses();
----------------
Unrelated?


================
Comment at: lib/Transforms/Scalar/LoopUnrollPass.cpp:1133
     return PreservedAnalyses::all();
+
   return getLoopPassPreservedAnalyses();
----------------
Unrelated?


================
Comment at: lib/Transforms/Scalar/NaryReassociate.cpp:168-169
 
   // FIXME: This should also 'preserve the CFG'.
-  PreservedAnalyses PA;
-  PA.preserve<DominatorTreeAnalysis>();
+  auto PA = PreservedAnalyses::allOnFunctionCFG();
   PA.preserve<ScalarEvolutionAnalysis>();
----------------
Remove the FIXME.


https://reviews.llvm.org/D28627