[PATCH] [PM/AA] Extract the ModRef enums from the AliasAnalysis class in preparation for de-coupling the AA implementations.

[Duncan P. N. Exon Smith]

> On 2015 Jun 19, at 02:08, Chandler Carruth <chandlerc at gmail.com> wrote:
> 
> In order to do this, they had to become fake-scoped using the
> traditional LLVM pattern of a leading initialism. These can't be actual
> scoped enumerations because they're bitfields and thus inherently we use
> them as integers.
> 
> I've also renamed the behavior enums that are specific to reasoning
> about the mod/ref behavior of functions when called. This makes it more
> clear that they have a very narrow domain of applicability.
> 
> I think there is a significantly cleaner API for all of this, but
> I don't want to try to do really substantive changes for now, I just
> want to refactor the things away from analysis groups so I'm preserving
> the exact original design and just cleaning up the names, style, and
> lifting out of the class.
> 
> Depends on D10494
> Depends on D10495
> 
> http://reviews.llvm.org/D10564
> 
> Files:
>  include/llvm/Analysis/AliasAnalysis.h
>  include/llvm/Analysis/LibCallAliasAnalysis.h
>  include/llvm/Analysis/LibCallSemantics.h
>  lib/Analysis/AliasAnalysis.cpp
>  lib/Analysis/AliasAnalysisCounter.cpp
>  lib/Analysis/AliasAnalysisEvaluator.cpp
>  lib/Analysis/AliasDebugger.cpp
>  lib/Analysis/AliasSetTracker.cpp
>  lib/Analysis/BasicAliasAnalysis.cpp
>  lib/Analysis/IPA/GlobalsModRef.cpp
>  lib/Analysis/LibCallAliasAnalysis.cpp
>  lib/Analysis/Loads.cpp
>  lib/Analysis/MemoryDependenceAnalysis.cpp
>  lib/Analysis/NoAliasAnalysis.cpp
>  lib/Analysis/ScopedNoAliasAA.cpp
>  lib/Analysis/TypeBasedAliasAnalysis.cpp
>  lib/Transforms/IPO/ArgumentPromotion.cpp
>  lib/Transforms/IPO/FunctionAttrs.cpp
>  lib/Transforms/Instrumentation/MemorySanitizer.cpp
>  lib/Transforms/ObjCARC/DependencyAnalysis.cpp
>  lib/Transforms/ObjCARC/ObjCARCAliasAnalysis.cpp
>  lib/Transforms/ObjCARC/ObjCARCAliasAnalysis.h
>  lib/Transforms/ObjCARC/ObjCARCContract.cpp
>  lib/Transforms/Scalar/DeadStoreElimination.cpp
>  lib/Transforms/Scalar/LICM.cpp
>  lib/Transforms/Scalar/LoopIdiomRecognize.cpp
>  lib/Transforms/Scalar/MemCpyOptimizer.cpp
>  lib/Transforms/Scalar/MergedLoadStoreMotion.cpp
>  lib/Transforms/Scalar/Sink.cpp
>  lib/Transforms/Utils/InlineFunction.cpp
>  unittests/Analysis/AliasAnalysisTest.cpp
>  utils/TableGen/IntrinsicEmitter.cpp
> 
> EMAIL PREFERENCES
>  http://reviews.llvm.org/settings/panel/emailpreferences/
> <D10564.28010.patch>_______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

LGTM, with minor comments below.  (I like the names you settled on.)

> Index: lib/Analysis/AliasAnalysisCounter.cpp
> ===================================================================
> --- lib/Analysis/AliasAnalysisCounter.cpp
> +++ lib/Analysis/AliasAnalysisCounter.cpp
> @@ -62,15 +62,16 @@
>                   << Must*100/AASum<<"%\n\n";
>          }
>  
> -        errs() << "  " << MRSum    << " Total Mod/Ref Queries Performed\n";
> +        errs() << "  " << MRSum << " Total MRI_Mod/MRI_Ref Queries Performed\n";

Any reason for this change?  I think Mod/Ref reads a little better.

>          if (MRSum) {
>            printLine("no mod/ref",    NoMR, MRSum);
>            printLine("ref",        JustRef, MRSum);
>            printLine("mod",        JustMod, MRSum);
>            printLine("mod/ref",         MR, MRSum);
> -          errs() << "  Mod/Ref Analysis Counter Summary: " <<NoMR*100/MRSum
> -                 << "%/" << JustRef*100/MRSum << "%/" << JustMod*100/MRSum
> -                 << "%/" << MR*100/MRSum <<"%\n\n";
> +          errs() << "  MRI_Mod/MRI_Ref Analysis Counter Summary: "
> +                 << NoMR * 100 / MRSum << "%/" << JustRef * 100 / MRSum << "%/"
> +                 << JustMod * 100 / MRSum << "%/" << MR * 100 / MRSum
> +                 << "%\n\n";

Similarly here, given that `printLine()` uses "mod" and "ref" without a
prefix.

>          }
>        }
>      }
> @@ -150,20 +151,31 @@
>    return R;
>  }
>  
> -AliasAnalysis::ModRefResult
> -AliasAnalysisCounter::getModRefInfo(ImmutableCallSite CS,
> -                                    const MemoryLocation &Loc) {
> -  ModRefResult R = getAnalysis<AliasAnalysis>().getModRefInfo(CS, Loc);
> +ModRefInfo AliasAnalysisCounter::getModRefInfo(ImmutableCallSite CS,
> +                                               const MemoryLocation &Loc) {
> +  ModRefInfo R = getAnalysis<AliasAnalysis>().getModRefInfo(CS, Loc);
>  
>    const char *MRString = nullptr;
>    switch (R) {
> -  case NoModRef: NoMR++;     MRString = "NoModRef"; break;
> -  case Ref:      JustRef++;  MRString = "JustRef"; break;
> -  case Mod:      JustMod++;  MRString = "JustMod"; break;
> -  case ModRef:   MR++;       MRString = "ModRef"; break;
> +  case MRI_NoModRef:
> +    NoMR++;
> +    MRString = "MRI_NoModRef";
> +    break;
> +  case MRI_Ref:
> +    JustRef++;
> +    MRString = "JustRef";
> +    break;
> +  case MRI_Mod:
> +    JustMod++;
> +    MRString = "JustMod";
> +    break;
> +  case MRI_ModRef:
> +    MR++;
> +    MRString = "MRI_ModRef";
> +    break;

(Boo, the table was easier to read.  Having it clang-formatted is
probably better, but I wish clang-format could be taught how to do
this.)

>    }
>  
> -  if (PrintAll || (PrintAllFailures && R == ModRef)) {
> +  if (PrintAll || (PrintAllFailures && R == MRI_ModRef)) {
>      errs() << MRString << ":  Ptr: ";
>      errs() << "[" << Loc.Size << "B] ";
>      Loc.Ptr->printAsOperand(errs(), true, M);