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.

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Chengjunp
Copy link
Contributor

Change the previous runEarly virtual function in ExternalAAWrapper to be a boolean flag.

@llvmbot
Copy link
Member

llvmbot commented May 8, 2025

@llvm/pr-subscribers-llvm-analysis

@llvm/pr-subscribers-backend-nvptx

Author: Chengjun (Chengjunp)

Changes

Change the previous runEarly virtual function in ExternalAAWrapper to be a boolean flag.


Full diff: https://github.com/llvm/llvm-project/pull/139158.diff

3 Files Affected:

  • (modified) llvm/include/llvm/Analysis/AliasAnalysis.h (+9-4)
  • (modified) llvm/lib/Analysis/AliasAnalysis.cpp (+2-2)
  • (modified) llvm/lib/Target/NVPTX/NVPTXAliasAnalysis.h (+3-3)
diff --git a/llvm/include/llvm/Analysis/AliasAnalysis.h b/llvm/include/llvm/Analysis/AliasAnalysis.h
index d23b81854c9ea..fad8bf7c8faea 100644
--- a/llvm/include/llvm/Analysis/AliasAnalysis.h
+++ b/llvm/include/llvm/Analysis/AliasAnalysis.h
@@ -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();
diff --git a/llvm/lib/Analysis/AliasAnalysis.cpp b/llvm/lib/Analysis/AliasAnalysis.cpp
index 27bd179a58ede..20c8ce67acbac 100644
--- a/llvm/lib/Analysis/AliasAnalysis.cpp
+++ b/llvm/lib/Analysis/AliasAnalysis.cpp
@@ -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);
@@ -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);
diff --git a/llvm/lib/Target/NVPTX/NVPTXAliasAnalysis.h b/llvm/lib/Target/NVPTX/NVPTXAliasAnalysis.h
index 430fcd741c1b6..e706f1ebc00fa 100644
--- a/llvm/lib/Target/NVPTX/NVPTXAliasAnalysis.h
+++ b/llvm/lib/Target/NVPTX/NVPTXAliasAnalysis.h
@@ -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;
+  }
 
   StringRef getPassName() const override {
     return "NVPTX Address space based Alias Analysis Wrapper";

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

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

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

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

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

@Chengjunp Chengjunp requested a review from arsenm May 8, 2025 22:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants