[llvm] r263231 - [PM] The order of evaluation of these analyses is actually significant,

Chandler Carruth via llvm-commits llvm-commits at lists.llvm.org
Fri Mar 11 05:26:48 PST 2016


Author: chandlerc
Date: Fri Mar 11 07:26:47 2016
New Revision: 263231

URL: http://llvm.org/viewvc/llvm-project?rev=263231&view=rev
Log:
[PM] The order of evaluation of these analyses is actually significant,
much to my horror, so use variables to fix it in place.

This terrifies me. Both basic-aa and memdep will provide more precise
information when the domtree and/or the loop info is available. Because
of this, if your pass (like GVN) requires domtree, and then queries
memdep or basic-aa, it will get more precise results. If it does this in
the other order, it gets less precise results.

All of the ideas I have for fixing this are, essentially, terrible. Here
I've just caused us to stop having unspecified behavior as different
implementations evaluate the order of these arguments differently. I'm
actually rather glad that they do, or the fragility of memdep and
basic-aa would have gone on unnoticed. I've left comments so we don't
immediately break this again. This should fix bots whose host compilers
evaluate the order of arguments differently from Clang.

Modified:
    llvm/trunk/lib/Transforms/Scalar/GVN.cpp

Modified: llvm/trunk/lib/Transforms/Scalar/GVN.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/GVN.cpp?rev=263231&r1=263230&r2=263231&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/GVN.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/GVN.cpp Fri Mar 11 07:26:47 2016
@@ -585,11 +585,16 @@ void GVN::ValueTable::verifyRemoved(cons
 //===----------------------------------------------------------------------===//
 
 PreservedAnalyses GVN::run(Function &F, AnalysisManager<Function> &AM) {
-  bool Changed = runImpl(F, AM.getResult<AssumptionAnalysis>(F),
-                         AM.getResult<DominatorTreeAnalysis>(F),
-                         AM.getResult<TargetLibraryAnalysis>(F),
-                         AM.getResult<AAManager>(F),
-                         &AM.getResult<MemoryDependenceAnalysis>(F));
+  // FIXME: The order of evaluation of these 'getResult' calls is very
+  // significant! Re-ordering these variables will cause GVN when run alone to
+  // be less effective! We should fix memdep and basic-aa to not exhibit this
+  // behavior, but until then don't change the order here.
+  auto &AC = AM.getResult<AssumptionAnalysis>(F);
+  auto &DT = AM.getResult<DominatorTreeAnalysis>(F);
+  auto &TLI = AM.getResult<TargetLibraryAnalysis>(F);
+  auto &AA = AM.getResult<AAManager>(F);
+  auto &MemDep = AM.getResult<MemoryDependenceAnalysis>(F);
+  bool Changed = runImpl(F, AC, DT, TLI, AA, &MemDep);
   return Changed ? PreservedAnalyses::none() : PreservedAnalyses::all();
 }
 




More information about the llvm-commits mailing list