Page MenuHomePhabricator

[AA] Add the notion of target-specific alias analyses.
Needs ReviewPublic

Authored by jlebar on Sep 10 2016, 12:12 PM.

Details

Reviewers
chandlerc
hfinkel
Summary

This allows the TTI to specify an alias analysis specifically for its
target.

Example usage (in MyTargetTTI.cpp):

class MyTargetAAresult : public AAResultBase<MyTargetAAResult> { ... };

class MyTargetAAResultProvider : public AAResultProvider {
  MyTargetAAResult Result;
public:
  void addAAResult(AAResults &AAR) override {
    // AAR only keeps a reference to Result; we have to keep it alive.
    AAR.addResult(Result);
  }
};

std::unique_ptr<AAResultProvider>
MyTargetTTI::getAAResultProvider(const Function *) {
  return make_unique<MyTargetAAResultProvider>();
}

A simpler API (that doesn't end up working) would be to have a different
function on TTI:

TargetTransformInfo::addAAResult(const Function *, AAResults &)

This would also more closely mirror how external AAs work.

The problem with this is that AAResults::addAAResult takes a reference
to the result it's given, so that object must be stored somewhere. With
the external AA framework, the external target is expected to register a
pass and store its AAResult in that pass. But that doesn't work for our
purposes; we have no way to get the TTI to register a pass (nor do I
think that's desirable).

So instead, we essentially have the TTI return a closure which stores
the AAResult and provides access to it via
AAResultProvider::addAAResult. We use a custom class rather than
std::function for this closure because std::functions must be copyable,
but it's natural for this closure to capture non-copyable objects.

Diff Detail

Event Timeline

jlebar updated this revision to Diff 70952.Sep 10 2016, 12:12 PM
jlebar retitled this revision from to [AA] Add the notion of target-specific alias analyses..
jlebar updated this object.
jlebar added a reviewer: chandlerc.
jlebar added subscribers: hfinkel, llvm-commits.
jlebar removed a subscriber: hfinkel.
hfinkel edited edge metadata.Sep 24 2016, 8:35 AM

It seems somewhat unfortunate to have both ExternalAAWrapperPass and TargetSpecificAAWrapperPass. It also seems unfortunate that we can only ever have one external AA provider. What if we let ExternalAAWrapperPass contain an array of callbacks and let the target add to that array?

It seems somewhat unfortunate to have both ExternalAAWrapperPass and TargetSpecificAAWrapperPass. It also seems unfortunate that we can only ever have one external AA provider. What if we let ExternalAAWrapperPass contain an array of callbacks and let the target add to that array?

That seems simple enough. We'd need to change the external wrapper pass to use AAResultsProvider (a raw callback isn't going to work for a target-specific pass, see the commit message), but if anything I think that will be a simplification for clients of the external thing.

chandlerc edited edge metadata.Sep 26 2016, 11:23 AM

While we don't have any way for TTI to register a pass, we do have a way for the *target* to register a pass.

It seems somewhat unfortunate to have both ExternalAAWrapperPass and TargetSpecificAAWrapperPass. It also seems unfortunate that we can only ever have one external AA provider. What if we let ExternalAAWrapperPass contain an array of callbacks and let the target add to that array?

That seems simple enough. We'd need to change the external wrapper pass to use AAResultsProvider (a raw callback isn't going to work for a target-specific pass, see the commit message), but if anything I think that will be a simplification for clients of the external thing.

I think instead, the target *can* actually add a separate pass to provide the AA results, and then you should be able to use the external AA hooks already present. Let me find the place where you can do this from the target.

I think instead, the target *can* actually add a separate pass to provide the AA results, and then you should be able to use the external AA hooks already present.

...with a modification to support running more than one, I presume?

I'm fine with that (if you can show me how this is supposed to work), although personally I think having to register a pass just to store this data seems...kind of weird? Maybe there is a good reason to do it that way as opposed to with the explicit callback objects as in the commit message here.

In case anyone is watching this, I'm currently waiting for Chandler to put together a promised Correct Way of doing this that will work with the new pass manager.