[NewPM][CodeGen] Add `MachineFunctionAnalysis` #88610

paperchalice · 2024-04-13T09:48:53Z

In new pass system, MachineFunction could be an analysis result again, machine module pass can now fetch them from analysis manager. MachineModuleInfo no longer owns them.
Remove FreeMachineFunctionPass, replaced by InvalidateAnalysisPass<MachineFunctionAnalysis>.

Now FreeMachineFunction is replaced by InvalidateAnalysisPass<MachineFunctionAnalysis>, the workaround in MachineFunctionPassManager is no longer needed, there is no difference between unittests/MIR/PassBuilderCallbacksTest.cpp and unittests/IR/PassBuilderCallbacksTest.cpp.

arsenm

Moving things out of MachineModuleInfo is good

aeubanks

this looks good in general

aeubanks · 2024-04-15T16:56:34Z

llvm/include/llvm/CodeGen/FreeMachineFunction.h

@@ -13,6 +13,7 @@

 namespace llvm {

+// TODO: Convert it to function pass.


this pass can be replaced with InvalidateAnalysisPass<MachineFunctionAnalysis> once everything else is in place

Still need somewhere to run runAfterPassInvalidated, perhaps need a specialization for InvalidateAnalysisPass<MachineFunctionAnalysis>.

I don't think we need runAfterPassInvalidated, since MachineFunction is analysis that will just disappear. from the Function point of view, there is no IR invalidation happening, just analysis invalidation

aeubanks · 2024-04-15T17:01:39Z

llvm/lib/CodeGen/MachineFunctionAnalysis.cpp

+MachineFunctionAnalysis::Result
+MachineFunctionAnalysis::run(Function &F, FunctionAnalysisManager &FAM) {
+  /// Next unique number available for a MachineFunction.
+  static unsigned NextFnNum = 0;


this seems problematic. I believe people use multiple LLVMContexts in a process and are able to parallelize optimization/codegen that way.

perhaps we should put this in LLVMContext? unsure

Checked the relevant code, this number's uniqueness is for the current module.

aeubanks · 2024-04-15T17:02:15Z

llvm/include/llvm/CodeGen/MachineFunctionAnalysis.h

+class MachineFunction;
+class LLVMTargetMachine;
+
+class MachineFunctionAnalysis


this name is confusing because we have MachineFunctionAnalysisManager which is at a different level

but I'm not sure of a better name :)

paperchalice · 2024-04-16T04:02:24Z

Move MachineFunctionNum generation to LLVMContext.

llvmbot · 2024-04-16T12:58:17Z

@llvm/pr-subscribers-llvm-ir

Author: None (paperchalice)

Changes

In new pass system, MachineFunction could be an analysis result again, machine module pass can now fetch them from analysis manager. MachineModuleInfo no longer owns them.
FreeMachineFunctionPass may not suitable as a machine function pass, but it need adaptor.

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

19 Files Affected:

(modified) llvm/include/llvm/CodeGen/FreeMachineFunction.h (+1)
(added) llvm/include/llvm/CodeGen/FunctionToMachineFunctionAnalysis.h (+50)
(modified) llvm/include/llvm/CodeGen/MIRParser/MIRParser.h (+12)
(modified) llvm/include/llvm/CodeGen/MachineModuleInfo.h (+4)
(modified) llvm/include/llvm/CodeGen/MachinePassManager.h (+4)
(modified) llvm/include/llvm/IR/LLVMContext.h (+7)
(modified) llvm/lib/CodeGen/CMakeLists.txt (+1)
(modified) llvm/lib/CodeGen/FreeMachineFunction.cpp (+4-4)
(added) llvm/lib/CodeGen/FunctionToMachineFunctionAnalysis.cpp (+48)
(modified) llvm/lib/CodeGen/MIRParser/MIRParser.cpp (+37-12)
(modified) llvm/lib/CodeGen/MachinePassManager.cpp (+22-17)
(modified) llvm/lib/IR/LLVMContext.cpp (+6)
(modified) llvm/lib/IR/LLVMContextImpl.h (+4)
(modified) llvm/lib/Passes/PassBuilder.cpp (+1)
(modified) llvm/lib/Passes/PassRegistry.def (+3)
(modified) llvm/lib/Passes/StandardInstrumentations.cpp (-8)
(modified) llvm/tools/llc/NewPMDriver.cpp (+2-2)
(modified) llvm/unittests/CodeGen/PassManagerTest.cpp (+1-1)
(modified) llvm/unittests/MIR/PassBuilderCallbacksTest.cpp (+4-7)

diff --git a/llvm/include/llvm/CodeGen/FreeMachineFunction.h b/llvm/include/llvm/CodeGen/FreeMachineFunction.h
index 5f21c6720350bc..9471b7a00e4472 100644
--- a/llvm/include/llvm/CodeGen/FreeMachineFunction.h
+++ b/llvm/include/llvm/CodeGen/FreeMachineFunction.h
@@ -13,6 +13,7 @@
 
 namespace llvm {
 
+// TODO: Convert it to function pass.
 class FreeMachineFunctionPass : public PassInfoMixin<FreeMachineFunctionPass> {
 public:
   PreservedAnalyses run(MachineFunction &MF,
diff --git a/llvm/include/llvm/CodeGen/FunctionToMachineFunctionAnalysis.h b/llvm/include/llvm/CodeGen/FunctionToMachineFunctionAnalysis.h
new file mode 100644
index 00000000000000..a4cfc305ed9ffe
--- /dev/null
+++ b/llvm/include/llvm/CodeGen/FunctionToMachineFunctionAnalysis.h
@@ -0,0 +1,50 @@
+//===- llvm/CodeGen/FunctionToMachineFunctionAnalysis.h ---------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file declares the FunctionToMachineFunctionAnalysis class.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CODEGEN_FUNCTIONTOMACHINEFUNCTIONANALYSIS
+#define LLVM_CODEGEN_FUNCTIONTOMACHINEFUNCTIONANALYSIS
+
+#include "llvm/IR/PassManager.h"
+
+namespace llvm {
+
+class MachineFunction;
+class LLVMTargetMachine;
+
+/// This analysis create MachineFunction for given Function.
+/// To release the MachineFunction, users should invalidate it explicitly.
+class FunctionToMachineFunctionAnalysis
+    : public AnalysisInfoMixin<FunctionToMachineFunctionAnalysis> {
+  friend AnalysisInfoMixin<FunctionToMachineFunctionAnalysis>;
+
+  static AnalysisKey Key;
+
+  const LLVMTargetMachine *TM;
+
+public:
+  class Result {
+    std::unique_ptr<MachineFunction> MF;
+
+  public:
+    Result(std::unique_ptr<MachineFunction> MF) : MF(std::move(MF)) {}
+    MachineFunction &getMF() { return *MF; };
+    bool invalidate(Function &, const PreservedAnalyses &PA,
+                    FunctionAnalysisManager::Invalidator &);
+  };
+
+  FunctionToMachineFunctionAnalysis(const LLVMTargetMachine *TM) : TM(TM){};
+  Result run(Function &F, FunctionAnalysisManager &FAM);
+};
+
+} // namespace llvm
+
+#endif // LLVM_CODEGEN_FUNCTIONTOMACHINEFUNCTIONANALYSIS
diff --git a/llvm/include/llvm/CodeGen/MIRParser/MIRParser.h b/llvm/include/llvm/CodeGen/MIRParser/MIRParser.h
index e1606e7c0ea72d..ae0938a48a7110 100644
--- a/llvm/include/llvm/CodeGen/MIRParser/MIRParser.h
+++ b/llvm/include/llvm/CodeGen/MIRParser/MIRParser.h
@@ -34,6 +34,9 @@ class MachineModuleInfo;
 class SMDiagnostic;
 class StringRef;
 
+template <typename IRUnitT, typename... ExtraArgTs> class AnalysisManager;
+using ModuleAnalysisManager = AnalysisManager<Module>;
+
 typedef llvm::function_ref<std::optional<std::string>(StringRef, StringRef)>
     DataLayoutCallbackTy;
 
@@ -60,6 +63,15 @@ class MIRParser {
   ///
   /// \returns true if an error occurred.
   bool parseMachineFunctions(Module &M, MachineModuleInfo &MMI);
+
+  /// Parses MachineFunctions in the MIR file and add them as the result
+  /// of MachineFunctionAnalysis in ModulePassManager \p MAM.
+  /// User should register at least MachineFunctionAnalysis,
+  /// MachineModuleAnalysis, FunctionAnalysisManagerModuleProxy and
+  /// PassInstrumentationAnalysis in \p MAM before parsing MIR.
+  ///
+  /// \returns true if an error occurred.
+  bool parseMachineFunctions(Module &M, ModuleAnalysisManager &MAM);
 };
 
 /// This function is the main interface to the MIR serialization format parser.
diff --git a/llvm/include/llvm/CodeGen/MachineModuleInfo.h b/llvm/include/llvm/CodeGen/MachineModuleInfo.h
index 5f66d1ada0d082..92ea3c902ce95e 100644
--- a/llvm/include/llvm/CodeGen/MachineModuleInfo.h
+++ b/llvm/include/llvm/CodeGen/MachineModuleInfo.h
@@ -147,10 +147,14 @@ class MachineModuleInfo {
 
   /// Returns the MachineFunction constructed for the IR function \p F.
   /// Creates a new MachineFunction if none exists yet.
+  /// NOTE: New pass manager clients shall not use this method to get
+  /// the `MachineFunction`, use `MachineFunctionAnalysis` instead.
   MachineFunction &getOrCreateMachineFunction(Function &F);
 
   /// \brief Returns the MachineFunction associated to IR function \p F if there
   /// is one, otherwise nullptr.
+  /// NOTE: New pass manager clients shall not use this method to get
+  /// the `MachineFunction`, use `MachineFunctionAnalysis` instead.
   MachineFunction *getMachineFunction(const Function &F) const;
 
   /// Delete the MachineFunction \p MF and reset the link in the IR Function to
diff --git a/llvm/include/llvm/CodeGen/MachinePassManager.h b/llvm/include/llvm/CodeGen/MachinePassManager.h
index 4f0b6ba2b1e738..f2bb456c94f8cc 100644
--- a/llvm/include/llvm/CodeGen/MachinePassManager.h
+++ b/llvm/include/llvm/CodeGen/MachinePassManager.h
@@ -76,6 +76,10 @@ struct MachinePassModel
 #endif
 
     auto PA = this->Pass.run(IR, AM);
+    // Machine function passes are not allowed to modify the LLVM
+    // representation, therefore we should preserve all IR analyses.
+    PA.template preserveSet<AllAnalysesOn<Module>>();
+    PA.template preserveSet<AllAnalysesOn<Function>>();
 
     if constexpr (is_detected<has_get_set_properties_t, PassT>::value)
       IR.getProperties().set(PassT::getSetProperties());
diff --git a/llvm/include/llvm/IR/LLVMContext.h b/llvm/include/llvm/IR/LLVMContext.h
index e5786afd72214b..bab38e66d7cc17 100644
--- a/llvm/include/llvm/IR/LLVMContext.h
+++ b/llvm/include/llvm/IR/LLVMContext.h
@@ -327,12 +327,19 @@ class LLVMContext {
   // Module needs access to the add/removeModule methods.
   friend class Module;
 
+  // FunctionToMachineFunctionAnalysis need get a MachineFunction ID
+  // in current module.
+  friend class FunctionToMachineFunctionAnalysis;
+
   /// addModule - Register a module as being instantiated in this context.  If
   /// the context is deleted, the module will be deleted as well.
   void addModule(Module*);
 
   /// removeModule - Unregister a module from this context.
   void removeModule(Module*);
+
+  /// getMachineFunctionNum
+  unsigned generateMachineFunctionNum(Function &);
 };
 
 // Create wrappers for C Binding types (see CBindingWrapping.h).
diff --git a/llvm/lib/CodeGen/CMakeLists.txt b/llvm/lib/CodeGen/CMakeLists.txt
index 2c24de60edd43e..6c77ab0b2cef63 100644
--- a/llvm/lib/CodeGen/CMakeLists.txt
+++ b/llvm/lib/CodeGen/CMakeLists.txt
@@ -67,6 +67,7 @@ add_llvm_component_library(LLVMCodeGen
   FixupStatepointCallerSaved.cpp
   FreeMachineFunction.cpp
   FuncletLayout.cpp
+  FunctionToMachineFunctionAnalysis.cpp
   GCMetadata.cpp
   GCMetadataPrinter.cpp
   GCRootLowering.cpp
diff --git a/llvm/lib/CodeGen/FreeMachineFunction.cpp b/llvm/lib/CodeGen/FreeMachineFunction.cpp
index f38f3e63a3f37a..183eceb95c9c1f 100644
--- a/llvm/lib/CodeGen/FreeMachineFunction.cpp
+++ b/llvm/lib/CodeGen/FreeMachineFunction.cpp
@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "llvm/CodeGen/FreeMachineFunction.h"
+#include "llvm/CodeGen/FunctionToMachineFunctionAnalysis.h"
 #include "llvm/CodeGen/MachineFunction.h"
 #include "llvm/CodeGen/MachineModuleInfo.h"
 
@@ -15,8 +16,7 @@ using namespace llvm;
 PreservedAnalyses
 FreeMachineFunctionPass::run(MachineFunction &MF,
                              MachineFunctionAnalysisManager &MFAM) {
-  auto &MMI = MF.getMMI();
-  MFAM.invalidate(MF, PreservedAnalyses::none());
-  MMI.deleteMachineFunctionFor(MF.getFunction()); // MF is dangling now.
-  return PreservedAnalyses::none();
+  PreservedAnalyses PA = PreservedAnalyses::none();
+  PA.abandon<FunctionToMachineFunctionAnalysis>();
+  return PA;
 }
diff --git a/llvm/lib/CodeGen/FunctionToMachineFunctionAnalysis.cpp b/llvm/lib/CodeGen/FunctionToMachineFunctionAnalysis.cpp
new file mode 100644
index 00000000000000..27f5e2761cfaac
--- /dev/null
+++ b/llvm/lib/CodeGen/FunctionToMachineFunctionAnalysis.cpp
@@ -0,0 +1,48 @@
+//===- FunctionToMachineFunctionAnalysis.cpp ------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file contains the definitions of the FunctionToMachineFunctionAnalysis
+// members.
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/CodeGen/FunctionToMachineFunctionAnalysis.h"
+#include "llvm/CodeGen/MachineFunction.h"
+#include "llvm/CodeGen/MachineModuleInfo.h"
+#include "llvm/Target/TargetMachine.h"
+#include <atomic>
+
+using namespace llvm;
+
+AnalysisKey FunctionToMachineFunctionAnalysis::Key;
+
+bool FunctionToMachineFunctionAnalysis::Result::invalidate(
+    Function &, const PreservedAnalyses &PA,
+    FunctionAnalysisManager::Invalidator &) {
+  // Unless it is invalidated explicitly, it should remain preserved.
+  auto PAC = PA.getChecker<FunctionToMachineFunctionAnalysis>();
+  return !PAC.preservedWhenStateless();
+}
+
+FunctionToMachineFunctionAnalysis::Result
+FunctionToMachineFunctionAnalysis::run(Function &F,
+                                       FunctionAnalysisManager &FAM) {
+  auto &Context = F.getContext();
+  const TargetSubtargetInfo &STI = *TM->getSubtargetImpl(F);
+  auto &MMI = FAM.getResult<ModuleAnalysisManagerFunctionProxy>(F)
+                  .getCachedResult<MachineModuleAnalysis>(*F.getParent())
+                  ->getMMI();
+  auto MF = std::make_unique<MachineFunction>(
+      F, *TM, STI, Context.generateMachineFunctionNum(F), MMI);
+  MF->initTargetMachineFunctionInfo(STI);
+
+  // MRI callback for target specific initializations.
+  TM->registerMachineRegisterInfoCallback(*MF);
+
+  return Result(std::move(MF));
+}
diff --git a/llvm/lib/CodeGen/MIRParser/MIRParser.cpp b/llvm/lib/CodeGen/MIRParser/MIRParser.cpp
index 4d9a8dc5602ba6..2f1de9c3da43d2 100644
--- a/llvm/lib/CodeGen/MIRParser/MIRParser.cpp
+++ b/llvm/lib/CodeGen/MIRParser/MIRParser.cpp
@@ -16,6 +16,7 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/AsmParser/Parser.h"
 #include "llvm/AsmParser/SlotMapping.h"
+#include "llvm/CodeGen/FunctionToMachineFunctionAnalysis.h"
 #include "llvm/CodeGen/MIRParser/MIParser.h"
 #include "llvm/CodeGen/MIRYamlMapping.h"
 #include "llvm/CodeGen/MachineConstantPool.h"
@@ -97,13 +98,15 @@ class MIRParserImpl {
   /// Create an empty function with the given name.
   Function *createDummyFunction(StringRef Name, Module &M);
 
-  bool parseMachineFunctions(Module &M, MachineModuleInfo &MMI);
+  bool parseMachineFunctions(Module &M, MachineModuleInfo &MMI,
+                             ModuleAnalysisManager *FAM = nullptr);
 
   /// Parse the machine function in the current YAML document.
   ///
   ///
   /// Return true if an error occurred.
-  bool parseMachineFunction(Module &M, MachineModuleInfo &MMI);
+  bool parseMachineFunction(Module &M, MachineModuleInfo &MMI,
+                            ModuleAnalysisManager *FAM);
 
   /// Initialize the machine function to the state that's described in the MIR
   /// file.
@@ -275,13 +278,14 @@ MIRParserImpl::parseIRModule(DataLayoutCallbackTy DataLayoutCallback) {
   return M;
 }
 
-bool MIRParserImpl::parseMachineFunctions(Module &M, MachineModuleInfo &MMI) {
+bool MIRParserImpl::parseMachineFunctions(Module &M, MachineModuleInfo &MMI,
+                                          ModuleAnalysisManager *MAM) {
   if (NoMIRDocuments)
     return false;
 
   // Parse the machine functions.
   do {
-    if (parseMachineFunction(M, MMI))
+    if (parseMachineFunction(M, MMI, MAM))
       return true;
     In.nextDocument();
   } while (In.setCurrentDocument());
@@ -303,7 +307,8 @@ Function *MIRParserImpl::createDummyFunction(StringRef Name, Module &M) {
   return F;
 }
 
-bool MIRParserImpl::parseMachineFunction(Module &M, MachineModuleInfo &MMI) {
+bool MIRParserImpl::parseMachineFunction(Module &M, MachineModuleInfo &MMI,
+                                         ModuleAnalysisManager *MAM) {
   // Parse the yaml.
   yaml::MachineFunction YamlMF;
   yaml::EmptyContext Ctx;
@@ -327,14 +332,29 @@ bool MIRParserImpl::parseMachineFunction(Module &M, MachineModuleInfo &MMI) {
                    "' isn't defined in the provided LLVM IR");
     }
   }
-  if (MMI.getMachineFunction(*F) != nullptr)
-    return error(Twine("redefinition of machine function '") + FunctionName +
-                 "'");
 
-  // Create the MachineFunction.
-  MachineFunction &MF = MMI.getOrCreateMachineFunction(*F);
-  if (initializeMachineFunction(YamlMF, MF))
-    return true;
+  if (!MAM) {
+    if (MMI.getMachineFunction(*F) != nullptr)
+      return error(Twine("redefinition of machine function '") + FunctionName +
+                   "'");
+
+    // Create the MachineFunction.
+    MachineFunction &MF = MMI.getOrCreateMachineFunction(*F);
+    if (initializeMachineFunction(YamlMF, MF))
+      return true;
+  } else {
+    auto &FAM =
+        MAM->getResult<FunctionAnalysisManagerModuleProxy>(M).getManager();
+    if (FAM.getCachedResult<FunctionToMachineFunctionAnalysis>(*F))
+      return error(Twine("redefinition of machine function '") + FunctionName +
+                   "'");
+
+    // Create the MachineFunction.
+    MachineFunction &MF =
+        FAM.getResult<FunctionToMachineFunctionAnalysis>(*F).getMF();
+    if (initializeMachineFunction(YamlMF, MF))
+      return true;
+  }
 
   return false;
 }
@@ -1101,6 +1121,11 @@ bool MIRParser::parseMachineFunctions(Module &M, MachineModuleInfo &MMI) {
   return Impl->parseMachineFunctions(M, MMI);
 }
 
+bool MIRParser::parseMachineFunctions(Module &M, ModuleAnalysisManager &MAM) {
+  auto &MMI = MAM.getResult<MachineModuleAnalysis>(M).getMMI();
+  return Impl->parseMachineFunctions(M, MMI, &MAM);
+}
+
 std::unique_ptr<MIRParser> llvm::createMIRParserFromFile(
     StringRef Filename, SMDiagnostic &Error, LLVMContext &Context,
     std::function<void(Function &)> ProcessIRFunction) {
diff --git a/llvm/lib/CodeGen/MachinePassManager.cpp b/llvm/lib/CodeGen/MachinePassManager.cpp
index 2763193b2c306b..65dee2388fbb64 100644
--- a/llvm/lib/CodeGen/MachinePassManager.cpp
+++ b/llvm/lib/CodeGen/MachinePassManager.cpp
@@ -11,6 +11,8 @@
 //===----------------------------------------------------------------------===//
 
 #include "llvm/CodeGen/MachinePassManager.h"
+#include "llvm/CodeGen/FreeMachineFunction.h"
+#include "llvm/CodeGen/FunctionToMachineFunctionAnalysis.h"
 #include "llvm/CodeGen/MachineFunction.h"
 #include "llvm/CodeGen/MachineModuleInfo.h"
 #include "llvm/IR/PassManagerImpl.h"
@@ -71,7 +73,9 @@ bool MachineFunctionAnalysisManagerModuleProxy::Result::invalidate(
 
 PreservedAnalyses
 ModuleToMachineFunctionPassAdaptor::run(Module &M, ModuleAnalysisManager &AM) {
-  auto &MMI = AM.getResult<MachineModuleAnalysis>(M).getMMI();
+  // Ensure we have a MachineModuleInfo
+  AM.getResult<MachineModuleAnalysis>(M).getMMI();
+  auto &FAM = AM.getResult<FunctionAnalysisManagerModuleProxy>(M).getManager();
   MachineFunctionAnalysisManager &MFAM =
       AM.getResult<MachineFunctionAnalysisManagerModuleProxy>(M).getManager();
   PassInstrumentation PI = AM.getResult<PassInstrumentationAnalysis>(M);
@@ -82,19 +86,21 @@ ModuleToMachineFunctionPassAdaptor::run(Module &M, ModuleAnalysisManager &AM) {
     if (F.isDeclaration() || F.hasAvailableExternallyLinkage())
       continue;
 
-    MachineFunction &MF = MMI.getOrCreateMachineFunction(F);
+    MachineFunction &MF =
+        FAM.getResult<FunctionToMachineFunctionAnalysis>(F).getMF();
 
     if (!PI.runBeforePass<MachineFunction>(*Pass, MF))
       continue;
     PreservedAnalyses PassPA = Pass->run(MF, MFAM);
-    if (MMI.getMachineFunction(F)) {
-      MFAM.invalidate(MF, PassPA);
+    MFAM.invalidate(MF, PassPA);
+    if (Pass->name() != FreeMachineFunctionPass::name()) {
       PI.runAfterPass(*Pass, MF, PassPA);
+      PA.intersect(std::move(PassPA));
     } else {
-      MFAM.clear(MF, F.getName());
-      PI.runAfterPassInvalidated<MachineFunction>(*Pass, PassPA);
+      PA.intersect(std::move(PassPA));
+      FAM.invalidate(F, PA);
+      PI.runAfterPassInvalidated<MachineFunction>(*Pass, PA);
     }
-    PA.intersect(std::move(PassPA));
   }
 
   return PA;
@@ -112,25 +118,24 @@ PreservedAnalyses
 PassManager<MachineFunction>::run(MachineFunction &MF,
                                   AnalysisManager<MachineFunction> &MFAM) {
   PassInstrumentation PI = MFAM.getResult<PassInstrumentationAnalysis>(MF);
-  Function &F = MF.getFunction();
-  MachineModuleInfo &MMI =
-      MFAM.getResult<ModuleAnalysisManagerMachineFunctionProxy>(MF)
-          .getCachedResult<MachineModuleAnalysis>(*F.getParent())
-          ->getMMI();
+  FunctionAnalysisManager &FAM =
+      MFAM.getResult<FunctionAnalysisManagerMachineFunctionProxy>(MF)
+          .getManager();
   PreservedAnalyses PA = PreservedAnalyses::all();
   for (auto &Pass : Passes) {
     if (!PI.runBeforePass<MachineFunction>(*Pass, MF))
       continue;
 
     PreservedAnalyses PassPA = Pass->run(MF, MFAM);
-    if (MMI.getMachineFunction(F)) {
-      MFAM.invalidate(MF, PassPA);
+    MFAM.invalidate(MF, PassPA);
+    if (Pass->name() != FreeMachineFunctionPass::name()) {
       PI.runAfterPass(*Pass, MF, PassPA);
+      PA.intersect(std::move(PassPA));
     } else {
-      MFAM.clear(MF, F.getName());
-      PI.runAfterPassInvalidated<MachineFunction>(*Pass, PassPA);
+      PA.intersect(std::move(PassPA));
+      FAM.invalidate(MF.getFunction(), PA);
+      PI.runAfterPassInvalidated<MachineFunction>(*Pass, PA);
     }
-    PA.intersect(std::move(PassPA));
   }
   return PA;
 }
diff --git a/llvm/lib/IR/LLVMContext.cpp b/llvm/lib/IR/LLVMContext.cpp
index 57077e786efc26..3e8cba5fa401a5 100644
--- a/llvm/lib/IR/LLVMContext.cpp
+++ b/llvm/lib/IR/LLVMContext.cpp
@@ -120,6 +120,12 @@ void LLVMContext::removeModule(Module *M) {
   pImpl->OwnedModules.erase(M);
 }
 
+unsigned LLVMContext::generateMachineFunctionNum(Function &F) {
+  Module *M = F.getParent();
+  assert(pImpl->OwnedModules.contains(M) && "Unexpected module!");
+  return pImpl->MachineFunctionNums[M]++;
+}
+
 //===----------------------------------------------------------------------===//
 // Recoverable Backend Errors
 //===----------------------------------------------------------------------===//
diff --git a/llvm/lib/IR/LLVMContextImpl.h b/llvm/lib/IR/LLVMContextImpl.h
index 7c67e191348eaf..2713015c266c7e 100644
--- a/llvm/lib/IR/LLVMContextImpl.h
+++ b/llvm/lib/IR/LLVMContextImpl.h
@@ -1450,6 +1450,10 @@ class LLVMContextImpl {
   /// will be automatically deleted if this context is deleted.
   SmallPtrSet<Module *, 4> OwnedModules;
 
+  /// MachineFunctionNums - Keep the next available unique number available for
+  /// a MachineFunction in given module. Module must in OwnedModules.
+  DenseMap<Module *, unsigned> MachineFunctionNums;
+
   /// The main remark streamer used by all the other streamers (e.g. IR, MIR,
   /// frontends, etc.). This should only be used by the specific streamers, and
   /// never directly.
diff --git a/llvm/lib/Passes/PassBuilder.cpp b/llvm/lib/Passes/PassBuilder.cpp
index 8d408ca2363a98..0ffe5e8d60edfb 100644
--- a/llvm/lib/Passes/PassBuilder.cpp
+++ b/llvm/lib/Passes/PassBuilder.cpp
@@ -83,6 +83,7 @@
 #include "llvm/CodeGen/ExpandLargeFpConvert.h"
 #include "llvm/CodeGen/ExpandMemCmp.h"
 #include "llvm/CodeGen/FreeMachineFunction.h"
+#include "llvm/CodeGen/FunctionToMachineFunctionAnalysis.h"
 #include "llvm/CodeGen/GCMetadata.h"
 #include "llvm/CodeGen/GlobalMerge.h"
 #include "llvm/CodeGen/HardwareLoops.h"
diff --git a/llvm/lib/Passes/PassRegistry.def b/llvm/lib/Passes/PassRegistry.def
index d15f58d7adfae4..98ef4b13f1e586 100644
--- a/llvm/lib/Passes/PassRegistry.def
+++ b/llvm/lib/Passes/PassRegistry.def
@@ -254,6 +254,9 @@ FUNCTION_ANALYSIS("demanded-bits", DemandedBitsAnalysis())
 FUNCTION_ANALYSIS("domfrontier", DominanceFrontierAnalysis())
 FUNCTION_ANALYSIS("domtree", DominatorTreeAnalysis())
 FUNCTION_ANALYSIS("func-properties", FunctionPropertiesAnalysis())
+FUNCTION_ANALYSIS("function-to-machine-function",
+             ...
[truncated]

aeubanks · 2024-04-16T20:00:02Z

llvm/include/llvm/CodeGen/FreeMachineFunction.h

@@ -13,6 +13,7 @@

 namespace llvm {

+// TODO: Convert it to function pass.


I don't think we need runAfterPassInvalidated, since MachineFunction is analysis that will just disappear. from the Function point of view, there is no IR invalidation happening, just analysis invalidation

aeubanks · 2024-04-16T22:15:08Z

llvm/include/llvm/IR/LLVMContext.h

@@ -327,12 +327,20 @@ class LLVMContext {
  // Module needs access to the add/removeModule methods.
  friend class Module;

+  // Each MachineFunction need a unique number.
+  // See FunctionNumber in MachineFunction.
+  friend class FunctionToMachineFunctionAnalysis;


is this necessary?

Move to public now.

aeubanks · 2024-04-16T22:17:14Z

llvm/lib/CodeGen/FunctionToMachineFunctionAnalysis.cpp

+    Function &, const PreservedAnalyses &PA,
+    FunctionAnalysisManager::Invalidator &) {
+  // Unless it is invalidated explicitly, it should remain preserved.
+  auto PAC = PA.getChecker<FunctionToMachineFunctionAnalysis>();


I'd actually leave this to be the default implementation. machine function passes should say that they preserve all non-machine function analyses anyway (getLoopPassPreservedAnalyses() is the equivalent for loop passes). Perhaps we should add some checks somewhere for that.

IR analyses are unconditional preserved in MachinePassModel.

see comment there

aeubanks · 2024-04-16T22:17:47Z

llvm/lib/CodeGen/FunctionToMachineFunctionAnalysis.cpp

+#include "llvm/CodeGen/MachineFunction.h"
+#include "llvm/CodeGen/MachineModuleInfo.h"
+#include "llvm/Target/TargetMachine.h"
+#include <atomic>


don't need <atomic> anymore

aeubanks · 2024-04-16T22:19:57Z

llvm/lib/CodeGen/MachinePassManager.cpp

@@ -71,7 +73,9 @@ bool MachineFunctionAnalysisManagerModuleProxy::Result::invalidate(

 PreservedAnalyses
 ModuleToMachineFunctionPassAdaptor::run(Module &M, ModuleAnalysisManager &AM) {
-  auto &MMI = AM.getResult<MachineModuleAnalysis>(M).getMMI();
+  // Ensure we have a MachineModuleInfo
+  AM.getResult<MachineModuleAnalysis>(M).getMMI();


is this temporary (if so add a TODO)? is it too much to remove MachineModuleAnalysis from the various APIs in this patch as well?

Found it is unnecessary now, there is always a MachineModuleInfo after parsing input file, move it to codegen pipeline.

aeubanks · 2024-04-16T22:28:39Z

llvm/lib/IR/LLVMContext.cpp

+unsigned LLVMContext::generateMachineFunctionNum(Function &F) {
+  Module *M = F.getParent();
+  assert(pImpl->OwnedModules.contains(M) && "Unexpected module!");
+  return pImpl->MachineFunctionNums[M]++;


do these need to start at 0 per module? otherwise could we just keep counting up ignoring the module?

otherwise we'll have to erase these entries in removeModule() or risk stale entries getting reused

It start at 0 per module in current implementation and it is related to symbol table, currently I'd prefer to keep the old behavior.

aeubanks · 2024-04-16T22:34:03Z

llvm/tools/llc/NewPMDriver.cpp

@@ -121,10 +121,10 @@ int llvm::compileModuleWithNewPM(
  registerCodeGenCallback(PIC, LLVMTM);

  LoopAnalysisManager LAM;
+  MachineFunctionAnalysisManager MFAM;


ultra nit, I'd put this before LoopAnalysisManager since MachineFunction is further from Function than Loop is from Function

paperchalice · 2024-04-16T23:59:40Z

Address comments.

aeubanks · 2024-04-22T17:31:28Z

llvm/include/llvm/CodeGen/FunctionToMachineFunctionAnalysis.h

+
+/// This analysis create MachineFunction for given Function.
+/// To release the MachineFunction, users should invalidate it explicitly.
+class FunctionToMachineFunctionAnalysis


I'd prefer the previous MachineFunctionAnalysis over this tbh, this is more verbose for no benefit, plus is still similar to FunctionToMachineFunctionAdaptor. I really can't think of a better name :(. I'd go back to the previous name and perhaps if we find something better in the future we can rename it.

aeubanks · 2024-04-22T17:34:04Z

llvm/include/llvm/CodeGen/MachinePassManager.h

@@ -76,6 +76,10 @@ struct MachinePassModel
 #endif

    auto PA = this->Pass.run(IR, AM);
+    // Machine function passes are not allowed to modify the LLVM
+    // representation, therefore we should preserve all IR analyses.
+    PA.template preserveSet<AllAnalysesOn<Module>>();


we shouldn't put this here because it's not required to go through a MachinePassModel to run a pass, you can always manually run a MachineFunctionPass.

as mentioned before, we should have a helper method similar to getLoopPassPreservedAnalyses() that every machine function pass must use as its return value

aeubanks · 2024-04-22T17:34:40Z

llvm/lib/CodeGen/FreeMachineFunction.cpp

-  MFAM.invalidate(MF, PreservedAnalyses::none());
-  MMI.deleteMachineFunctionFor(MF.getFunction()); // MF is dangling now.
-  return PreservedAnalyses::none();
+  PreservedAnalyses PA = PreservedAnalyses::none();


we should not be invalidating function analyses from a machine function pass. this should be a function pass. so the pipeline should be something like module(function(some-codegen-ir-passes,machine-function(isel,some-codegen-mir-passes),invalidate<machine-function>)). and if we have a machine pass in the middle of the codegen pipeline, module(function(some-codegen-ir-passes,machine-function(isel,some-codegen-mir-passes)),some-module-pass,function(machine-function(some-more-codegen-mir-passes),invalidate<machine-function>).

now that I think about it, we should just get rid of the module -> machine function adaptor and always go through a function -> machine function adaptor so that we can add an invalidate<machine-function> pass at the end if necessary. having two different adaptors that allow running machine function passes seems somewhat redundant anyway, and we definitely have to have the function -> machine function adaptor because we want to run function passes on a function, then mir passes on the same function before moving to the next function.

Yes, agreed we should just use function -> machine function adaptor. I also realized this when I refactor CodeGenPassBuilder. Will remove this pass firstly.

aeubanks · 2024-04-22T17:45:26Z

llvm/lib/CodeGen/FunctionToMachineFunctionAnalysis.cpp

+    Function &, const PreservedAnalyses &PA,
+    FunctionAnalysisManager::Invalidator &) {
+  // Unless it is invalidated explicitly, it should remain preserved.
+  auto PAC = PA.getChecker<FunctionToMachineFunctionAnalysis>();


see comment there

aeubanks · 2024-04-22T17:46:52Z

llvm/lib/CodeGen/MIRParser/MIRParser.cpp

+    if (initializeMachineFunction(YamlMF, MF))
+      return true;
+  } else {
+    auto &FAM =


can we directly pass FunctionAnalysisManager to parseMachineFunctions?

It needs MachineModuleInfo to initialize MachineFunction.

llvm-project/llvm/lib/CodeGen/MachineFunction.cpp

Lines 164 to 166 in ff153bd

MachineFunction::MachineFunction(Function &F, const LLVMTargetMachine &Target,

const TargetSubtargetInfo &STI,

unsigned FunctionNum, MachineModuleInfo &mmi)

hmm, ok for now. as @arsenm said, it'd be nice to move completely away from MMI, but there are a lot of uses right now. although spot checking a few, a lot of them are easily removed, e.g. getting MF.getMMI().getContext() -> MF.getContext()

could you file a bug for removing MMI?

paperchalice · 2024-04-23T04:38:58Z

Rename to MachineFunctionAnalysis
Remove FreeMachineFunctionPass
ModuleToMachineFunctionPassAdaptor -> FunctionToMachineFunctionPassAdaptor

paperchalice · 2024-04-23T04:44:54Z

llvm/include/llvm/Passes/CodeGenPassBuilder.h

@@ -538,7 +540,7 @@ Error CodeGenPassBuilder<Derived, TargetMachineT>::buildPipeline(
  if (PrintMIR)
    addPass(PrintMIRPass(Out), /*Force=*/true);

-  addPass(FreeMachineFunctionPass());
+  // TODO: invalidate MachineFunctionAnalysis


I'm refactoring this part, will show it in another PR.

paperchalice · 2024-04-23T04:52:41Z

llvm/include/llvm/CodeGen/MachinePassManager.h

-      // didn't even see an invalidate call when we got invalidated.
-      FAM->clear();
-    }
-


This causes double free.

do you know why? I know that the analysis managers have to be declared in a specific order due to these sorts of things. does this still occur even with the following order:

MachineFunctionAnalysisManager MFAM; LoopAnalysisManager LAM; FunctionAnalysisManager FAM; CGSCCAnalysisManager CGAM; ModuleAnalysisManager MAM;

?

I haven't thought too hard about whether or not this is actually required (meaning maybe we can't just remove this code for correctness)...

It would be the circular reference between MachineFunctionAnalysis and FunctionAnalysis in FunctionAnalysisManagerMachineFunctionProxy and MachineFunctionAnalysisManagerFunctionProxy.

Consider:

MachineFunctionAnalysisManager MFAM; // ... FunctionAnalysisManager FAM; // Fetch some analysis proxies FAM.clear(); MFAM.clear();

FAM.clear(); will clear MachineFunctionAnalysisManagerFunctionProxy -> MachineFunctionAnalysisManager -> FunctionAnalysisManagerMachineFunctionProxy -> FunctionAnalysisManager, i.e. clear FAM will try to clear itself again indirectly. This is also true for MachineFunctionAnalysisManager.

ah this code was copied from InnerAnalysisManagerProxy where an outer IR analysis manager can clear an inner IR analysis manager, which makes sense, but it doesn't make sense for a MachineFunctionAnalysisManager to clear a FunctionAnalysisManager. my bad for this code

aeubanks · 2024-04-24T16:07:40Z

llvm/tools/llc/NewPMDriver.cpp

-    MFPM.addPass(FreeMachineFunctionPass());
-    MPM.addPass(createModuleToMachineFunctionPassAdaptor(std::move(MFPM)));
+    FPM.addPass(createFunctionToMachineFunctionPassAdaptor(std::move(MFPM)));
+    FPM.addPass(InvalidateAnalysisPass<MachineFunctionAnalysis>());


this instance of invalidating machine functions isn't useful because we don't invalidate a machine function after running the pipeline on a machine function before moving onto the next machine function, but rather we invalidate all machine functions after running the pipeline on all machine functions. it needs to be in the function pass manager that also runs machine function passes to be useful

I don't think this is super important to have here since this is just for testing specific passes, I'd just drop it

aeubanks · 2024-04-24T16:13:20Z

llvm/unittests/MIR/PassBuilderCallbacksTest.cpp

why was this file deleted?

Now FreeMachineFunction is replaced by InvalidateAnalysisPass<MachineFunctionAnalysis>, there is no different between this test and unittests/IR/PassBuilderCallbacksTest.cpp.

would be good to put this sort of info in the commit description for reviewers

aeubanks · 2024-04-24T16:15:07Z

llvm/lib/CodeGen/DeadMachineInstructionElim.cpp

@@ -68,7 +68,7 @@ DeadMachineInstructionElimPass::run(MachineFunction &MF,
                                    MachineFunctionAnalysisManager &) {
  if (!DeadMachineInstructionElimImpl().runImpl(MF))
    return PreservedAnalyses::all();
-  PreservedAnalyses PA;
+  PreservedAnalyses PA = getMachineFunctionPassPreservedAnalyses();


we should have an assert in the function -> machine function adaptor that function analyses were not invalidated, but can do that in a followup patch

aeubanks · 2024-04-24T16:52:25Z

llvm/include/llvm/CodeGen/MachinePassManager.h

-      // didn't even see an invalidate call when we got invalidated.
-      FAM->clear();
-    }
-


do you know why? I know that the analysis managers have to be declared in a specific order due to these sorts of things. does this still occur even with the following order:

MachineFunctionAnalysisManager MFAM; LoopAnalysisManager LAM; FunctionAnalysisManager FAM; CGSCCAnalysisManager CGAM; ModuleAnalysisManager MAM;

?

I haven't thought too hard about whether or not this is actually required (meaning maybe we can't just remove this code for correctness)...

Now FreeMachineFunction is replaced by InvalidateAnalysisPass<MachineFunctionAnalysis>, the workaround in MachineFunctionPassManager is no longer needed, there is no difference between unittests/MIR/PassBuilderCallbacksTest.cpp and unittests/IR/PassBuilderCallbacksTest.cpp.

…put is mir

aeubanks

lgtm, thanks for this!

aeubanks · 2024-04-29T17:06:54Z

llvm/include/llvm/CodeGen/MachinePassManager.h

-      // didn't even see an invalidate call when we got invalidated.
-      FAM->clear();
-    }
-


ah this code was copied from InnerAnalysisManagerProxy where an outer IR analysis manager can clear an inner IR analysis manager, which makes sense, but it doesn't make sense for a MachineFunctionAnalysisManager to clear a FunctionAnalysisManager. my bad for this code

aeubanks · 2024-04-29T17:07:51Z

llvm/lib/CodeGen/MIRParser/MIRParser.cpp

+    if (initializeMachineFunction(YamlMF, MF))
+      return true;
+  } else {
+    auto &FAM =


could you file a bug for removing MMI?

paperchalice · 2024-04-30T01:52:13Z

Opened #90542.

paperchalice force-pushed the mf branch from 5c44ddc to 4584ad3 Compare April 13, 2024 10:02

paperchalice requested review from aeubanks and arsenm April 13, 2024 10:16

paperchalice force-pushed the mf branch from 4584ad3 to 13fc48f Compare April 14, 2024 08:17

arsenm approved these changes Apr 15, 2024

View reviewed changes

paperchalice mentioned this pull request Apr 15, 2024

[NewPM] Add FunctionToMachineFunctionPassAdaptor #88711

Closed

aeubanks reviewed Apr 15, 2024

View reviewed changes

paperchalice force-pushed the mf branch from 13fc48f to a763213 Compare April 16, 2024 04:00

paperchalice changed the title ~~[NewPM][CodeGen] Add MachineFunctionAnalysis~~ [NewPM][CodeGen] Add FunctionToMachineFunctionAnalysis Apr 16, 2024

paperchalice force-pushed the mf branch 3 times, most recently from 863ee18 to ed194c7 Compare April 16, 2024 12:57

llvmbot added the llvm:ir label Apr 16, 2024

paperchalice force-pushed the mf branch from ed194c7 to 80abec8 Compare April 16, 2024 13:09

aeubanks reviewed Apr 16, 2024

View reviewed changes

paperchalice force-pushed the mf branch 4 times, most recently from a5a1ba1 to 2a7a8ce Compare April 18, 2024 12:37

paperchalice requested a review from aeubanks April 19, 2024 12:57

aeubanks reviewed Apr 22, 2024

View reviewed changes

paperchalice force-pushed the mf branch from 2a7a8ce to 347c26b Compare April 23, 2024 04:36

paperchalice changed the title ~~[NewPM][CodeGen] Add FunctionToMachineFunctionAnalysis~~ [NewPM][CodeGen] Add MachineFunctionAnalysis Apr 23, 2024

paperchalice commented Apr 23, 2024

View reviewed changes

paperchalice force-pushed the mf branch from 347c26b to fabee4d Compare April 23, 2024 05:24

paperchalice force-pushed the mf branch 2 times, most recently from 15e28f2 to 09fc85c Compare April 23, 2024 06:02

paperchalice mentioned this pull request Apr 23, 2024

[NewPM][CodeGen] Refactoring CodeGenPassBuilder #89708

Open

paperchalice force-pushed the mf branch from 09fc85c to b77835b Compare April 24, 2024 13:10

aeubanks reviewed Apr 24, 2024

View reviewed changes

paperchalice force-pushed the mf branch 2 times, most recently from 45a9bf8 to 74c986c Compare April 29, 2024 02:12

paperchalice added 3 commits April 29, 2024 13:14

invalidate MachineFunctionAnalysis

9aed79c

remove InvalidateAnalysisPass<MachineFunctionAnalysis> in llc when in…

95c5981

…put is mir

paperchalice force-pushed the mf branch from 74c986c to 95c5981 Compare April 29, 2024 05:14

aeubanks approved these changes Apr 29, 2024

View reviewed changes

paperchalice merged commit 6ea0c0a into llvm:main Apr 30, 2024
3 of 4 checks passed

		@@ -13,6 +13,7 @@

		namespace llvm {

		// TODO: Convert it to function pass.

	MachineFunction::MachineFunction(Function &F, const LLVMTargetMachine &Target,
	const TargetSubtargetInfo &STI,
	unsigned FunctionNum, MachineModuleInfo &mmi)

[NewPM][CodeGen] Add MachineFunctionAnalysis #88610

[NewPM][CodeGen] Add MachineFunctionAnalysis #88610

Conversation

paperchalice commented Apr 13, 2024 • edited

arsenm left a comment

Choose a reason for hiding this comment

aeubanks left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paperchalice Apr 16, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paperchalice commented Apr 16, 2024 • edited

llvmbot commented Apr 16, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paperchalice Apr 16, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paperchalice Apr 16, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paperchalice commented Apr 16, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paperchalice Apr 23, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paperchalice Apr 23, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paperchalice commented Apr 23, 2024

paperchalice Apr 23, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aeubanks left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paperchalice commented Apr 30, 2024

[NewPM][CodeGen] Add `MachineFunctionAnalysis` #88610

[NewPM][CodeGen] Add `MachineFunctionAnalysis` #88610

paperchalice commented Apr 13, 2024 •

edited

paperchalice Apr 16, 2024 •

edited

paperchalice commented Apr 16, 2024 •

edited

paperchalice Apr 16, 2024 •

edited

paperchalice Apr 16, 2024 •

edited

paperchalice Apr 23, 2024 •

edited

paperchalice Apr 23, 2024 •

edited

paperchalice Apr 23, 2024 •

edited