Skip to content

[NewPM][CodeGen] Add MachineFunctionAnalysis #88610

New issue

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

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

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 0 additions & 24 deletions llvm/include/llvm/CodeGen/FreeMachineFunction.h

This file was deleted.

12 changes: 12 additions & 0 deletions llvm/include/llvm/CodeGen/MIRParser/MIRParser.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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.
Expand Down
50 changes: 50 additions & 0 deletions llvm/include/llvm/CodeGen/MachineFunctionAnalysis.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
//===- llvm/CodeGen/MachineFunctionAnalysis.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 MachineFunctionAnalysis class.
//
//===----------------------------------------------------------------------===//

#ifndef LLVM_CODEGEN_MACHINEFUNCTIONANALYSIS
#define LLVM_CODEGEN_MACHINEFUNCTIONANALYSIS

#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 MachineFunctionAnalysis
Copy link
Contributor

Choose a reason for hiding this comment

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

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

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

: public AnalysisInfoMixin<MachineFunctionAnalysis> {
friend AnalysisInfoMixin<MachineFunctionAnalysis>;

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 &);
};

MachineFunctionAnalysis(const LLVMTargetMachine *TM) : TM(TM){};
Result run(Function &F, FunctionAnalysisManager &FAM);
};

} // namespace llvm

#endif // LLVM_CODEGEN_MachineFunctionAnalysis
4 changes: 4 additions & 0 deletions llvm/include/llvm/CodeGen/MachineModuleInfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
41 changes: 22 additions & 19 deletions llvm/include/llvm/CodeGen/MachinePassManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,15 @@ bool MachineFunctionAnalysisManagerModuleProxy::Result::invalidate(
ModuleAnalysisManager::Invalidator &Inv);
extern template class InnerAnalysisManagerProxy<MachineFunctionAnalysisManager,
Module>;
using MachineFunctionAnalysisManagerFunctionProxy =
InnerAnalysisManagerProxy<MachineFunctionAnalysisManager, Function>;

template <>
bool MachineFunctionAnalysisManagerFunctionProxy::Result::invalidate(
Function &F, const PreservedAnalyses &PA,
FunctionAnalysisManager::Invalidator &Inv);
extern template class InnerAnalysisManagerProxy<MachineFunctionAnalysisManager,
Function>;

extern template class OuterAnalysisManagerProxy<ModuleAnalysisManager,
MachineFunction>;
Expand All @@ -129,16 +138,6 @@ class FunctionAnalysisManagerMachineFunctionProxy
Arg.FAM = nullptr;
}

~Result() {
// FAM is cleared in a moved from state where there is nothing to do.
if (!FAM)
return;

// Clear out the analysis manager if we're being destroyed -- it means we
// didn't even see an invalidate call when we got invalidated.
FAM->clear();
}

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 causes double free.

Copy link
Contributor

Choose a reason for hiding this comment

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

do you 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)...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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

Result &operator=(Result &&RHS) {
FAM = RHS.FAM;
// We have to null out the analysis manager in the moved-from state
Expand Down Expand Up @@ -187,18 +186,18 @@ class FunctionAnalysisManagerMachineFunctionProxy
FunctionAnalysisManager *FAM;
};

class ModuleToMachineFunctionPassAdaptor
: public PassInfoMixin<ModuleToMachineFunctionPassAdaptor> {
class FunctionToMachineFunctionPassAdaptor
: public PassInfoMixin<FunctionToMachineFunctionPassAdaptor> {
public:
using PassConceptT =
detail::PassConcept<MachineFunction, MachineFunctionAnalysisManager>;

explicit ModuleToMachineFunctionPassAdaptor(
explicit FunctionToMachineFunctionPassAdaptor(
std::unique_ptr<PassConceptT> Pass)
: Pass(std::move(Pass)) {}

/// Runs the function pass across every function in the module.
PreservedAnalyses run(Module &M, ModuleAnalysisManager &AM);
/// Runs the function pass across every function in the function.
PreservedAnalyses run(Function &F, FunctionAnalysisManager &FAM);
void printPipeline(raw_ostream &OS,
function_ref<StringRef(StringRef)> MapClassName2PassName);

Expand All @@ -209,14 +208,14 @@ class ModuleToMachineFunctionPassAdaptor
};

template <typename MachineFunctionPassT>
ModuleToMachineFunctionPassAdaptor
createModuleToMachineFunctionPassAdaptor(MachineFunctionPassT &&Pass) {
FunctionToMachineFunctionPassAdaptor
createFunctionToMachineFunctionPassAdaptor(MachineFunctionPassT &&Pass) {
using PassModelT = detail::PassModel<MachineFunction, MachineFunctionPassT,
MachineFunctionAnalysisManager>;
// Do not use make_unique, it causes too many template instantiations,
// causing terrible compile times.
return ModuleToMachineFunctionPassAdaptor(
std::unique_ptr<ModuleToMachineFunctionPassAdaptor::PassConceptT>(
return FunctionToMachineFunctionPassAdaptor(
std::unique_ptr<FunctionToMachineFunctionPassAdaptor::PassConceptT>(
new PassModelT(std::forward<MachineFunctionPassT>(Pass))));
}

Expand Down Expand Up @@ -244,6 +243,10 @@ extern template class PassManager<MachineFunction>;
/// Convenience typedef for a pass manager over functions.
using MachineFunctionPassManager = PassManager<MachineFunction>;

/// Returns the minimum set of Analyses that all machine function passes must
/// preserve.
PreservedAnalyses getMachineFunctionPassPreservedAnalyses();

} // end namespace llvm

#endif // LLVM_CODEGEN_MACHINEPASSMANAGER_H
6 changes: 5 additions & 1 deletion llvm/include/llvm/IR/LLVMContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,10 @@ class LLVMContext {
void enableDebugTypeODRUniquing();
void disableDebugTypeODRUniquing();

/// generateMachineFunctionNum - Get a unique number for MachineFunction
/// that associated with the given Function.
unsigned generateMachineFunctionNum(Function &);

/// Defines the type of a yield callback.
/// \see LLVMContext::setYieldCallback.
using YieldCallbackTy = void (*)(LLVMContext *Context, void *OpaqueHandle);
Expand Down Expand Up @@ -332,7 +336,7 @@ class LLVMContext {
void addModule(Module*);

/// removeModule - Unregister a module from this context.
void removeModule(Module*);
void removeModule(Module *);
};

// Create wrappers for C Binding types (see CBindingWrapping.h).
Expand Down
18 changes: 12 additions & 6 deletions llvm/include/llvm/Passes/CodeGenPassBuilder.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
#include "llvm/CodeGen/DwarfEHPrepare.h"
#include "llvm/CodeGen/ExpandMemCmp.h"
#include "llvm/CodeGen/ExpandReductions.h"
#include "llvm/CodeGen/FreeMachineFunction.h"
#include "llvm/CodeGen/GCMetadata.h"
#include "llvm/CodeGen/GlobalMerge.h"
#include "llvm/CodeGen/IndirectBrExpand.h"
Expand All @@ -38,6 +37,8 @@
#include "llvm/CodeGen/JMCInstrumenter.h"
#include "llvm/CodeGen/LowerEmuTLS.h"
#include "llvm/CodeGen/MIRPrinter.h"
#include "llvm/CodeGen/MachineFunctionAnalysis.h"
#include "llvm/CodeGen/MachineModuleInfo.h"
#include "llvm/CodeGen/MachinePassManager.h"
#include "llvm/CodeGen/PreISelIntrinsicLowering.h"
#include "llvm/CodeGen/ReplaceWithVeclib.h"
Expand Down Expand Up @@ -199,8 +200,13 @@ template <typename DerivedT, typename TargetMachineT> class CodeGenPassBuilder {
AddMachinePass(ModulePassManager &MPM, const DerivedT &PB)
: MPM(MPM), PB(PB) {}
~AddMachinePass() {
if (!MFPM.isEmpty())
MPM.addPass(createModuleToMachineFunctionPassAdaptor(std::move(MFPM)));
if (!MFPM.isEmpty()) {
FunctionPassManager FPM;
FPM.addPass(
createFunctionToMachineFunctionPassAdaptor(std::move(MFPM)));
FPM.addPass(InvalidateAnalysisPass<MachineFunctionAnalysis>());
MPM.addPass(createModuleToFunctionPassAdaptor(std::move(FPM)));
}
}

template <typename PassT>
Expand All @@ -219,8 +225,8 @@ template <typename DerivedT, typename TargetMachineT> class CodeGenPassBuilder {
} else {
// Add Module Pass
if (!MFPM.isEmpty()) {
MPM.addPass(
createModuleToMachineFunctionPassAdaptor(std::move(MFPM)));
MPM.addPass(createModuleToFunctionPassAdaptor(
createFunctionToMachineFunctionPassAdaptor(std::move(MFPM))));
MFPM = MachineFunctionPassManager();
}

Expand Down Expand Up @@ -512,6 +518,7 @@ Error CodeGenPassBuilder<Derived, TargetMachineT>::buildPipeline(

{
AddIRPass addIRPass(MPM, derived());
addIRPass(RequireAnalysisPass<MachineModuleAnalysis, Module>());
addIRPass(RequireAnalysisPass<ProfileSummaryAnalysis, Module>());
addIRPass(RequireAnalysisPass<CollectorMetadataAnalysis, Module>());
addISelPasses(addIRPass);
Expand All @@ -538,7 +545,6 @@ Error CodeGenPassBuilder<Derived, TargetMachineT>::buildPipeline(
if (PrintMIR)
addPass(PrintMIRPass(Out), /*Force=*/true);

addPass(FreeMachineFunctionPass());
return verifyStartStop(*StartStopInfo);
}

Expand Down
1 change: 0 additions & 1 deletion llvm/include/llvm/Passes/MachinePassRegistry.def
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,6 @@ MACHINE_FUNCTION_ANALYSIS("pass-instrumentation", PassInstrumentationAnalysis(PI
#define MACHINE_FUNCTION_PASS(NAME, CREATE_PASS)
#endif
MACHINE_FUNCTION_PASS("dead-mi-elimination", DeadMachineInstructionElimPass())
// MACHINE_FUNCTION_PASS("free-machine-function", FreeMachineFunctionPass())
MACHINE_FUNCTION_PASS("no-op-machine-function", NoOpMachineFunctionPass())
MACHINE_FUNCTION_PASS("print", PrintMIRPass())
MACHINE_FUNCTION_PASS("require-all-machine-function-properties",
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/CodeGen/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,8 @@ add_llvm_component_library(LLVMCodeGen
FEntryInserter.cpp
FinalizeISel.cpp
FixupStatepointCallerSaved.cpp
FreeMachineFunction.cpp
FuncletLayout.cpp
MachineFunctionAnalysis.cpp
GCMetadata.cpp
GCMetadataPrinter.cpp
GCRootLowering.cpp
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/CodeGen/DeadMachineInstructionElim.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ DeadMachineInstructionElimPass::run(MachineFunction &MF,
MachineFunctionAnalysisManager &) {
if (!DeadMachineInstructionElimImpl().runImpl(MF))
return PreservedAnalyses::all();
PreservedAnalyses PA;
PreservedAnalyses PA = getMachineFunctionPassPreservedAnalyses();
Copy link
Contributor

Choose a reason for hiding this comment

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

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

PA.preserveSet<CFGAnalyses>();
return PA;
}
Expand Down
22 changes: 0 additions & 22 deletions llvm/lib/CodeGen/FreeMachineFunction.cpp

This file was deleted.

Loading
Loading