[PATCH] D15124: Use @llvm.invariant.start/end intrinsics to extend the meaning of basic AA's pointsToConstantMemory(), for GVN-based load elimination purposes [Local objects only]

[Larisse Voufo via llvm-commits]

On Thu, Dec 10, 2015 at 11:43 AM, Daniel Berlin <dberlin at dberlin.org> wrote:

>
>
> On Thu, Dec 10, 2015 at 11:27 AM, Larisse Voufo <lvoufo at gmail.com> wrote:
>
>> lvoufo added a comment.
>>
>> In http://reviews.llvm.org/D15124#307279, @dberlin wrote:
>>
>> > "(2) -gvn already requires -basicaa. So, -basicaa would be redundant in
>> >  '-basicaa -gvn'. I could add it for clarity if you prefer.
>> >  "
>> >  -gvn does not require -basicaa.
>> >  You've stated this a few times, and i'm not sure why you think this.
>> >
>> > <from GVN>:
>> >
>> >   void getAnalysisUsage(AnalysisUsage &AU) const override {
>> >         AU.addRequired<AssumptionCacheTracker>();
>> >         AU.addRequired<DominatorTreeWrapperPass>();
>> >         AU.addRequired<TargetLibraryInfoWrapperPass>();
>> >         if (!NoLoads)
>> >           AU.addRequired<MemoryDependenceAnalysis>();
>> >         AU.addRequired<AAResultsWrapperPass>();
>> >
>> >
>> > }
>> >
>> > GVN requires AC, DominatorTree, and TLI all the time.
>> >  It uses AA results, which is "whatever aa you have enabled".
>> >  It requires memorydependence if you want to optimize loads.
>> >  Memory dependence, in turn, does not require basicaa either:
>> >
>> > void MemoryDependenceAnalysis::getAnalysisUsage(AnalysisUsage &AU)
>> const {
>> >
>> >     AU.setPreservesAll();
>> >     AU.addRequired<AssumptionCacheTracker>();
>> >     AU.addRequiredTransitive<AAResultsWrapperPass>();
>> >     AU.addRequiredTransitive<TargetLibraryInfoWrapperPass>();
>> >   }
>> >
>> >
>> >
>> > No passes *require* basicaa, because basicaa is an optional pass
>> providing
>> >  AA results.
>>
>>
>> Point taken. I may be misusing "require" here, but I sure understand what
>> is going on better now thanks to this.
>> To clarify my statements and for added clarity from your points above, I
>> have 2 questions:
>>
>> 1 - What is one to make of the dependence on `BasicAAWrapperPass` in the
>> following?
>>
>>   INITIALIZE_PASS_BEGIN(AAResultsWrapperPass, "aa",
>>                         "Function Alias Analysis Results", false, true)
>>   INITIALIZE_PASS_DEPENDENCY(BasicAAWrapperPass)
>>   INITIALIZE_PASS_DEPENDENCY(CFLAAWrapperPass)
>>   INITIALIZE_PASS_DEPENDENCY(ExternalAAWrapperPass)
>>   INITIALIZE_PASS_DEPENDENCY(GlobalsAAWrapperPass)
>>   INITIALIZE_PASS_DEPENDENCY(ObjCARCAAWrapperPass)
>>   INITIALIZE_PASS_DEPENDENCY(SCEVAAWrapperPass)
>>   INITIALIZE_PASS_DEPENDENCY(ScopedNoAliasAAWrapperPass)
>>   INITIALIZE_PASS_DEPENDENCY(TypeBasedAAWrapperPass)
>>   INITIALIZE_PASS_END(AAResultsWrapperPass, "aa",
>>                       "Function Alias Analysis Results", false, true)
>>
>> So far as I understand, this *will* initialize a `BaiscAAWrapperPass`
>> pass which, when run (either via `run()` or `runOnFunction()`), will build
>> a `BasicAAResult` object.
>>
>
> None of these wrapper passes do anything in their runOnFunction.
> The resulting result object does nothing unless it is added by the
> aaresultswrapperpass to the aa results.
>
> See aaresultswrapperpass::runOnFunction.
>

Okay. So here's the interesting part
from AAResultsWrapperPass::runOnFunction:
...
  // BasicAA is always available for function analyses. Also, we add it
first
  // so that it can trump TBAA results when it proves MustAlias.
  // FIXME: TBAA should have an explicit mode to support this and then we
  // should reconsider the ordering here.
  if (!DisableBasicAA)
    AAR->addAAResult(getAnalysis<BasicAAWrapperPass>().getResult());
...

Unless basic AA is disabled (via `DisableBasicAA`), the result from the
`BasicAAWrapperPass` is added to the AA results.
`DisableBasicAA` is false by default, and `opt -disable-basicaa -gvn` *does
not* eliminate loads that `opt -gvn` does (that depend on -basicaa).

To your point, the `BasicAAWrapperPass` pass is initialized, and a basic AA
result object is created, regardless of whether basic AA is (explicitly)
disabled (e.g. via -disable-basicaa). Then, basic AA results only have an
effect in -gvn if basic AA is not disabled---which is the default behavior.
Right?

So, it seems fair to say that "-gvn *requires* -basicaa for our load
elimination purposes (which depend on basic AA)". Wouldn't you say?
Or would *depends on* (or else), instead of *requires*, be more accurate?
It is however true that simply saying "-gvn *requires* -basicaa" without
the added qualifiers is not entirely true, as -gvn would still function
without basic AA, but without eliminating our loads.


>
>
>>
>> 2 - There does not seem to be a difference between `opt -gvn` and `opt
>> -basicaa -gvn`. In fact, running `opt -gvn -debug-pass=Arguments` and  `opt
>> -basicaa -gvn -debug-pass=Arguments` both return the following, where
>> -basicaa is automatically added.
>>
>>   Pass Arguments:  -targetlibinfo -tti -assumption-cache-tracker
>> -invariant-info-marker -basicaa -domtree -aa -memdep -gvn -verify
>>
>
>
> Starting at the new AAResultsWrapperPass, it looks like this changed the
> default to require basicaa on August 17th (intentionally or not).
>

Indeed, running `opt -gvn -debug-pass=Arguments -disable-basicaa` produces
the exact same output as `opt -gvn -debug-pass=Arguments`, with the
disabled effect in -gvn only being visible in the output program.



> --Dan
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151210/c8fcbc21/attachment.html>