Skip to content

[AA] Change RunEarly to be a Boolean Flag in ExternalAAWrapper #139158

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
May 15, 2025
Merged

[AA] Change RunEarly to be a Boolean Flag in ExternalAAWrapper #139158

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b950df7
Change RunEarly to be a boolean flag in ExternalAAWrapper
Chengjunp May 8, 2025
fd88987
Update comment
Chengjunp May 8, 2025
d847755
Make RunEarly a constructor argument
Chengjunp May 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 9 additions & 4 deletions llvm/include/llvm/Analysis/AliasAnalysis.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1013,17 +1013,22 @@ struct ExternalAAWrapperPass : ImmutablePass {

explicit ExternalAAWrapperPass(CallbackT CB);

/// Returns whether this external AA should run before Basic AA.
/// Flag indicating whether this external AA should run before Basic AA.
///
/// By default, external AA passes are run after Basic AA. If this returns
/// true, the external AA will be run before Basic AA during alias analysis.
/// This flag is for LegacyPassManager only. To run an external AA early
/// with the NewPassManager, override the registerEarlyDefaultAliasAnalyses
/// method on the target machine.
///
/// By default, external AA passes are run after Basic AA. If this flag is
/// set to true, the external AA will be run before Basic AA during alias
/// analysis.
///
/// For some targets, we prefer to run the external AA early to improve
/// compile time as it has more target-specific information. This is
/// particularly useful when the external AA can provide more precise results
/// than Basic AA so that Basic AA does not need to spend time recomputing
/// them.
virtual bool runEarly() { return false; }
bool RunEarly = false;

void getAnalysisUsage(AnalysisUsage &AU) const override {
AU.setPreservesAll();
Expand Down
4 changes: 2 additions & 2 deletions llvm/lib/Analysis/AliasAnalysis.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -741,7 +741,7 @@ bool AAResultsWrapperPass::runOnFunction(Function &F) {

// Add any target-specific alias analyses that should be run early.
auto *ExtWrapperPass = getAnalysisIfAvailable<ExternalAAWrapperPass>();
if (ExtWrapperPass && ExtWrapperPass->runEarly() && ExtWrapperPass->CB) {
if (ExtWrapperPass && ExtWrapperPass->RunEarly && ExtWrapperPass->CB) {
LLVM_DEBUG(dbgs() << "AAResults register Early ExternalAA: "
<< ExtWrapperPass->getPassName() << "\n");
ExtWrapperPass->CB(*this, F, *AAR);
Expand Down Expand Up @@ -777,7 +777,7 @@ bool AAResultsWrapperPass::runOnFunction(Function &F) {

// If available, run an external AA providing callback over the results as
// well.
if (ExtWrapperPass && !ExtWrapperPass->runEarly() && ExtWrapperPass->CB) {
if (ExtWrapperPass && !ExtWrapperPass->RunEarly && ExtWrapperPass->CB) {
LLVM_DEBUG(dbgs() << "AAResults register Late ExternalAA: "
<< ExtWrapperPass->getPassName() << "\n");
ExtWrapperPass->CB(*this, F, *AAR);
Expand Down
6 changes: 3 additions & 3 deletions llvm/lib/Target/NVPTX/NVPTXAliasAnalysis.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,14 +90,14 @@ class NVPTXExternalAAWrapper : public ExternalAAWrapperPass {
public:
static char ID;

bool runEarly() override { return true; }

NVPTXExternalAAWrapper()
: ExternalAAWrapperPass([](Pass &P, Function &, AAResults &AAR) {
if (auto *WrapperPass =
P.getAnalysisIfAvailable<NVPTXAAWrapperPass>())
AAR.addAAResult(WrapperPass->getResult());
}) {}
}) {
RunEarly = true;
Copy link
Contributor

@arsenm arsenm May 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make it a constructor argument (which you could hide under an EarlyExternalAA and LateExternalAA wrapper class)

Copy link
Contributor Author

@Chengjunp Chengjunp May 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you further explain how to "hide" RunEarly under an EarlyExternalAA and LateExternalAA wrapper class?

Do you mean making the constructor of ExternalAAWrapperPass to be

explicit ExternalAAWrapperPass(CallbackT CB, bool RunEarly = false);

and having

  NVPTXExternalAAWrapper()
      : ExternalAAWrapperPass([](Pass &P, Function &, AAResults &AAR) {
          if (auto *WrapperPass =
                  P.getAnalysisIfAvailable<NVPTXAAWrapperPass>())
            AAR.addAAResult(WrapperPass->getResult());
        }, /*RunEarly=*/true) {}

?

Copy link
Contributor

@Prince781 Prince781 May 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

which you could hide under an EarlyExternalAA and LateExternalAA wrapper class

What's the intent of doing this? To make it possible to use isa<> to determine whether an AA should run early?

Copy link
Contributor

@arsenm arsenm May 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean making the constructor of ExternalAAWrapperPass to be

Yes

What's the intent of doing this? To make it possible to use isa<> to determine whether an AA should run early?

No, passes don't do the RTTI thing. This is the minimal amount of typing for a pass to swap between the two forms

}

StringRef getPassName() const override {
return "NVPTX Address space based Alias Analysis Wrapper";
Expand Down
Loading