Skip to content

AMDGPU/NewPM: Port SIAnnotateControlFlow to new pass manager #102653

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

Merged

Conversation

arsenm
Copy link
Contributor

@arsenm arsenm commented Aug 9, 2024

Does not yet add it to the pass pipeline. Somehow it causes
2 tests to assert in SelectionDAG, in functions without any
control flow.

Copy link
Contributor Author

arsenm commented Aug 9, 2024

@llvmbot
Copy link
Member

llvmbot commented Aug 9, 2024

@llvm/pr-subscribers-backend-amdgpu

Author: Matt Arsenault (arsenm)

Changes

Does not yet add it to the pass pipeline. Somehow it causes
2 tests to assert in SelectionDAG, in functions without any
control flow.


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

6 Files Affected:

  • (modified) llvm/lib/Target/AMDGPU/AMDGPU.h (+14-3)
  • (modified) llvm/lib/Target/AMDGPU/AMDGPUPassRegistry.def (+1)
  • (modified) llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp (+2-2)
  • (modified) llvm/lib/Target/AMDGPU/SIAnnotateControlFlow.cpp (+76-37)
  • (modified) llvm/test/CodeGen/AMDGPU/si-annotate-cf-noloop.ll (+1)
  • (modified) llvm/test/CodeGen/AMDGPU/si-annotate-cf-unreachable.ll (+1)
diff --git a/llvm/lib/Target/AMDGPU/AMDGPU.h b/llvm/lib/Target/AMDGPU/AMDGPU.h
index 195e2a19214e8..6b5754dad1770 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPU.h
+++ b/llvm/lib/Target/AMDGPU/AMDGPU.h
@@ -18,6 +18,7 @@
 namespace llvm {
 
 class AMDGPUTargetMachine;
+class GCNTargetMachine;
 class TargetMachine;
 
 // GlobalISel passes
@@ -32,7 +33,7 @@ void initializeAMDGPURegBankSelectPass(PassRegistry &);
 
 // SI Passes
 FunctionPass *createGCNDPPCombinePass();
-FunctionPass *createSIAnnotateControlFlowPass();
+FunctionPass *createSIAnnotateControlFlowLegacyPass();
 FunctionPass *createSIFoldOperandsPass();
 FunctionPass *createSIPeepholeSDWAPass();
 FunctionPass *createSILowerI1CopiesPass();
@@ -343,8 +344,18 @@ class AMDGPURewriteUndefForPHIPass
   PreservedAnalyses run(Function &F, FunctionAnalysisManager &AM);
 };
 
-void initializeSIAnnotateControlFlowPass(PassRegistry&);
-extern char &SIAnnotateControlFlowPassID;
+class SIAnnotateControlFlowPass
+    : public PassInfoMixin<SIAnnotateControlFlowPass> {
+private:
+  const AMDGPUTargetMachine &TM;
+
+public:
+  SIAnnotateControlFlowPass(const AMDGPUTargetMachine &TM) : TM(TM) {}
+  PreservedAnalyses run(Function &F, FunctionAnalysisManager &AM);
+};
+
+void initializeSIAnnotateControlFlowLegacyPass(PassRegistry &);
+extern char &SIAnnotateControlFlowLegacyPassID;
 
 void initializeSIMemoryLegalizerPass(PassRegistry&);
 extern char &SIMemoryLegalizerID;
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUPassRegistry.def b/llvm/lib/Target/AMDGPU/AMDGPUPassRegistry.def
index b6a6c33d85f83..c08b6d33d6a76 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUPassRegistry.def
+++ b/llvm/lib/Target/AMDGPU/AMDGPUPassRegistry.def
@@ -46,6 +46,7 @@ FUNCTION_PASS("amdgpu-rewrite-undef-for-phi", AMDGPURewriteUndefForPHIPass())
 FUNCTION_PASS("amdgpu-unify-divergent-exit-nodes",
               AMDGPUUnifyDivergentExitNodesPass())
 FUNCTION_PASS("amdgpu-usenative", AMDGPUUseNativeCallsPass())
+FUNCTION_PASS("si-annotate-control-flow", SIAnnotateControlFlowPass(*static_cast<const GCNTargetMachine *>(this)))
 #undef FUNCTION_PASS
 
 #ifndef FUNCTION_ANALYSIS
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp b/llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
index 584dbd571a814..f8abfceb76987 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
@@ -438,7 +438,7 @@ extern "C" LLVM_EXTERNAL_VISIBILITY void LLVMInitializeAMDGPUTarget() {
   initializeAMDGPURewriteOutArgumentsPass(*PR);
   initializeAMDGPURewriteUndefForPHILegacyPass(*PR);
   initializeAMDGPUUnifyMetadataPass(*PR);
-  initializeSIAnnotateControlFlowPass(*PR);
+  initializeSIAnnotateControlFlowLegacyPass(*PR);
   initializeAMDGPUInsertSingleUseVDSTPass(*PR);
   initializeAMDGPUInsertDelayAluPass(*PR);
   initializeSIInsertHardClausesPass(*PR);
@@ -1220,7 +1220,7 @@ bool GCNPassConfig::addPreISel() {
   }
   addPass(createAMDGPUAnnotateUniformValues());
   if (!LateCFGStructurize && !DisableStructurizer) {
-    addPass(createSIAnnotateControlFlowPass());
+    addPass(createSIAnnotateControlFlowLegacyPass());
     // TODO: Move this right after structurizeCFG to avoid extra divergence
     // analysis. This depends on stopping SIAnnotateControlFlow from making
     // control flow modifications.
diff --git a/llvm/lib/Target/AMDGPU/SIAnnotateControlFlow.cpp b/llvm/lib/Target/AMDGPU/SIAnnotateControlFlow.cpp
index 08e1d6b87b0df..edd881c84078c 100644
--- a/llvm/lib/Target/AMDGPU/SIAnnotateControlFlow.cpp
+++ b/llvm/lib/Target/AMDGPU/SIAnnotateControlFlow.cpp
@@ -12,6 +12,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "AMDGPU.h"
+#include "AMDGPUTargetMachine.h"
 #include "GCNSubtarget.h"
 #include "llvm/Analysis/LoopInfo.h"
 #include "llvm/Analysis/UniformityAnalysis.h"
@@ -36,7 +37,8 @@ namespace {
 using StackEntry = std::pair<BasicBlock *, Value *>;
 using StackVector = SmallVector<StackEntry, 16>;
 
-class SIAnnotateControlFlow : public FunctionPass {
+class SIAnnotateControlFlow {
+private:
   UniformityInfo *UA;
 
   Type *Boolean;
@@ -89,37 +91,17 @@ class SIAnnotateControlFlow : public FunctionPass {
   bool closeControlFlow(BasicBlock *BB);
 
 public:
-  static char ID;
-
-  SIAnnotateControlFlow() : FunctionPass(ID) {}
-
-  bool runOnFunction(Function &F) override;
-
-  StringRef getPassName() const override { return "SI annotate control flow"; }
-
-  void getAnalysisUsage(AnalysisUsage &AU) const override {
-    AU.addRequired<LoopInfoWrapperPass>();
-    AU.addRequired<DominatorTreeWrapperPass>();
-    AU.addRequired<UniformityInfoWrapperPass>();
-    AU.addPreserved<LoopInfoWrapperPass>();
-    AU.addPreserved<DominatorTreeWrapperPass>();
-    AU.addRequired<TargetPassConfig>();
-    FunctionPass::getAnalysisUsage(AU);
+  SIAnnotateControlFlow(Module &M, const GCNSubtarget &ST, DominatorTree &DT,
+                        LoopInfo &LI, UniformityInfo &UA)
+      : UA(&UA), DT(&DT), LI(&LI) {
+    initialize(M, ST);
   }
+
+  bool run(Function &F);
 };
 
 } // end anonymous namespace
 
-INITIALIZE_PASS_BEGIN(SIAnnotateControlFlow, DEBUG_TYPE,
-                      "Annotate SI Control Flow", false, false)
-INITIALIZE_PASS_DEPENDENCY(DominatorTreeWrapperPass)
-INITIALIZE_PASS_DEPENDENCY(UniformityInfoWrapperPass)
-INITIALIZE_PASS_DEPENDENCY(TargetPassConfig)
-INITIALIZE_PASS_END(SIAnnotateControlFlow, DEBUG_TYPE,
-                    "Annotate SI Control Flow", false, false)
-
-char SIAnnotateControlFlow::ID = 0;
-
 /// Initialize all the types and constants used in the pass
 void SIAnnotateControlFlow::initialize(Module &M, const GCNSubtarget &ST) {
   LLVMContext &Context = M.getContext();
@@ -349,15 +331,9 @@ bool SIAnnotateControlFlow::closeControlFlow(BasicBlock *BB) {
 
 /// Annotate the control flow with intrinsics so the backend can
 /// recognize if/then/else and loops.
-bool SIAnnotateControlFlow::runOnFunction(Function &F) {
-  DT = &getAnalysis<DominatorTreeWrapperPass>().getDomTree();
-  LI = &getAnalysis<LoopInfoWrapperPass>().getLoopInfo();
-  UA = &getAnalysis<UniformityInfoWrapperPass>().getUniformityInfo();
-  TargetPassConfig &TPC = getAnalysis<TargetPassConfig>();
-  const TargetMachine &TM = TPC.getTM<TargetMachine>();
-
+bool SIAnnotateControlFlow::run(Function &F) {
   bool Changed = false;
-  initialize(*F.getParent(), TM.getSubtarget<GCNSubtarget>(F));
+
   for (df_iterator<BasicBlock *> I = df_begin(&F.getEntryBlock()),
        E = df_end(&F.getEntryBlock()); I != E; ++I) {
     BasicBlock *BB = *I;
@@ -401,7 +377,70 @@ bool SIAnnotateControlFlow::runOnFunction(Function &F) {
   return Changed;
 }
 
+PreservedAnalyses SIAnnotateControlFlowPass::run(Function &F,
+                                                 FunctionAnalysisManager &FAM) {
+  const GCNSubtarget &ST = TM.getSubtarget<GCNSubtarget>(F);
+
+  DominatorTree &DT = FAM.getResult<DominatorTreeAnalysis>(F);
+  UniformityInfo &UI = FAM.getResult<UniformityInfoAnalysis>(F);
+  LoopInfo &LI = FAM.getResult<LoopAnalysis>(F);
+
+  SIAnnotateControlFlow Impl(*F.getParent(), ST, DT, LI, UI);
+
+  // FIXME: We introduce dead declarations of intrinsics even if never used.
+  bool Changed = Impl.run(F);
+  if (!Changed)
+    return PreservedAnalyses::none();
+
+  // TODO: Is LoopInfo preserved?
+  PreservedAnalyses PA = PreservedAnalyses::none();
+  PA.preserve<DominatorTreeAnalysis>();
+  return PA;
+}
+
+class SIAnnotateControlFlowLegacy : public FunctionPass {
+public:
+  static char ID;
+
+  SIAnnotateControlFlowLegacy() : FunctionPass(ID) {}
+
+  StringRef getPassName() const override { return "SI annotate control flow"; }
+
+  void getAnalysisUsage(AnalysisUsage &AU) const override {
+    AU.addRequired<LoopInfoWrapperPass>();
+    AU.addRequired<DominatorTreeWrapperPass>();
+    AU.addRequired<UniformityInfoWrapperPass>();
+    AU.addPreserved<LoopInfoWrapperPass>();
+    AU.addPreserved<DominatorTreeWrapperPass>();
+    AU.addRequired<TargetPassConfig>();
+    FunctionPass::getAnalysisUsage(AU);
+  }
+
+  bool runOnFunction(Function &F) override {
+    DominatorTree &DT = getAnalysis<DominatorTreeWrapperPass>().getDomTree();
+    LoopInfo &LI = getAnalysis<LoopInfoWrapperPass>().getLoopInfo();
+    UniformityInfo &UI =
+        getAnalysis<UniformityInfoWrapperPass>().getUniformityInfo();
+    TargetPassConfig &TPC = getAnalysis<TargetPassConfig>();
+    const TargetMachine &TM = TPC.getTM<TargetMachine>();
+    const GCNSubtarget &ST = TM.getSubtarget<GCNSubtarget>(F);
+
+    SIAnnotateControlFlow Impl(*F.getParent(), ST, DT, LI, UI);
+    return Impl.run(F);
+  }
+};
+
+INITIALIZE_PASS_BEGIN(SIAnnotateControlFlowLegacy, DEBUG_TYPE,
+                      "Annotate SI Control Flow", false, false)
+INITIALIZE_PASS_DEPENDENCY(DominatorTreeWrapperPass)
+INITIALIZE_PASS_DEPENDENCY(UniformityInfoWrapperPass)
+INITIALIZE_PASS_DEPENDENCY(TargetPassConfig)
+INITIALIZE_PASS_END(SIAnnotateControlFlowLegacy, DEBUG_TYPE,
+                    "Annotate SI Control Flow", false, false)
+
+char SIAnnotateControlFlowLegacy::ID = 0;
+
 /// Create the annotation pass
-FunctionPass *llvm::createSIAnnotateControlFlowPass() {
-  return new SIAnnotateControlFlow();
+FunctionPass *llvm::createSIAnnotateControlFlowLegacyPass() {
+  return new SIAnnotateControlFlowLegacy();
 }
diff --git a/llvm/test/CodeGen/AMDGPU/si-annotate-cf-noloop.ll b/llvm/test/CodeGen/AMDGPU/si-annotate-cf-noloop.ll
index 2495c0dff8929..644e223369ac6 100644
--- a/llvm/test/CodeGen/AMDGPU/si-annotate-cf-noloop.ll
+++ b/llvm/test/CodeGen/AMDGPU/si-annotate-cf-noloop.ll
@@ -1,4 +1,5 @@
 ; RUN: opt -mtriple=amdgcn-- -S -structurizecfg -si-annotate-control-flow -simplifycfg-require-and-preserve-domtree=1 %s | FileCheck -check-prefix=OPT %s
+; RUN: opt -mtriple=amdgcn-- -S -passes=structurizecfg,si-annotate-control-flow -simplifycfg-require-and-preserve-domtree=1 %s | FileCheck -check-prefix=OPT %s
 ; RUN: llc -mtriple=amdgcn -verify-machineinstrs -simplifycfg-require-and-preserve-domtree=1 < %s | FileCheck -check-prefix=GCN %s
 
 
diff --git a/llvm/test/CodeGen/AMDGPU/si-annotate-cf-unreachable.ll b/llvm/test/CodeGen/AMDGPU/si-annotate-cf-unreachable.ll
index 165b996981e34..58e3ee143dd20 100644
--- a/llvm/test/CodeGen/AMDGPU/si-annotate-cf-unreachable.ll
+++ b/llvm/test/CodeGen/AMDGPU/si-annotate-cf-unreachable.ll
@@ -1,4 +1,5 @@
 ; RUN: opt -mtriple=amdgcn-- -S -structurizecfg -si-annotate-control-flow %s | FileCheck -check-prefix=OPT %s
+; RUN: opt -mtriple=amdgcn-- -S -passes=structurizecfg,si-annotate-control-flow %s | FileCheck -check-prefix=OPT %s
 ; RUN: llc -mtriple=amdgcn -verify-machineinstrs < %s | FileCheck -check-prefix=GCN %s
 
 

@arsenm arsenm marked this pull request as ready for review August 9, 2024 18:01
Copy link
Contributor Author

arsenm commented Aug 10, 2024

Merge activity

  • Aug 9, 10:57 PM EDT: @arsenm started a stack merge that includes this pull request via Graphite.
  • Aug 9, 11:00 PM EDT: Graphite rebased this pull request as part of a merge.
  • Aug 9, 11:02 PM EDT: @arsenm merged this pull request with Graphite.

Does not yet add it to the pass pipeline. Somehow it causes
2 tests to assert in SelectionDAG, in functions without any
control flow.
@arsenm arsenm force-pushed the users/arsenm/newpm/amdgpu-port-si-annotate-control-flow branch from 034501e to aa17615 Compare August 10, 2024 02:59
@arsenm arsenm merged commit 76f722f into main Aug 10, 2024
5 of 8 checks passed
@arsenm arsenm deleted the users/arsenm/newpm/amdgpu-port-si-annotate-control-flow branch August 10, 2024 03:02
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.

3 participants