Skip to content
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

[NewPM][CodeGen] Port selection dag isel to new pass manager #83567

Merged
merged 27 commits into from
Jun 2, 2024

Conversation

paperchalice
Copy link
Contributor

It currently uses X86 as a proof of concept, will propagate to all targets until it looks good. Tests with --stop=after=x86-isel and without -verify-machineinstrs looks good.

@llvmbot llvmbot added backend:X86 llvm:SelectionDAG SelectionDAGISel as well labels Mar 1, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Mar 1, 2024

@llvm/pr-subscribers-backend-x86

Author: None (paperchalice)

Changes

It currently uses X86 as a proof of concept, will propagate to all targets until it looks good. Tests with --stop=after=x86-isel and without -verify-machineinstrs looks good.


Patch is 28.03 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/83567.diff

12 Files Affected:

  • (modified) llvm/include/llvm/CodeGen/SelectionDAG.h (+12)
  • (modified) llvm/include/llvm/CodeGen/SelectionDAGISel.h (+40-5)
  • (modified) llvm/include/llvm/CodeGen/StackProtector.h (+2)
  • (modified) llvm/lib/CodeGen/GCRootLowering.cpp (+3)
  • (modified) llvm/lib/CodeGen/MachinePassManager.cpp (+1-1)
  • (modified) llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp (+159-53)
  • (modified) llvm/lib/Target/X86/X86.h (+1-1)
  • (modified) llvm/lib/Target/X86/X86CodeGenPassBuilder.cpp (+12-1)
  • (modified) llvm/lib/Target/X86/X86ISelDAGToDAG.cpp (+19-8)
  • (added) llvm/lib/Target/X86/X86ISelDAGToDAG.h (+25)
  • (modified) llvm/lib/Target/X86/X86TargetMachine.cpp (+1-1)
  • (modified) llvm/lib/Target/X86/X86TargetMachine.h (+3)
diff --git a/llvm/include/llvm/CodeGen/SelectionDAG.h b/llvm/include/llvm/CodeGen/SelectionDAG.h
index 25e6c525b672a1..db256e5e3d3c5e 100644
--- a/llvm/include/llvm/CodeGen/SelectionDAG.h
+++ b/llvm/include/llvm/CodeGen/SelectionDAG.h
@@ -29,6 +29,7 @@
 #include "llvm/CodeGen/ISDOpcodes.h"
 #include "llvm/CodeGen/MachineFunction.h"
 #include "llvm/CodeGen/MachineMemOperand.h"
+#include "llvm/CodeGen/MachinePassManager.h"
 #include "llvm/CodeGen/SelectionDAGNodes.h"
 #include "llvm/CodeGen/ValueTypes.h"
 #include "llvm/CodeGenTypes/MachineValueType.h"
@@ -229,6 +230,7 @@ class SelectionDAG {
   const TargetLibraryInfo *LibInfo = nullptr;
   const FunctionVarLocs *FnVarLocs = nullptr;
   MachineFunction *MF;
+  MachineFunctionAnalysisManager *MFAM = nullptr;
   Pass *SDAGISelPass = nullptr;
   LLVMContext *Context;
   CodeGenOptLevel OptLevel;
@@ -458,6 +460,15 @@ class SelectionDAG {
             UniformityInfo *UA, ProfileSummaryInfo *PSIin,
             BlockFrequencyInfo *BFIin, FunctionVarLocs const *FnVarLocs);
 
+  void init(MachineFunction &NewMF, OptimizationRemarkEmitter &NewORE,
+            MachineFunctionAnalysisManager &AM,
+            const TargetLibraryInfo *LibraryInfo, UniformityInfo *UA,
+            ProfileSummaryInfo *PSIin, BlockFrequencyInfo *BFIin,
+            FunctionVarLocs const *FnVarLocs) {
+    init(NewMF, NewORE, nullptr, LibraryInfo, UA, PSIin, BFIin, FnVarLocs);
+    MFAM = &AM;
+  }
+
   void setFunctionLoweringInfo(FunctionLoweringInfo * FuncInfo) {
     FLI = FuncInfo;
   }
@@ -468,6 +479,7 @@ class SelectionDAG {
 
   MachineFunction &getMachineFunction() const { return *MF; }
   const Pass *getPass() const { return SDAGISelPass; }
+  MachineFunctionAnalysisManager *getMFAM() { return MFAM; }
 
   const DataLayout &getDataLayout() const { return MF->getDataLayout(); }
   const TargetMachine &getTarget() const { return TM; }
diff --git a/llvm/include/llvm/CodeGen/SelectionDAGISel.h b/llvm/include/llvm/CodeGen/SelectionDAGISel.h
index dbd9b391f4a431..1157e66f282467 100644
--- a/llvm/include/llvm/CodeGen/SelectionDAGISel.h
+++ b/llvm/include/llvm/CodeGen/SelectionDAGISel.h
@@ -15,6 +15,7 @@
 #define LLVM_CODEGEN_SELECTIONDAGISEL_H
 
 #include "llvm/CodeGen/MachineFunctionPass.h"
+#include "llvm/CodeGen/MachinePassManager.h"
 #include "llvm/CodeGen/SelectionDAG.h"
 #include "llvm/IR/BasicBlock.h"
 #include <memory>
@@ -24,6 +25,7 @@ class AAResults;
 class AssumptionCache;
 class TargetInstrInfo;
 class TargetMachine;
+class SSPLayoutInfo;
 class SelectionDAGBuilder;
 class SDValue;
 class MachineRegisterInfo;
@@ -31,6 +33,7 @@ class MachineFunction;
 class OptimizationRemarkEmitter;
 class TargetLowering;
 class TargetLibraryInfo;
+class TargetTransformInfo;
 class FunctionLoweringInfo;
 class SwiftErrorValueTracking;
 class GCFunctionInfo;
@@ -38,7 +41,7 @@ class ScheduleDAGSDNodes;
 
 /// SelectionDAGISel - This is the common base class used for SelectionDAG-based
 /// pattern-matching instruction selectors.
-class SelectionDAGISel : public MachineFunctionPass {
+class SelectionDAGISel {
 public:
   TargetMachine &TM;
   const TargetLibraryInfo *LibInfo;
@@ -51,6 +54,8 @@ class SelectionDAGISel : public MachineFunctionPass {
   AAResults *AA = nullptr;
   AssumptionCache *AC = nullptr;
   GCFunctionInfo *GFI = nullptr;
+  SSPLayoutInfo *SP = nullptr;
+  TargetTransformInfo *TTI = nullptr;
   CodeGenOptLevel OptLevel;
   const TargetInstrInfo *TII;
   const TargetLowering *TLI;
@@ -67,16 +72,22 @@ class SelectionDAGISel : public MachineFunctionPass {
   /// functions. Storing the filter result here so that we only need to do the
   /// filtering once.
   bool MatchFilterFuncName = false;
+  StringRef FuncName;
 
-  explicit SelectionDAGISel(char &ID, TargetMachine &tm,
+  explicit SelectionDAGISel(TargetMachine &tm,
                             CodeGenOptLevel OL = CodeGenOptLevel::Default);
-  ~SelectionDAGISel() override;
+  ~SelectionDAGISel();
 
   const TargetLowering *getTargetLowering() const { return TLI; }
 
-  void getAnalysisUsage(AnalysisUsage &AU) const override;
+  void initialize(MachineFunctionAnalysisManager &MFAM);
+  void initialize(MachineFunctionPass &MFP);
 
-  bool runOnMachineFunction(MachineFunction &MF) override;
+  /// Subclasses can override this function to set Subtarget
+  /// before running the pass.
+  virtual void prepare(MachineFunction &MF) {}
+
+  bool runOnMachineFunction(MachineFunction &mf, bool Skipped);
 
   virtual void emitFunctionEntryCode() {}
 
@@ -513,6 +524,30 @@ class SelectionDAGISel : public MachineFunctionPass {
                     bool isMorphNodeTo);
 };
 
+class SelectionDAGISelLegacy : public MachineFunctionPass {
+  std::unique_ptr<SelectionDAGISel> Selector;
+
+public:
+  SelectionDAGISelLegacy(char &ID, std::unique_ptr<SelectionDAGISel> S);
+
+  ~SelectionDAGISelLegacy() override = default;
+
+  void getAnalysisUsage(AnalysisUsage &AU) const override;
+
+  bool runOnMachineFunction(MachineFunction &MF) override;
+};
+
+class SelectionDAGISelPass : public PassInfoMixin<SelectionDAGISelPass> {
+  std::unique_ptr<SelectionDAGISel> Selector;
+
+protected:
+  SelectionDAGISelPass(std::unique_ptr<SelectionDAGISel> Selector)
+      : Selector(std::move(Selector)) {}
+
+public:
+  PreservedAnalyses run(MachineFunction &MF,
+                        MachineFunctionAnalysisManager &MFAM);
+};
 }
 
 #endif /* LLVM_CODEGEN_SELECTIONDAGISEL_H */
diff --git a/llvm/include/llvm/CodeGen/StackProtector.h b/llvm/include/llvm/CodeGen/StackProtector.h
index eb5d9d0caebc60..dfafc781067d71 100644
--- a/llvm/include/llvm/CodeGen/StackProtector.h
+++ b/llvm/include/llvm/CodeGen/StackProtector.h
@@ -109,6 +109,8 @@ class StackProtector : public FunctionPass {
 
   StackProtector();
 
+  SSPLayoutInfo &getLayoutInfo() { return LayoutInfo; }
+
   void getAnalysisUsage(AnalysisUsage &AU) const override;
 
   // Return true if StackProtector is supposed to be handled by SelectionDAG.
diff --git a/llvm/lib/CodeGen/GCRootLowering.cpp b/llvm/lib/CodeGen/GCRootLowering.cpp
index 894ab9a0486a7b..0d82f2bab8960b 100644
--- a/llvm/lib/CodeGen/GCRootLowering.cpp
+++ b/llvm/lib/CodeGen/GCRootLowering.cpp
@@ -81,6 +81,9 @@ class GCMachineCodeAnalysis : public MachineFunctionPass {
 
 PreservedAnalyses GCLoweringPass::run(Function &F,
                                       FunctionAnalysisManager &FAM) {
+  if (!F.hasGC())
+    return PreservedAnalyses::all();
+
   auto &Info = FAM.getResult<GCFunctionAnalysis>(F);
 
   bool Changed = DoLowering(F, Info.getStrategy());
diff --git a/llvm/lib/CodeGen/MachinePassManager.cpp b/llvm/lib/CodeGen/MachinePassManager.cpp
index 9a750b5bed4339..96556d549a2548 100644
--- a/llvm/lib/CodeGen/MachinePassManager.cpp
+++ b/llvm/lib/CodeGen/MachinePassManager.cpp
@@ -79,7 +79,7 @@ ModuleToMachineFunctionPassAdaptor::run(Module &M, ModuleAnalysisManager &AM) {
   for (Function &F : M) {
     // Do not codegen any 'available_externally' functions at all, they have
     // definitions outside the translation unit.
-    if (F.hasAvailableExternallyLinkage())
+    if (F.hasAvailableExternallyLinkage() || F.isDeclaration())
       continue;
 
     MachineFunction &MF = MMI.getOrCreateMachineFunction(F);
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
index 9b5ab4267b80e9..81e03b885fee03 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
@@ -341,9 +341,34 @@ void TargetLowering::AdjustInstrPostInstrSelection(MachineInstr &MI,
 // SelectionDAGISel code
 //===----------------------------------------------------------------------===//
 
-SelectionDAGISel::SelectionDAGISel(char &ID, TargetMachine &tm,
-                                   CodeGenOptLevel OL)
-    : MachineFunctionPass(ID), TM(tm), FuncInfo(new FunctionLoweringInfo()),
+SelectionDAGISelLegacy::SelectionDAGISelLegacy(
+    char &ID, std::unique_ptr<SelectionDAGISel> S)
+    : MachineFunctionPass(ID), Selector(std::move(S)) {
+  initializeGCModuleInfoPass(*PassRegistry::getPassRegistry());
+  initializeBranchProbabilityInfoWrapperPassPass(
+      *PassRegistry::getPassRegistry());
+  initializeAAResultsWrapperPassPass(*PassRegistry::getPassRegistry());
+  initializeTargetLibraryInfoWrapperPassPass(*PassRegistry::getPassRegistry());
+}
+
+bool SelectionDAGISelLegacy::runOnMachineFunction(MachineFunction &MF) {
+  // If we already selected that function, we do not need to run SDISel.
+  if (MF.getProperties().hasProperty(
+          MachineFunctionProperties::Property::Selected))
+      return false;
+
+  // Do some sanity-checking on the command-line options.
+  assert((!EnableFastISelAbort || Selector->TM.Options.EnableFastISel) &&
+         "-fast-isel-abort > 0 requires -fast-isel");
+
+  Selector->MF = &MF;
+  Selector->setSubtarget(MF);
+  Selector->initialize(*this);
+  return Selector->runOnMachineFunction(MF, skipFunction(MF.getFunction()));
+}
+
+SelectionDAGISel::SelectionDAGISel(TargetMachine &tm, CodeGenOptLevel OL)
+    : TM(tm), FuncInfo(new FunctionLoweringInfo()),
       SwiftError(new SwiftErrorValueTracking()),
       CurDAG(new SelectionDAG(tm, OL)),
       SDB(std::make_unique<SelectionDAGBuilder>(*CurDAG, *FuncInfo, *SwiftError,
@@ -361,7 +386,8 @@ SelectionDAGISel::~SelectionDAGISel() {
   delete SwiftError;
 }
 
-void SelectionDAGISel::getAnalysisUsage(AnalysisUsage &AU) const {
+void SelectionDAGISelLegacy::getAnalysisUsage(AnalysisUsage &AU) const {
+  const auto &OptLevel = Selector->OptLevel;
   if (OptLevel != CodeGenOptLevel::None)
       AU.addRequired<AAResultsWrapperPass>();
   AU.addRequired<GCModuleInfo>();
@@ -406,64 +432,117 @@ static void computeUsesMSVCFloatingPoint(const Triple &TT, const Function &F,
   }
 }
 
-bool SelectionDAGISel::runOnMachineFunction(MachineFunction &mf) {
+PreservedAnalyses
+SelectionDAGISelPass::run(MachineFunction &MF,
+                          MachineFunctionAnalysisManager &MFAM) {
   // If we already selected that function, we do not need to run SDISel.
-  if (mf.getProperties().hasProperty(
+  if (MF.getProperties().hasProperty(
           MachineFunctionProperties::Property::Selected))
-    return false;
+    return PreservedAnalyses::all();
+
   // Do some sanity-checking on the command-line options.
-  assert((!EnableFastISelAbort || TM.Options.EnableFastISel) &&
+  assert((!EnableFastISelAbort || Selector->TM.Options.EnableFastISel) &&
          "-fast-isel-abort > 0 requires -fast-isel");
 
-  const Function &Fn = mf.getFunction();
-  MF = &mf;
+  Selector->MF = &MF;
+  Selector->prepare(MF);
+  Selector->initialize(MFAM);
+  Selector->runOnMachineFunction(MF, false);
+
+  return PreservedAnalyses::none();
+}
 
+void SelectionDAGISel::initialize(MachineFunctionAnalysisManager &MFAM) {
+  auto &FAM = MFAM.getResult<FunctionAnalysisManagerMachineFunctionProxy>(*MF)
+                  .getManager();
+  auto &MAMP = MFAM.getResult<ModuleAnalysisManagerMachineFunctionProxy>(*MF);
+  Function &Fn = MF->getFunction();
 #ifndef NDEBUG
-  StringRef FuncName = Fn.getName();
+  FuncName = Fn.getName();
   MatchFilterFuncName = isFunctionInPrintList(FuncName);
 #else
   (void)MatchFilterFuncName;
 #endif
 
-  // Decide what flavour of variable location debug-info will be used, before
-  // we change the optimisation level.
-  bool InstrRef = mf.shouldUseDebugInstrRef();
-  mf.setUseDebugInstrRef(InstrRef);
+  TII = MF->getSubtarget().getInstrInfo();
+  TLI = MF->getSubtarget().getTargetLowering();
+  RegInfo = &MF->getRegInfo();
+  LibInfo = &FAM.getResult<TargetLibraryAnalysis>(Fn);
+  GFI = Fn.hasGC() ? &FAM.getResult<GCFunctionAnalysis>(Fn) : nullptr;
+  ORE = std::make_unique<OptimizationRemarkEmitter>(&Fn);
+  AC = &FAM.getResult<AssumptionAnalysis>(Fn);
+  auto *PSI = MAMP.getCachedResult<ProfileSummaryAnalysis>(*Fn.getParent());
+  BlockFrequencyInfo *BFI = nullptr;
+  FAM.getResult<BlockFrequencyAnalysis>(Fn);
+  if (PSI && PSI->hasProfileSummary() && OptLevel != CodeGenOptLevel::None)
+    BFI = &FAM.getResult<BlockFrequencyAnalysis>(Fn);
 
-  // Reset the target options before resetting the optimization
-  // level below.
-  // FIXME: This is a horrible hack and should be processed via
-  // codegen looking at the optimization level explicitly when
-  // it wants to look at it.
-  TM.resetTargetOptions(Fn);
-  // Reset OptLevel to None for optnone functions.
-  CodeGenOptLevel NewOptLevel = OptLevel;
-  if (OptLevel != CodeGenOptLevel::None && skipFunction(Fn))
-    NewOptLevel = CodeGenOptLevel::None;
-  OptLevelChanger OLC(*this, NewOptLevel);
+  FunctionVarLocs const *FnVarLocs = nullptr;
+  if (isAssignmentTrackingEnabled(*Fn.getParent()))
+    FnVarLocs = &FAM.getResult<DebugAssignmentTrackingAnalysis>(Fn);
+
+  ISEL_DUMP(dbgs() << "\n\n\n=== " << FuncName << "\n");
+
+  auto *UA = FAM.getCachedResult<UniformityInfoAnalysis>(Fn);
+  CurDAG->init(*MF, *ORE, MFAM, LibInfo, UA, PSI, BFI, FnVarLocs);
+  FuncInfo->set(Fn, *MF, CurDAG);
+  SwiftError->setFunction(*MF);
+
+  // Now get the optional analyzes if we want to.
+  // This is based on the possibly changed OptLevel (after optnone is taken
+  // into account).  That's unfortunate but OK because it just means we won't
+  // ask for passes that have been required anyway.
+
+  FAM.getResult<BranchProbabilityAnalysis>(Fn);
+  if (UseMBPI && OptLevel != CodeGenOptLevel::None)
+    FuncInfo->BPI = &FAM.getResult<BranchProbabilityAnalysis>(Fn);
+  else
+    FuncInfo->BPI = nullptr;
+
+  if (OptLevel != CodeGenOptLevel::None)
+    AA = &FAM.getResult<AAManager>(Fn);
+  else
+    AA = nullptr;
+
+  SP = &FAM.getResult<SSPLayoutAnalysis>(Fn);
+
+#ifndef NDEBUG
+  TTI = &FAM.getResult<TargetIRAnalysis>(Fn);
+#endif
+}
+
+void SelectionDAGISel::initialize(MachineFunctionPass &MFP) {
+  Function &Fn = MF->getFunction();
+#ifndef NDEBUG
+  FuncName = Fn.getName();
+  MatchFilterFuncName = isFunctionInPrintList(FuncName);
+#else
+  (void)MatchFilterFuncName;
+#endif
 
   TII = MF->getSubtarget().getInstrInfo();
   TLI = MF->getSubtarget().getTargetLowering();
   RegInfo = &MF->getRegInfo();
-  LibInfo = &getAnalysis<TargetLibraryInfoWrapperPass>().getTLI(Fn);
-  GFI = Fn.hasGC() ? &getAnalysis<GCModuleInfo>().getFunctionInfo(Fn) : nullptr;
+  LibInfo = &MFP.getAnalysis<TargetLibraryInfoWrapperPass>().getTLI(Fn);
+  GFI = Fn.hasGC() ? &MFP.getAnalysis<GCModuleInfo>().getFunctionInfo(Fn)
+                   : nullptr;
   ORE = std::make_unique<OptimizationRemarkEmitter>(&Fn);
-  AC = &getAnalysis<AssumptionCacheTracker>().getAssumptionCache(mf.getFunction());
-  auto *PSI = &getAnalysis<ProfileSummaryInfoWrapperPass>().getPSI();
+  AC = &MFP.getAnalysis<AssumptionCacheTracker>().getAssumptionCache(Fn);
+  auto *PSI = &MFP.getAnalysis<ProfileSummaryInfoWrapperPass>().getPSI();
   BlockFrequencyInfo *BFI = nullptr;
   if (PSI && PSI->hasProfileSummary() && OptLevel != CodeGenOptLevel::None)
-    BFI = &getAnalysis<LazyBlockFrequencyInfoPass>().getBFI();
+    BFI = &MFP.getAnalysis<LazyBlockFrequencyInfoPass>().getBFI();
 
   FunctionVarLocs const *FnVarLocs = nullptr;
   if (isAssignmentTrackingEnabled(*Fn.getParent()))
-    FnVarLocs = getAnalysis<AssignmentTrackingAnalysis>().getResults();
+    FnVarLocs = MFP.getAnalysis<AssignmentTrackingAnalysis>().getResults();
 
   ISEL_DUMP(dbgs() << "\n\n\n=== " << FuncName << "\n");
 
   UniformityInfo *UA = nullptr;
-  if (auto *UAPass = getAnalysisIfAvailable<UniformityInfoWrapperPass>())
+  if (auto *UAPass = MFP.getAnalysisIfAvailable<UniformityInfoWrapperPass>())
     UA = &UAPass->getUniformityInfo();
-  CurDAG->init(*MF, *ORE, this, LibInfo, UA, PSI, BFI, FnVarLocs);
+  CurDAG->init(*MF, *ORE, &MFP, LibInfo, UA, PSI, BFI, FnVarLocs);
   FuncInfo->set(Fn, *MF, CurDAG);
   SwiftError->setFunction(*MF);
 
@@ -473,15 +552,46 @@ bool SelectionDAGISel::runOnMachineFunction(MachineFunction &mf) {
   // ask for passes that have been required anyway.
 
   if (UseMBPI && OptLevel != CodeGenOptLevel::None)
-    FuncInfo->BPI = &getAnalysis<BranchProbabilityInfoWrapperPass>().getBPI();
+    FuncInfo->BPI =
+        &MFP.getAnalysis<BranchProbabilityInfoWrapperPass>().getBPI();
   else
     FuncInfo->BPI = nullptr;
 
   if (OptLevel != CodeGenOptLevel::None)
-    AA = &getAnalysis<AAResultsWrapperPass>().getAAResults();
+    AA = &MFP.getAnalysis<AAResultsWrapperPass>().getAAResults();
   else
     AA = nullptr;
 
+  SP = &MFP.getAnalysis<StackProtector>().getLayoutInfo();
+
+#ifndef NDEBUG
+  TTI =
+      &MFP.getAnalysis<TargetTransformInfoWrapperPass>().getTTI(*FuncInfo->Fn);
+#endif
+}
+
+bool SelectionDAGISel::runOnMachineFunction(MachineFunction &mf, bool Skipped) {
+  const Function &Fn = mf.getFunction();
+
+  // Decide what flavour of variable location debug-info will be used, before
+  // we change the optimisation level.
+  bool InstrRef = mf.shouldUseDebugInstrRef();
+  mf.setUseDebugInstrRef(InstrRef);
+
+  // Reset the target options before resetting the optimization
+  // level below.
+  // FIXME: This is a horrible hack and should be processed via
+  // codegen looking at the optimization level explicitly when
+  // it wants to look at it.
+  TM.resetTargetOptions(Fn);
+  // Reset OptLevel to None for optnone functions.
+  CodeGenOptLevel NewOptLevel = OptLevel;
+  if (OptLevel != CodeGenOptLevel::None && Skipped)
+    NewOptLevel = CodeGenOptLevel::None;
+  OptLevelChanger OLC(*this, NewOptLevel);
+
+  ISEL_DUMP(dbgs() << "\n\n\n=== " << FuncName << "\n");
+
   SDB->init(GFI, AA, AC, LibInfo);
 
   MF->setHasInlineAsm(false);
@@ -779,11 +889,8 @@ void SelectionDAGISel::CodeGenAndEmitDAG() {
   StringRef GroupName = "sdag";
   StringRef GroupDescription = "Instruction Selection and Scheduling";
   std::string BlockName;
-  bool MatchFilterBB = false; (void)MatchFilterBB;
-#ifndef NDEBUG
-  TargetTransformInfo &TTI =
-      getAnalysis<TargetTransformInfoWrapperPass>().getTTI(*FuncInfo->Fn);
-#endif
+  bool MatchFilterBB = false;
+  (void)MatchFilterBB;
 
   // Pre-type legalization allow creation of any node types.
   CurDAG->NewNodesMustHaveLegalTypes = false;
@@ -808,7 +915,7 @@ void SelectionDAGISel::CodeGenAndEmitDAG() {
             CurDAG->dump());
 
 #ifndef NDEBUG
-  if (TTI.hasBranchDivergence())
+  if (TTI->hasBranchDivergence())
     CurDAG->VerifyDAGDivergence();
 #endif
 
@@ -828,7 +935,7 @@ void SelectionDAGISel::CodeGenAndEmitDAG() {
             CurDAG->dump());
 
 #ifndef NDEBUG
-  if (TTI.hasBranchDivergence())
+  if (TTI->hasBranchDivergence())
     CurDAG->VerifyDAGDivergence();
 #endif
 
@@ -850,7 +957,7 @@ void SelectionDAGISel::CodeGenAndEmitDAG() {
             CurDAG->dump());
 
 #ifndef NDEBUG
-  if (TTI.hasBranchDivergence())
+  if (TTI->hasBranchDivergence())
     CurDAG->VerifyDAGDivergence();
 #endif
 
@@ -874,7 +981,7 @@ void SelectionDAGISel::CodeGenAndEmitDAG() {
               CurDAG->dump());
 
 #ifndef NDEBUG
-    if (TTI.hasBranchDivergence())
+    if (TTI->hasBranchDivergence())
       CurDAG->VerifyDAGDivergence();
 #endif
   }
@@ -892,7 +999,7 @@ void SelectionDAGISel::CodeGenAndEmitDAG() {
               CurDAG->dump());
 
 #ifndef NDEBUG
-    if (TTI.hasBranchDivergence())
+    if (TTI->hasBranchDivergence())
       CurDAG->VerifyDAGDivergence();
 #endif
 
@@ -908,7 +1015,7 @@ void SelectionDAGISel::CodeGenAndEmitDAG() {
               CurDAG->dump());
 
 #ifndef NDEBUG
-    if (TTI.hasBranchDivergence())
+    if (TTI->hasBranchDivergence())
       CurDAG->VerifyDAGDivergence();
 #endif
 
@@ -928,7 +1035,7 @@ void SelectionDAGISel::CodeGenAndEmitDAG() {
               CurDAG->dump());
 
 #ifndef NDEBUG
-    if (TTI.hasBranchDivergence())
+    if (TTI->hasBranchDivergence())
       CurDAG->VerifyDAGDivergence();
 #endif
   }
@@ -948,7 +1055,7 @@ void SelectionDAGISel::CodeGenAndEmitDAG() {
             CurDAG->dump());
 
 #ifndef NDEBUG
-  if (TTI.hasBranchDivergence())
+  if (TTI->hasBranchDivergence())
     CurDAG->VerifyDAGDivergence();
 #endif
 
@@ -968,7 +1075,7 @@ void SelectionDAGISel::CodeGenAndEmitDAG() {
             CurDAG->dump());
 
 #ifndef NDEBUG
-  if (TTI.hasBranchDivergence())
+  if (TTI->hasBranchDivergence())
     CurDAG->VerifyDAGDivergence();
 #endif
 
@@ -1554,7 +1661,6 @@ void SelectionDAGISel::SelectAllBasicBlocks(const Function &Fn) {
   }
 
   // Iterate over all basic blocks in the function.
-  StackProtector &SP = getAnalysis<StackProtector>();
   for (const BasicBlock *LLVMBB : RPOT...
[truncated]

@llvmbot
Copy link
Collaborator

llvmbot commented Mar 1, 2024

@llvm/pr-subscribers-llvm-selectiondag

Author: None (paperchalice)

Changes

It currently uses X86 as a proof of concept, will propagate to all targets until it looks good. Tests with --stop=after=x86-isel and without -verify-machineinstrs looks good.


Patch is 28.03 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/83567.diff

12 Files Affected:

  • (modified) llvm/include/llvm/CodeGen/SelectionDAG.h (+12)
  • (modified) llvm/include/llvm/CodeGen/SelectionDAGISel.h (+40-5)
  • (modified) llvm/include/llvm/CodeGen/StackProtector.h (+2)
  • (modified) llvm/lib/CodeGen/GCRootLowering.cpp (+3)
  • (modified) llvm/lib/CodeGen/MachinePassManager.cpp (+1-1)
  • (modified) llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp (+159-53)
  • (modified) llvm/lib/Target/X86/X86.h (+1-1)
  • (modified) llvm/lib/Target/X86/X86CodeGenPassBuilder.cpp (+12-1)
  • (modified) llvm/lib/Target/X86/X86ISelDAGToDAG.cpp (+19-8)
  • (added) llvm/lib/Target/X86/X86ISelDAGToDAG.h (+25)
  • (modified) llvm/lib/Target/X86/X86TargetMachine.cpp (+1-1)
  • (modified) llvm/lib/Target/X86/X86TargetMachine.h (+3)
diff --git a/llvm/include/llvm/CodeGen/SelectionDAG.h b/llvm/include/llvm/CodeGen/SelectionDAG.h
index 25e6c525b672a1..db256e5e3d3c5e 100644
--- a/llvm/include/llvm/CodeGen/SelectionDAG.h
+++ b/llvm/include/llvm/CodeGen/SelectionDAG.h
@@ -29,6 +29,7 @@
 #include "llvm/CodeGen/ISDOpcodes.h"
 #include "llvm/CodeGen/MachineFunction.h"
 #include "llvm/CodeGen/MachineMemOperand.h"
+#include "llvm/CodeGen/MachinePassManager.h"
 #include "llvm/CodeGen/SelectionDAGNodes.h"
 #include "llvm/CodeGen/ValueTypes.h"
 #include "llvm/CodeGenTypes/MachineValueType.h"
@@ -229,6 +230,7 @@ class SelectionDAG {
   const TargetLibraryInfo *LibInfo = nullptr;
   const FunctionVarLocs *FnVarLocs = nullptr;
   MachineFunction *MF;
+  MachineFunctionAnalysisManager *MFAM = nullptr;
   Pass *SDAGISelPass = nullptr;
   LLVMContext *Context;
   CodeGenOptLevel OptLevel;
@@ -458,6 +460,15 @@ class SelectionDAG {
             UniformityInfo *UA, ProfileSummaryInfo *PSIin,
             BlockFrequencyInfo *BFIin, FunctionVarLocs const *FnVarLocs);
 
+  void init(MachineFunction &NewMF, OptimizationRemarkEmitter &NewORE,
+            MachineFunctionAnalysisManager &AM,
+            const TargetLibraryInfo *LibraryInfo, UniformityInfo *UA,
+            ProfileSummaryInfo *PSIin, BlockFrequencyInfo *BFIin,
+            FunctionVarLocs const *FnVarLocs) {
+    init(NewMF, NewORE, nullptr, LibraryInfo, UA, PSIin, BFIin, FnVarLocs);
+    MFAM = &AM;
+  }
+
   void setFunctionLoweringInfo(FunctionLoweringInfo * FuncInfo) {
     FLI = FuncInfo;
   }
@@ -468,6 +479,7 @@ class SelectionDAG {
 
   MachineFunction &getMachineFunction() const { return *MF; }
   const Pass *getPass() const { return SDAGISelPass; }
+  MachineFunctionAnalysisManager *getMFAM() { return MFAM; }
 
   const DataLayout &getDataLayout() const { return MF->getDataLayout(); }
   const TargetMachine &getTarget() const { return TM; }
diff --git a/llvm/include/llvm/CodeGen/SelectionDAGISel.h b/llvm/include/llvm/CodeGen/SelectionDAGISel.h
index dbd9b391f4a431..1157e66f282467 100644
--- a/llvm/include/llvm/CodeGen/SelectionDAGISel.h
+++ b/llvm/include/llvm/CodeGen/SelectionDAGISel.h
@@ -15,6 +15,7 @@
 #define LLVM_CODEGEN_SELECTIONDAGISEL_H
 
 #include "llvm/CodeGen/MachineFunctionPass.h"
+#include "llvm/CodeGen/MachinePassManager.h"
 #include "llvm/CodeGen/SelectionDAG.h"
 #include "llvm/IR/BasicBlock.h"
 #include <memory>
@@ -24,6 +25,7 @@ class AAResults;
 class AssumptionCache;
 class TargetInstrInfo;
 class TargetMachine;
+class SSPLayoutInfo;
 class SelectionDAGBuilder;
 class SDValue;
 class MachineRegisterInfo;
@@ -31,6 +33,7 @@ class MachineFunction;
 class OptimizationRemarkEmitter;
 class TargetLowering;
 class TargetLibraryInfo;
+class TargetTransformInfo;
 class FunctionLoweringInfo;
 class SwiftErrorValueTracking;
 class GCFunctionInfo;
@@ -38,7 +41,7 @@ class ScheduleDAGSDNodes;
 
 /// SelectionDAGISel - This is the common base class used for SelectionDAG-based
 /// pattern-matching instruction selectors.
-class SelectionDAGISel : public MachineFunctionPass {
+class SelectionDAGISel {
 public:
   TargetMachine &TM;
   const TargetLibraryInfo *LibInfo;
@@ -51,6 +54,8 @@ class SelectionDAGISel : public MachineFunctionPass {
   AAResults *AA = nullptr;
   AssumptionCache *AC = nullptr;
   GCFunctionInfo *GFI = nullptr;
+  SSPLayoutInfo *SP = nullptr;
+  TargetTransformInfo *TTI = nullptr;
   CodeGenOptLevel OptLevel;
   const TargetInstrInfo *TII;
   const TargetLowering *TLI;
@@ -67,16 +72,22 @@ class SelectionDAGISel : public MachineFunctionPass {
   /// functions. Storing the filter result here so that we only need to do the
   /// filtering once.
   bool MatchFilterFuncName = false;
+  StringRef FuncName;
 
-  explicit SelectionDAGISel(char &ID, TargetMachine &tm,
+  explicit SelectionDAGISel(TargetMachine &tm,
                             CodeGenOptLevel OL = CodeGenOptLevel::Default);
-  ~SelectionDAGISel() override;
+  ~SelectionDAGISel();
 
   const TargetLowering *getTargetLowering() const { return TLI; }
 
-  void getAnalysisUsage(AnalysisUsage &AU) const override;
+  void initialize(MachineFunctionAnalysisManager &MFAM);
+  void initialize(MachineFunctionPass &MFP);
 
-  bool runOnMachineFunction(MachineFunction &MF) override;
+  /// Subclasses can override this function to set Subtarget
+  /// before running the pass.
+  virtual void prepare(MachineFunction &MF) {}
+
+  bool runOnMachineFunction(MachineFunction &mf, bool Skipped);
 
   virtual void emitFunctionEntryCode() {}
 
@@ -513,6 +524,30 @@ class SelectionDAGISel : public MachineFunctionPass {
                     bool isMorphNodeTo);
 };
 
+class SelectionDAGISelLegacy : public MachineFunctionPass {
+  std::unique_ptr<SelectionDAGISel> Selector;
+
+public:
+  SelectionDAGISelLegacy(char &ID, std::unique_ptr<SelectionDAGISel> S);
+
+  ~SelectionDAGISelLegacy() override = default;
+
+  void getAnalysisUsage(AnalysisUsage &AU) const override;
+
+  bool runOnMachineFunction(MachineFunction &MF) override;
+};
+
+class SelectionDAGISelPass : public PassInfoMixin<SelectionDAGISelPass> {
+  std::unique_ptr<SelectionDAGISel> Selector;
+
+protected:
+  SelectionDAGISelPass(std::unique_ptr<SelectionDAGISel> Selector)
+      : Selector(std::move(Selector)) {}
+
+public:
+  PreservedAnalyses run(MachineFunction &MF,
+                        MachineFunctionAnalysisManager &MFAM);
+};
 }
 
 #endif /* LLVM_CODEGEN_SELECTIONDAGISEL_H */
diff --git a/llvm/include/llvm/CodeGen/StackProtector.h b/llvm/include/llvm/CodeGen/StackProtector.h
index eb5d9d0caebc60..dfafc781067d71 100644
--- a/llvm/include/llvm/CodeGen/StackProtector.h
+++ b/llvm/include/llvm/CodeGen/StackProtector.h
@@ -109,6 +109,8 @@ class StackProtector : public FunctionPass {
 
   StackProtector();
 
+  SSPLayoutInfo &getLayoutInfo() { return LayoutInfo; }
+
   void getAnalysisUsage(AnalysisUsage &AU) const override;
 
   // Return true if StackProtector is supposed to be handled by SelectionDAG.
diff --git a/llvm/lib/CodeGen/GCRootLowering.cpp b/llvm/lib/CodeGen/GCRootLowering.cpp
index 894ab9a0486a7b..0d82f2bab8960b 100644
--- a/llvm/lib/CodeGen/GCRootLowering.cpp
+++ b/llvm/lib/CodeGen/GCRootLowering.cpp
@@ -81,6 +81,9 @@ class GCMachineCodeAnalysis : public MachineFunctionPass {
 
 PreservedAnalyses GCLoweringPass::run(Function &F,
                                       FunctionAnalysisManager &FAM) {
+  if (!F.hasGC())
+    return PreservedAnalyses::all();
+
   auto &Info = FAM.getResult<GCFunctionAnalysis>(F);
 
   bool Changed = DoLowering(F, Info.getStrategy());
diff --git a/llvm/lib/CodeGen/MachinePassManager.cpp b/llvm/lib/CodeGen/MachinePassManager.cpp
index 9a750b5bed4339..96556d549a2548 100644
--- a/llvm/lib/CodeGen/MachinePassManager.cpp
+++ b/llvm/lib/CodeGen/MachinePassManager.cpp
@@ -79,7 +79,7 @@ ModuleToMachineFunctionPassAdaptor::run(Module &M, ModuleAnalysisManager &AM) {
   for (Function &F : M) {
     // Do not codegen any 'available_externally' functions at all, they have
     // definitions outside the translation unit.
-    if (F.hasAvailableExternallyLinkage())
+    if (F.hasAvailableExternallyLinkage() || F.isDeclaration())
       continue;
 
     MachineFunction &MF = MMI.getOrCreateMachineFunction(F);
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
index 9b5ab4267b80e9..81e03b885fee03 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
@@ -341,9 +341,34 @@ void TargetLowering::AdjustInstrPostInstrSelection(MachineInstr &MI,
 // SelectionDAGISel code
 //===----------------------------------------------------------------------===//
 
-SelectionDAGISel::SelectionDAGISel(char &ID, TargetMachine &tm,
-                                   CodeGenOptLevel OL)
-    : MachineFunctionPass(ID), TM(tm), FuncInfo(new FunctionLoweringInfo()),
+SelectionDAGISelLegacy::SelectionDAGISelLegacy(
+    char &ID, std::unique_ptr<SelectionDAGISel> S)
+    : MachineFunctionPass(ID), Selector(std::move(S)) {
+  initializeGCModuleInfoPass(*PassRegistry::getPassRegistry());
+  initializeBranchProbabilityInfoWrapperPassPass(
+      *PassRegistry::getPassRegistry());
+  initializeAAResultsWrapperPassPass(*PassRegistry::getPassRegistry());
+  initializeTargetLibraryInfoWrapperPassPass(*PassRegistry::getPassRegistry());
+}
+
+bool SelectionDAGISelLegacy::runOnMachineFunction(MachineFunction &MF) {
+  // If we already selected that function, we do not need to run SDISel.
+  if (MF.getProperties().hasProperty(
+          MachineFunctionProperties::Property::Selected))
+      return false;
+
+  // Do some sanity-checking on the command-line options.
+  assert((!EnableFastISelAbort || Selector->TM.Options.EnableFastISel) &&
+         "-fast-isel-abort > 0 requires -fast-isel");
+
+  Selector->MF = &MF;
+  Selector->setSubtarget(MF);
+  Selector->initialize(*this);
+  return Selector->runOnMachineFunction(MF, skipFunction(MF.getFunction()));
+}
+
+SelectionDAGISel::SelectionDAGISel(TargetMachine &tm, CodeGenOptLevel OL)
+    : TM(tm), FuncInfo(new FunctionLoweringInfo()),
       SwiftError(new SwiftErrorValueTracking()),
       CurDAG(new SelectionDAG(tm, OL)),
       SDB(std::make_unique<SelectionDAGBuilder>(*CurDAG, *FuncInfo, *SwiftError,
@@ -361,7 +386,8 @@ SelectionDAGISel::~SelectionDAGISel() {
   delete SwiftError;
 }
 
-void SelectionDAGISel::getAnalysisUsage(AnalysisUsage &AU) const {
+void SelectionDAGISelLegacy::getAnalysisUsage(AnalysisUsage &AU) const {
+  const auto &OptLevel = Selector->OptLevel;
   if (OptLevel != CodeGenOptLevel::None)
       AU.addRequired<AAResultsWrapperPass>();
   AU.addRequired<GCModuleInfo>();
@@ -406,64 +432,117 @@ static void computeUsesMSVCFloatingPoint(const Triple &TT, const Function &F,
   }
 }
 
-bool SelectionDAGISel::runOnMachineFunction(MachineFunction &mf) {
+PreservedAnalyses
+SelectionDAGISelPass::run(MachineFunction &MF,
+                          MachineFunctionAnalysisManager &MFAM) {
   // If we already selected that function, we do not need to run SDISel.
-  if (mf.getProperties().hasProperty(
+  if (MF.getProperties().hasProperty(
           MachineFunctionProperties::Property::Selected))
-    return false;
+    return PreservedAnalyses::all();
+
   // Do some sanity-checking on the command-line options.
-  assert((!EnableFastISelAbort || TM.Options.EnableFastISel) &&
+  assert((!EnableFastISelAbort || Selector->TM.Options.EnableFastISel) &&
          "-fast-isel-abort > 0 requires -fast-isel");
 
-  const Function &Fn = mf.getFunction();
-  MF = &mf;
+  Selector->MF = &MF;
+  Selector->prepare(MF);
+  Selector->initialize(MFAM);
+  Selector->runOnMachineFunction(MF, false);
+
+  return PreservedAnalyses::none();
+}
 
+void SelectionDAGISel::initialize(MachineFunctionAnalysisManager &MFAM) {
+  auto &FAM = MFAM.getResult<FunctionAnalysisManagerMachineFunctionProxy>(*MF)
+                  .getManager();
+  auto &MAMP = MFAM.getResult<ModuleAnalysisManagerMachineFunctionProxy>(*MF);
+  Function &Fn = MF->getFunction();
 #ifndef NDEBUG
-  StringRef FuncName = Fn.getName();
+  FuncName = Fn.getName();
   MatchFilterFuncName = isFunctionInPrintList(FuncName);
 #else
   (void)MatchFilterFuncName;
 #endif
 
-  // Decide what flavour of variable location debug-info will be used, before
-  // we change the optimisation level.
-  bool InstrRef = mf.shouldUseDebugInstrRef();
-  mf.setUseDebugInstrRef(InstrRef);
+  TII = MF->getSubtarget().getInstrInfo();
+  TLI = MF->getSubtarget().getTargetLowering();
+  RegInfo = &MF->getRegInfo();
+  LibInfo = &FAM.getResult<TargetLibraryAnalysis>(Fn);
+  GFI = Fn.hasGC() ? &FAM.getResult<GCFunctionAnalysis>(Fn) : nullptr;
+  ORE = std::make_unique<OptimizationRemarkEmitter>(&Fn);
+  AC = &FAM.getResult<AssumptionAnalysis>(Fn);
+  auto *PSI = MAMP.getCachedResult<ProfileSummaryAnalysis>(*Fn.getParent());
+  BlockFrequencyInfo *BFI = nullptr;
+  FAM.getResult<BlockFrequencyAnalysis>(Fn);
+  if (PSI && PSI->hasProfileSummary() && OptLevel != CodeGenOptLevel::None)
+    BFI = &FAM.getResult<BlockFrequencyAnalysis>(Fn);
 
-  // Reset the target options before resetting the optimization
-  // level below.
-  // FIXME: This is a horrible hack and should be processed via
-  // codegen looking at the optimization level explicitly when
-  // it wants to look at it.
-  TM.resetTargetOptions(Fn);
-  // Reset OptLevel to None for optnone functions.
-  CodeGenOptLevel NewOptLevel = OptLevel;
-  if (OptLevel != CodeGenOptLevel::None && skipFunction(Fn))
-    NewOptLevel = CodeGenOptLevel::None;
-  OptLevelChanger OLC(*this, NewOptLevel);
+  FunctionVarLocs const *FnVarLocs = nullptr;
+  if (isAssignmentTrackingEnabled(*Fn.getParent()))
+    FnVarLocs = &FAM.getResult<DebugAssignmentTrackingAnalysis>(Fn);
+
+  ISEL_DUMP(dbgs() << "\n\n\n=== " << FuncName << "\n");
+
+  auto *UA = FAM.getCachedResult<UniformityInfoAnalysis>(Fn);
+  CurDAG->init(*MF, *ORE, MFAM, LibInfo, UA, PSI, BFI, FnVarLocs);
+  FuncInfo->set(Fn, *MF, CurDAG);
+  SwiftError->setFunction(*MF);
+
+  // Now get the optional analyzes if we want to.
+  // This is based on the possibly changed OptLevel (after optnone is taken
+  // into account).  That's unfortunate but OK because it just means we won't
+  // ask for passes that have been required anyway.
+
+  FAM.getResult<BranchProbabilityAnalysis>(Fn);
+  if (UseMBPI && OptLevel != CodeGenOptLevel::None)
+    FuncInfo->BPI = &FAM.getResult<BranchProbabilityAnalysis>(Fn);
+  else
+    FuncInfo->BPI = nullptr;
+
+  if (OptLevel != CodeGenOptLevel::None)
+    AA = &FAM.getResult<AAManager>(Fn);
+  else
+    AA = nullptr;
+
+  SP = &FAM.getResult<SSPLayoutAnalysis>(Fn);
+
+#ifndef NDEBUG
+  TTI = &FAM.getResult<TargetIRAnalysis>(Fn);
+#endif
+}
+
+void SelectionDAGISel::initialize(MachineFunctionPass &MFP) {
+  Function &Fn = MF->getFunction();
+#ifndef NDEBUG
+  FuncName = Fn.getName();
+  MatchFilterFuncName = isFunctionInPrintList(FuncName);
+#else
+  (void)MatchFilterFuncName;
+#endif
 
   TII = MF->getSubtarget().getInstrInfo();
   TLI = MF->getSubtarget().getTargetLowering();
   RegInfo = &MF->getRegInfo();
-  LibInfo = &getAnalysis<TargetLibraryInfoWrapperPass>().getTLI(Fn);
-  GFI = Fn.hasGC() ? &getAnalysis<GCModuleInfo>().getFunctionInfo(Fn) : nullptr;
+  LibInfo = &MFP.getAnalysis<TargetLibraryInfoWrapperPass>().getTLI(Fn);
+  GFI = Fn.hasGC() ? &MFP.getAnalysis<GCModuleInfo>().getFunctionInfo(Fn)
+                   : nullptr;
   ORE = std::make_unique<OptimizationRemarkEmitter>(&Fn);
-  AC = &getAnalysis<AssumptionCacheTracker>().getAssumptionCache(mf.getFunction());
-  auto *PSI = &getAnalysis<ProfileSummaryInfoWrapperPass>().getPSI();
+  AC = &MFP.getAnalysis<AssumptionCacheTracker>().getAssumptionCache(Fn);
+  auto *PSI = &MFP.getAnalysis<ProfileSummaryInfoWrapperPass>().getPSI();
   BlockFrequencyInfo *BFI = nullptr;
   if (PSI && PSI->hasProfileSummary() && OptLevel != CodeGenOptLevel::None)
-    BFI = &getAnalysis<LazyBlockFrequencyInfoPass>().getBFI();
+    BFI = &MFP.getAnalysis<LazyBlockFrequencyInfoPass>().getBFI();
 
   FunctionVarLocs const *FnVarLocs = nullptr;
   if (isAssignmentTrackingEnabled(*Fn.getParent()))
-    FnVarLocs = getAnalysis<AssignmentTrackingAnalysis>().getResults();
+    FnVarLocs = MFP.getAnalysis<AssignmentTrackingAnalysis>().getResults();
 
   ISEL_DUMP(dbgs() << "\n\n\n=== " << FuncName << "\n");
 
   UniformityInfo *UA = nullptr;
-  if (auto *UAPass = getAnalysisIfAvailable<UniformityInfoWrapperPass>())
+  if (auto *UAPass = MFP.getAnalysisIfAvailable<UniformityInfoWrapperPass>())
     UA = &UAPass->getUniformityInfo();
-  CurDAG->init(*MF, *ORE, this, LibInfo, UA, PSI, BFI, FnVarLocs);
+  CurDAG->init(*MF, *ORE, &MFP, LibInfo, UA, PSI, BFI, FnVarLocs);
   FuncInfo->set(Fn, *MF, CurDAG);
   SwiftError->setFunction(*MF);
 
@@ -473,15 +552,46 @@ bool SelectionDAGISel::runOnMachineFunction(MachineFunction &mf) {
   // ask for passes that have been required anyway.
 
   if (UseMBPI && OptLevel != CodeGenOptLevel::None)
-    FuncInfo->BPI = &getAnalysis<BranchProbabilityInfoWrapperPass>().getBPI();
+    FuncInfo->BPI =
+        &MFP.getAnalysis<BranchProbabilityInfoWrapperPass>().getBPI();
   else
     FuncInfo->BPI = nullptr;
 
   if (OptLevel != CodeGenOptLevel::None)
-    AA = &getAnalysis<AAResultsWrapperPass>().getAAResults();
+    AA = &MFP.getAnalysis<AAResultsWrapperPass>().getAAResults();
   else
     AA = nullptr;
 
+  SP = &MFP.getAnalysis<StackProtector>().getLayoutInfo();
+
+#ifndef NDEBUG
+  TTI =
+      &MFP.getAnalysis<TargetTransformInfoWrapperPass>().getTTI(*FuncInfo->Fn);
+#endif
+}
+
+bool SelectionDAGISel::runOnMachineFunction(MachineFunction &mf, bool Skipped) {
+  const Function &Fn = mf.getFunction();
+
+  // Decide what flavour of variable location debug-info will be used, before
+  // we change the optimisation level.
+  bool InstrRef = mf.shouldUseDebugInstrRef();
+  mf.setUseDebugInstrRef(InstrRef);
+
+  // Reset the target options before resetting the optimization
+  // level below.
+  // FIXME: This is a horrible hack and should be processed via
+  // codegen looking at the optimization level explicitly when
+  // it wants to look at it.
+  TM.resetTargetOptions(Fn);
+  // Reset OptLevel to None for optnone functions.
+  CodeGenOptLevel NewOptLevel = OptLevel;
+  if (OptLevel != CodeGenOptLevel::None && Skipped)
+    NewOptLevel = CodeGenOptLevel::None;
+  OptLevelChanger OLC(*this, NewOptLevel);
+
+  ISEL_DUMP(dbgs() << "\n\n\n=== " << FuncName << "\n");
+
   SDB->init(GFI, AA, AC, LibInfo);
 
   MF->setHasInlineAsm(false);
@@ -779,11 +889,8 @@ void SelectionDAGISel::CodeGenAndEmitDAG() {
   StringRef GroupName = "sdag";
   StringRef GroupDescription = "Instruction Selection and Scheduling";
   std::string BlockName;
-  bool MatchFilterBB = false; (void)MatchFilterBB;
-#ifndef NDEBUG
-  TargetTransformInfo &TTI =
-      getAnalysis<TargetTransformInfoWrapperPass>().getTTI(*FuncInfo->Fn);
-#endif
+  bool MatchFilterBB = false;
+  (void)MatchFilterBB;
 
   // Pre-type legalization allow creation of any node types.
   CurDAG->NewNodesMustHaveLegalTypes = false;
@@ -808,7 +915,7 @@ void SelectionDAGISel::CodeGenAndEmitDAG() {
             CurDAG->dump());
 
 #ifndef NDEBUG
-  if (TTI.hasBranchDivergence())
+  if (TTI->hasBranchDivergence())
     CurDAG->VerifyDAGDivergence();
 #endif
 
@@ -828,7 +935,7 @@ void SelectionDAGISel::CodeGenAndEmitDAG() {
             CurDAG->dump());
 
 #ifndef NDEBUG
-  if (TTI.hasBranchDivergence())
+  if (TTI->hasBranchDivergence())
     CurDAG->VerifyDAGDivergence();
 #endif
 
@@ -850,7 +957,7 @@ void SelectionDAGISel::CodeGenAndEmitDAG() {
             CurDAG->dump());
 
 #ifndef NDEBUG
-  if (TTI.hasBranchDivergence())
+  if (TTI->hasBranchDivergence())
     CurDAG->VerifyDAGDivergence();
 #endif
 
@@ -874,7 +981,7 @@ void SelectionDAGISel::CodeGenAndEmitDAG() {
               CurDAG->dump());
 
 #ifndef NDEBUG
-    if (TTI.hasBranchDivergence())
+    if (TTI->hasBranchDivergence())
       CurDAG->VerifyDAGDivergence();
 #endif
   }
@@ -892,7 +999,7 @@ void SelectionDAGISel::CodeGenAndEmitDAG() {
               CurDAG->dump());
 
 #ifndef NDEBUG
-    if (TTI.hasBranchDivergence())
+    if (TTI->hasBranchDivergence())
       CurDAG->VerifyDAGDivergence();
 #endif
 
@@ -908,7 +1015,7 @@ void SelectionDAGISel::CodeGenAndEmitDAG() {
               CurDAG->dump());
 
 #ifndef NDEBUG
-    if (TTI.hasBranchDivergence())
+    if (TTI->hasBranchDivergence())
       CurDAG->VerifyDAGDivergence();
 #endif
 
@@ -928,7 +1035,7 @@ void SelectionDAGISel::CodeGenAndEmitDAG() {
               CurDAG->dump());
 
 #ifndef NDEBUG
-    if (TTI.hasBranchDivergence())
+    if (TTI->hasBranchDivergence())
       CurDAG->VerifyDAGDivergence();
 #endif
   }
@@ -948,7 +1055,7 @@ void SelectionDAGISel::CodeGenAndEmitDAG() {
             CurDAG->dump());
 
 #ifndef NDEBUG
-  if (TTI.hasBranchDivergence())
+  if (TTI->hasBranchDivergence())
     CurDAG->VerifyDAGDivergence();
 #endif
 
@@ -968,7 +1075,7 @@ void SelectionDAGISel::CodeGenAndEmitDAG() {
             CurDAG->dump());
 
 #ifndef NDEBUG
-  if (TTI.hasBranchDivergence())
+  if (TTI->hasBranchDivergence())
     CurDAG->VerifyDAGDivergence();
 #endif
 
@@ -1554,7 +1661,6 @@ void SelectionDAGISel::SelectAllBasicBlocks(const Function &Fn) {
   }
 
   // Iterate over all basic blocks in the function.
-  StackProtector &SP = getAnalysis<StackProtector>();
   for (const BasicBlock *LLVMBB : RPOT...
[truncated]

Copy link

github-actions bot commented Mar 1, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

Comment on lines 365 to 366
Selector->prepare(MF);
Selector->initialize(*this);
Copy link
Contributor

Choose a reason for hiding this comment

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

Having both "prepare" and "initialize" feels redundant

Copy link
Contributor

@aeubanks aeubanks left a comment

Choose a reason for hiding this comment

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

this should definitely have a test to make sure things generally work

@@ -229,6 +230,7 @@ class SelectionDAG {
const TargetLibraryInfo *LibInfo = nullptr;
const FunctionVarLocs *FnVarLocs = nullptr;
MachineFunction *MF;
MachineFunctionAnalysisManager *MFAM = nullptr;
Copy link
Contributor

Choose a reason for hiding this comment

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

SDAGISelPass is only used in SIISelLowering.cpp, I don't think we need this for now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

AMDGPU backend needs some analysis results.

auto &ArgUsageInfo =
DAG.getPass()->getAnalysis<AMDGPUArgumentUsageInfo>();
ArgUsageInfo.setFuncArgInfo(Fn, Info->getArgInfo());

llvm/lib/CodeGen/GCRootLowering.cpp Outdated Show resolved Hide resolved
llvm/lib/CodeGen/MachinePassManager.cpp Outdated Show resolved Hide resolved
// If we already selected that function, we do not need to run SDISel.
if (mf.getProperties().hasProperty(
if (MF.getProperties().hasProperty(
Copy link
Contributor

Choose a reason for hiding this comment

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

unrelated, but do you know why we even have this check? seems like we should only ever run this once on any given function

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is introduced in e063e1f, GlobalISel may fallback to selection dag.

return Error::success();
}

} // namespace

void X86TargetMachine::registerPassBuilderCallbacks(
PassBuilder &PB, bool PopulateClassToPassNames) {
if (PopulateClassToPassNames) {
Copy link
Contributor

Choose a reason for hiding this comment

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

should we generalize the PassRegistry.def mechanism so each backend doesn't have to repeat code like

  PB.registerPipelineParsingCallback(
      [](StringRef PassName, ModulePassManager &PM,
         ArrayRef<PassBuilder::PipelineElement>) {
        if (PassName == "print-dxil-resource") {
          PM.addPass(DXILResourcePrinterPass(dbgs()));
          return true;
        }
        if (PassName == "print-dx-shader-flags") {
          PM.addPass(dxil::ShaderFlagsAnalysisPrinter(dbgs()));
          return true;
        }
        return false;
      });

and can more easily populate class names?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, each target should provide <Target>PassRegistry.def, like #70921.

@cdevadas
Copy link
Collaborator

cdevadas commented Mar 8, 2024

Is there any npm-codegen channel/team that exists?
I would be interested in getting a notification for PRs related to this.

@paperchalice
Copy link
Contributor Author

Is there any npm-codegen channel/team that exists? I would be interested in getting a notification for PRs related to this.

Currently no, but there are indeed some people interested in related PRs, a new PR label would be good.

@cdevadas
Copy link
Collaborator

Is there any npm-codegen channel/team that exists? I would be interested in getting a notification for PRs related to this.

Currently no, but there are indeed some people interested in related PRs, a new PR label would be good.

Yes, a label like backend::NewPM or backend::NPM would do.

Comment on lines +499 to +531
else
FuncInfo->BPI = nullptr;
Copy link
Contributor

Choose a reason for hiding this comment

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

I would assume this was already null initialized?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Still need to clear it manually, FuncInfo is associated with SelectionDAGISel not this function.

Comment on lines +504 to +536
else
AA = nullptr;
Copy link
Contributor

Choose a reason for hiding this comment

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

Ditto

paperchalice added a commit that referenced this pull request Mar 21, 2024
PR #83567 ports `SelectionDAGISel` to the new pass manager, then each
backend should provide `<Target>DagToDagISel()` in new pass manager
style. Then each target should provide `<Target>PassRegistry.def` to
register backend passes in `registerPassBuilderCallbacks` to reduce
duplicate code.
This PR adds `AArch64PassRegistry.def` to AArch64 backend and
boilerplate code in `registerPassBuilderCallbacks`.
chencha3 pushed a commit to chencha3/llvm-project that referenced this pull request Mar 23, 2024
PR llvm#83567 ports `SelectionDAGISel` to the new pass manager, then each
backend should provide `<Target>DagToDagISel()` in new pass manager
style. Then each target should provide `<Target>PassRegistry.def` to
register backend passes in `registerPassBuilderCallbacks` to reduce
duplicate code.
This PR adds `AArch64PassRegistry.def` to AArch64 backend and
boilerplate code in `registerPassBuilderCallbacks`.
@paperchalice paperchalice marked this pull request as draft April 11, 2024 05:42
@paperchalice paperchalice changed the title [CodeGen] Port selection dag isel to new pass manager [NewPM][CodeGen] Port selection dag isel to new pass manager Apr 11, 2024
@paperchalice paperchalice force-pushed the NPM/CodeGen/dag-isel branch 4 times, most recently from 3330a52 to 7af9b12 Compare April 12, 2024 04:39
@paperchalice paperchalice marked this pull request as ready for review April 12, 2024 04:51
Copy link
Contributor

@arsenm arsenm left a comment

Choose a reason for hiding this comment

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

Should add some AMDGPU tests since it has the exotic analysis requirement

Comment on lines 361 to 362
assert((!EnableFastISelAbort || Selector->TM.Options.EnableFastISel) &&
"-fast-isel-abort > 0 requires -fast-isel");
Copy link
Contributor

Choose a reason for hiding this comment

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

This shouldn't be an assert. If we want to do something other than press on, should be a proper error

Comment on lines 375 to 378
CodeGenOptLevel NewOptLevel = Selector->OptLevel;
if (Selector->OptLevel != CodeGenOptLevel::None &&
skipFunction(MF.getFunction()))
NewOptLevel = CodeGenOptLevel::None;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
CodeGenOptLevel NewOptLevel = Selector->OptLevel;
if (Selector->OptLevel != CodeGenOptLevel::None &&
skipFunction(MF.getFunction()))
NewOptLevel = CodeGenOptLevel::None;
CodeGenOptLevel NewOptLevel = skipFunction(MF.getFunction()) ?
CodeGenOptLevel::None : Selector->OptLevel;

@@ -361,7 +402,8 @@ SelectionDAGISel::~SelectionDAGISel() {
delete SwiftError;
}

void SelectionDAGISel::getAnalysisUsage(AnalysisUsage &AU) const {
void SelectionDAGISelLegacy::getAnalysisUsage(AnalysisUsage &AU) const {
const auto &OptLevel = Selector->OptLevel;
Copy link
Contributor

Choose a reason for hiding this comment

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

No reference? no auto?


FuncInfo->set(MF->getFunction(), *MF, CurDAG);

ISEL_DUMP(dbgs() << "\n\n\n=== " << FuncName << "\n");
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
ISEL_DUMP(dbgs() << "\n\n\n=== " << FuncName << "\n");
ISEL_DUMP(dbgs() << "\n\n\n=== " << FuncName << '\n');

@paperchalice
Copy link
Contributor Author

Use new pass manager in some AMDGPU tests.

@paperchalice paperchalice merged commit d2cdc8a into llvm:main Jun 2, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:X86 llvm:SelectionDAG SelectionDAGISel as well
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants