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

[Hal Finkel via llvm-commits]

----- Original Message -----
> From: "Chandler Carruth via llvm-commits" <llvm-commits at lists.llvm.org>
> To: llvm-commits at lists.llvm.org
> Sent: Friday, March 11, 2016 7:26:48 AM
> Subject: [llvm] r263231 - [PM] The order of evaluation of these analyses is actually significant,
> 
> 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.

I assume that we could delay the calls to getAnalysisIfAvailable until query time, instead of caching the results in runOnFunction. What are the other options?

 -Hal

> 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();
>  }
>  
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory