[PATCH] D17329: [AA] Hoist the logic to reformulate various AA queries in terms of other parts of the AA interface out of the base class of every single AA result object.

[Gerolf Hoflehner via llvm-commits]

Gerolf added inline comments.

================
Comment at: lib/Analysis/AliasAnalysis.cpp:139
@@ +138,3 @@
+  // aggregate set of AA results.
+  auto MRB = getModRefBehavior(CS);
+  if (MRB == FMRB_DoesNotAccessMemory)
----------------
chandlerc wrote:
> Gerolf wrote:
> > chandlerc wrote:
> > > Gerolf wrote:
> > > > chandlerc wrote:
> > > > > Gerolf wrote:
> > > > > > This routine queries AA again but this time for behavior rather than ModRefInfo, but the result (no mod ref info) could be the same. Can this be combined in some meta routine so that no mod ref is spit out early? The AA loops can be very compile-time expensive so we should avoid/shorten them when possible.
> > > > > This is interesting, but seems really orthogonal to this change. I don't want to try to fix everything in one go, I'm trying to fix a specific problem with the existing implementation.
> > > > My concern here is compile-time. The AA loops are compile-time intense and this code could make it worse. For a change like this it would be great if you could collect compile-time data before you commit. 
> > > I'm confused...
> > > 
> > > As far as I can tell, this change *cannot* make the compile time worse. Previously, this exact code existed in *every* base class for an AAResults, and would have been reached by the loop above for each one of them. That means it would have been run at least once (via BasicAA) and in many cases N times for the number of AAs active. In common Clang compiles that would be at least 4 times. So we're running the same code 1/4th as often with my change.
> > > 
> > > Unless I've missed something?
> > Hm, now I"m confused, although I can now see your ct argument.
> > 
> > Correct me wrong: The new code hoists the loop over the AA's hidden in AA->getModRef(CS, Loc) out of the loop. This makes the code more efficient. But isn't that function now just returning MRI_ModRef which renders loop above dead? 
> > 
> Er, the above loop calls through the AAResultConcept. It dynamically dispatches to each AA's implementation of getModRef. Some AAs implement this to provide more precise results, but certainly if *no* AA provides those precise results each call will end up at the base class returning MRI_ModRef, and we'll fall through that loop to here. But the whole point is to *allow* an AA result to handle this if desired.
Yep, now I'm in the boat and all makes sense. And it definitely helps a library compile-time problem I was looking at by cutting the time spent in AA time by about half(!!) in O3. So it all looks great to me . :-)


http://reviews.llvm.org/D17329