Skip to content

Add A Version Of MachineModuleInfoWrapperPass That Does Not Own Its Underlying MachineModuleInfo #134554

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.

Sign up for GitHub

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

Already on GitHub? Sign in to your account

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

Add A Version Of MachineModuleInfoWrapperPass That Does Not Own Its Underlying MachineModuleInfo #134554

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
ed5c332
- Removed move constructor for MMI.
matinraayai Apr 5, 2025
1a29779
Changed TargetMachine and CodeGenTargetMachineImpl interfaces to use …
matinraayai Apr 5, 2025
8715ed6
Created owning and non-owning variants of MMIWP.
matinraayai Apr 6, 2025
832e088
Fixed remaining issues with MMI/MMIWP constructions.
matinraayai Apr 6, 2025
cdb9975
Revert back style change.
matinraayai Apr 6, 2025
42c5c73
clang format.
matinraayai Apr 6, 2025
8c213d5
Merge branch 'llvm:main' into mra/make-mmiwp-take-mmi-reference
matinraayai Apr 6, 2025
c71cae3
Fixed missing uses of OwningMachineModuleInfoWrapperPass.
matinraayai Apr 6, 2025
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
29 changes: 15 additions & 14 deletions llvm/include/llvm/CodeGen/CodeGenTargetMachineImpl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,22 +44,23 @@ class CodeGenTargetMachineImpl : public TargetMachine {
virtual TargetPassConfig *createPassConfig(PassManagerBase &PM) override;

/// Add passes to the specified pass manager to get the specified file
/// emitted. Typically this will involve several steps of code generation.
/// \p MMIWP is an optional parameter that, if set to non-nullptr,
/// will be used to set the MachineModuloInfo for this PM.
bool
addPassesToEmitFile(PassManagerBase &PM, raw_pwrite_stream &Out,
raw_pwrite_stream *DwoOut, CodeGenFileType FileType,
bool DisableVerify = true,
MachineModuleInfoWrapperPass *MMIWP = nullptr) override;
/// emitted. Typically, this will involve several steps of code generation.
/// This method should return true if emission of this file type is not
/// supported, or false on success.
/// \p MMI an optional, externally-managed \c MachineModuleInfo to
/// hold the generated machine code even after the \p PM is destroyed
bool addPassesToEmitFile(PassManagerBase &PM, raw_pwrite_stream &Out,
raw_pwrite_stream *DwoOut, CodeGenFileType FileType,
bool DisableVerify = true,
MachineModuleInfo *MMI = nullptr) override;

/// Add passes to the specified pass manager to get machine code emitted with
/// the MCJIT. This method returns true if machine code is not supported. It
/// fills the MCContext Ctx pointer which can be used to build custom
/// MCStreamer.
bool addPassesToEmitMC(PassManagerBase &PM, MCContext *&Ctx,
raw_pwrite_stream &Out,
bool DisableVerify = true) override;
/// the MCJIT. This method returns true if machine code is not supported.
/// \p MMI an optional, externally-managed \c MachineModuleInfo to
/// hold the generated machine code even after the \p PM is destroyed
bool addPassesToEmitMC(PassManagerBase &PM, raw_pwrite_stream &Out,
bool DisableVerify = true,
MachineModuleInfo *MMI = nullptr) override;

/// Adds an AsmPrinter pass to the pipeline that prints assembly or
/// machine code from the MI representation.
Expand Down
47 changes: 36 additions & 11 deletions llvm/include/llvm/CodeGen/MachineModuleInfo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,15 +106,11 @@ class MachineModuleInfo {
const Function *LastRequest = nullptr; ///< Used for shortcut/cache.
MachineFunction *LastResult = nullptr; ///< Used for shortcut/cache.

MachineModuleInfo &operator=(MachineModuleInfo &&MMII) = delete;

public:
explicit MachineModuleInfo(const TargetMachine *TM = nullptr);

explicit MachineModuleInfo(const TargetMachine *TM, MCContext *ExtContext);

MachineModuleInfo(MachineModuleInfo &&MMII);

~MachineModuleInfo();

void initialize();
Expand Down Expand Up @@ -167,22 +163,51 @@ class MachineModuleInfo {
/// \}
}; // End class MachineModuleInfo

/// \brief Interface for pass that provide access to \c MachineModuleInfo
/// being worked on
class MachineModuleInfoWrapperPass : public ImmutablePass {
MachineModuleInfo MMI;

public:
static char ID; // Pass identification, replacement for typeid
explicit MachineModuleInfoWrapperPass(const TargetMachine *TM = nullptr);

explicit MachineModuleInfoWrapperPass(const TargetMachine *TM,
MCContext *ExtContext);
MachineModuleInfoWrapperPass();

// Initialization and Finalization
bool doInitialization(Module &) override;
bool doFinalization(Module &) override;

MachineModuleInfo &getMMI() { return MMI; }
const MachineModuleInfo &getMMI() const { return MMI; }
virtual MachineModuleInfo &getMMI() = 0;
virtual const MachineModuleInfo &getMMI() const = 0;
};

/// \brief a version of \c MachineModuleInfoWrapperPass that manages the
/// lifetime of its \c MachineModuleInfo
class OwningMachineModuleInfoWrapperPass : public MachineModuleInfoWrapperPass {
MachineModuleInfo MMI;
Comment on lines +182 to +185
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should not have different versions of the pass. We should change wholly change the ownership mode for the pass to not own it.

(Also I've been thinking we should rename MachineModuleInfo. It's really more of a "MachineFunctionMap" or something)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Originally I intended to only have a single version, but the main issue I found was that creating a MachineModuleInfo requires any client of TargetMachine::addPassesToEmitFile and TargetMachine::addPassesToEmitMC to be linked against CodeGen, which it might not be something they want to do. This only applies to targets that implement codegen, however. All other out-of-tree non-codegen targets are not affected.

I will change to my originally intended design if you think it's acceptable to force all TargetMachine clients to link against CodeGen.

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess I don't understand what the use case is for this specific pass if you aren't using codegen. MachineModuleInfo, and by extension MachineModuleInfoWrapperPass are a fundamental part of codegen (it is after all just a map of MachineFunction). I would expect for only the owning version to exist, and it to not change the linking requirements of addPassesToEmitFile users

Copy link
Contributor

Choose a reason for hiding this comment

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

One without MachineModuleInfo is for something else; Owning probably isn't the right name (but neither is MachineModuleInfo)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can combine the two passes together into a single MachineModuleInfoWrapperPass that looks like the following:

class MachineModuleInfoWrapperPass : public ImmutablePass {
  MachineModuleInfo *MMI{nullptr};
  bool OwnsMMI{true};
  
  public:
  explicit MachineModuleInfoWrapperPass(const TargetMachine &TM)
      : MMI(new MachineModuleInfo(&TM)), OwnsMMI(true) {};

  MachineModuleInfoWrapperPass(const TargetMachine &TM,
                                     MCContext &ExtContext)
      : MMI(new MachineModuleInfo(&TM, &ExtContext)), OwnsMMI(true) {};
  
  explicit MachineModuleInfoWrapperPass(MachineModuleInfo &MMI) : MMI(&MMI), OwnsMMI(false) {};
  
  MachineModuleInfo &getMMI() override { return MMI; }
  const MachineModuleInfo &getMMI() const override { return MMI; }
};


bool MachineModuleInfoWrapperPass::doFinalization(Module &M) {
  getMMI().finalize();
  if (OwnsMMI)
    delete MMI;
  return false;
};

This way, there is only a single version of the pass, and it will continue to behave the same way as it did before, while supporting the new functionality.
I'm curious, how does addPassestoEmitFiles for the new PM handles the issue of not owning the MMI reference? Does it force its users to link against CodeGen?


public:
explicit OwningMachineModuleInfoWrapperPass(const TargetMachine &TM)
: MMI(&TM) {};

OwningMachineModuleInfoWrapperPass(const TargetMachine &TM,
MCContext &ExtContext)
: MMI(&TM, &ExtContext) {};

MachineModuleInfo &getMMI() override { return MMI; }
const MachineModuleInfo &getMMI() const override { return MMI; }
};

/// \brief a version of \c MachineModuleInfoWrapperPass that does not manage the
/// lifetime of its \c MachineModuleInfo
class NonOwningMachineModuleInfoWrapperPass
: public MachineModuleInfoWrapperPass {
MachineModuleInfo &MMI;

public:
explicit NonOwningMachineModuleInfoWrapperPass(MachineModuleInfo &MMI)
: MMI(MMI) {};

MachineModuleInfo &getMMI() override { return MMI; }
const MachineModuleInfo &getMMI() const override { return MMI; }
};

/// An analysis that produces \c MachineModuleInfo for a module.
Expand Down
34 changes: 18 additions & 16 deletions llvm/include/llvm/Target/TargetMachine.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ using ModulePassManager = PassManager<Module>;

class Function;
class GlobalValue;
class MachineModuleInfoWrapperPass;
class MachineModuleInfo;
struct MachineSchedContext;
class Mangler;
class MCAsmInfo;
Expand Down Expand Up @@ -401,27 +401,29 @@ class TargetMachine {
virtual void registerDefaultAliasAnalyses(AAManager &) {}

/// Add passes to the specified pass manager to get the specified file
/// emitted. Typically this will involve several steps of code generation.
/// emitted. Typically, this will involve several steps of code generation.
/// This method should return true if emission of this file type is not
/// supported, or false on success.
/// \p MMIWP is an optional parameter that, if set to non-nullptr,
/// will be used to set the MachineModuloInfo for this PM.
virtual bool
addPassesToEmitFile(PassManagerBase &, raw_pwrite_stream &,
raw_pwrite_stream *, CodeGenFileType,
bool /*DisableVerify*/ = true,
MachineModuleInfoWrapperPass *MMIWP = nullptr) {
/// For targets that utilize the target-independent code generator
/// (CodeGen), an externally-managed \c MachineModuleInfo can be provided
/// to ensure persistence of the generated machine code even after the pass
/// manager is destroyed
virtual bool addPassesToEmitFile(PassManagerBase &, raw_pwrite_stream &,
raw_pwrite_stream *, CodeGenFileType,
bool /*DisableVerify*/ = true,
MachineModuleInfo * = nullptr) {
return true;
}

/// Add passes to the specified pass manager to get machine code emitted with
/// the MCJIT. This method returns true if machine code is not supported. It
/// fills the MCContext Ctx pointer which can be used to build custom
/// MCStreamer.
///
virtual bool addPassesToEmitMC(PassManagerBase &, MCContext *&,
raw_pwrite_stream &,
bool /*DisableVerify*/ = true) {
/// the MCJIT. This method returns true if machine code is not supported.
/// For targets that utilize the target-independent code generator
/// (CodeGen), an externally-managed \c MachineModuleInfo can be provided
/// to ensure persistence of the generated machine code even after the pass
/// manager is destroyed
virtual bool addPassesToEmitMC(PassManagerBase &, raw_pwrite_stream &,
bool /*DisableVerify*/ = true,
MachineModuleInfo * = nullptr) {
return true;
}

Expand Down
50 changes: 32 additions & 18 deletions llvm/lib/CodeGen/CodeGenTargetMachineImpl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,18 +210,27 @@ CodeGenTargetMachineImpl::createMCStreamer(raw_pwrite_stream &Out,

bool CodeGenTargetMachineImpl::addPassesToEmitFile(
PassManagerBase &PM, raw_pwrite_stream &Out, raw_pwrite_stream *DwoOut,
CodeGenFileType FileType, bool DisableVerify,
MachineModuleInfoWrapperPass *MMIWP) {
// Add common CodeGen passes.
if (!MMIWP)
MMIWP = new MachineModuleInfoWrapperPass(this);
CodeGenFileType FileType, bool DisableVerify, MachineModuleInfo *MMI) {
// Add the wrapper pass for MMI
MachineModuleInfoWrapperPass *MMIWP;
if (MMI) {
MMIWP = new NonOwningMachineModuleInfoWrapperPass(*MMI);
} else {
MMIWP = new OwningMachineModuleInfoWrapperPass(*this);
MMI = &MMIWP->getMMI();
}
// Check (for the case of externally-managed MMI) if the TM of MMI is
// the same as this target machine
if (&MMI->getTarget() != this)
return true;

TargetPassConfig *PassConfig =
addPassesToGenerateCode(*this, PM, DisableVerify, *MMIWP);
if (!PassConfig)
return true;

if (TargetPassConfig::willCompleteCodeGenPipeline()) {
if (addAsmPrinter(PM, Out, DwoOut, FileType, MMIWP->getMMI().getContext()))
if (addAsmPrinter(PM, Out, DwoOut, FileType, MMI->getContext()))
return true;
} else {
// MIR printing is redundant with -filetype=null.
Expand All @@ -233,25 +242,30 @@ bool CodeGenTargetMachineImpl::addPassesToEmitFile(
return false;
}

/// addPassesToEmitMC - Add passes to the specified pass manager to get
/// machine code emitted with the MCJIT. This method returns true if machine
/// code is not supported. It fills the MCContext Ctx pointer which can be
/// used to build custom MCStreamer.
///
bool CodeGenTargetMachineImpl::addPassesToEmitMC(PassManagerBase &PM,
MCContext *&Ctx,
raw_pwrite_stream &Out,
bool DisableVerify) {
// Add common CodeGen passes.
MachineModuleInfoWrapperPass *MMIWP = new MachineModuleInfoWrapperPass(this);
bool DisableVerify,
MachineModuleInfo *MMI) {
// Add the wrapper pass for MMI
MachineModuleInfoWrapperPass *MMIWP;
if (MMI) {
MMIWP = new NonOwningMachineModuleInfoWrapperPass(*MMI);
} else {
MMIWP = new OwningMachineModuleInfoWrapperPass(*this);
MMI = &MMIWP->getMMI();
}
// Check (for the case of externally-managed MMI) if the TM of MMI is
// the same as this target machine
if (&MMI->getTarget() != this)
return true;
TargetPassConfig *PassConfig =
addPassesToGenerateCode(*this, PM, DisableVerify, *MMIWP);
if (!PassConfig)
return true;
assert(TargetPassConfig::willCompleteCodeGenPipeline() &&
"Cannot emit MC with limited codegen pipeline");

Ctx = &MMIWP->getMMI().getContext();
MCContext &Ctx = MMI->getContext();
// libunwind is unable to load compact unwind dynamically, so we must generate
// DWARF unwind info for the JIT.
Options.MCOptions.EmitDwarfUnwind = EmitDwarfUnwindType::Always;
Expand All @@ -261,7 +275,7 @@ bool CodeGenTargetMachineImpl::addPassesToEmitMC(PassManagerBase &PM,
const MCSubtargetInfo &STI = *getMCSubtargetInfo();
const MCRegisterInfo &MRI = *getMCRegisterInfo();
std::unique_ptr<MCCodeEmitter> MCE(
getTarget().createMCCodeEmitter(*getMCInstrInfo(), *Ctx));
getTarget().createMCCodeEmitter(*getMCInstrInfo(), Ctx));
if (!MCE)
return true;
MCAsmBackend *MAB =
Expand All @@ -271,7 +285,7 @@ bool CodeGenTargetMachineImpl::addPassesToEmitMC(PassManagerBase &PM,

const Triple &T = getTargetTriple();
std::unique_ptr<MCStreamer> AsmStreamer(getTarget().createMCObjectStreamer(
T, *Ctx, std::unique_ptr<MCAsmBackend>(MAB), MAB->createObjectWriter(Out),
T, Ctx, std::unique_ptr<MCAsmBackend>(MAB), MAB->createObjectWriter(Out),
std::move(MCE), STI));

// Create the AsmPrinter, which takes ownership of AsmStreamer if successful.
Expand Down
25 changes: 4 additions & 21 deletions llvm/lib/CodeGen/MachineModuleInfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,17 +37,6 @@ void MachineModuleInfo::finalize() {
ObjFileMMI = nullptr;
}

MachineModuleInfo::MachineModuleInfo(MachineModuleInfo &&MMI)
: TM(std::move(MMI.TM)),
Context(TM.getTargetTriple(), TM.getMCAsmInfo(), TM.getMCRegisterInfo(),
TM.getMCSubtargetInfo(), nullptr, &TM.Options.MCOptions, false),
MachineFunctions(std::move(MMI.MachineFunctions)) {
Context.setObjectFileInfo(TM.getObjFileLowering());
ObjFileMMI = MMI.ObjFileMMI;
ExternalContext = MMI.ExternalContext;
TheModule = MMI.TheModule;
}

MachineModuleInfo::MachineModuleInfo(const TargetMachine *TM)
: TM(*TM), Context(TM->getTargetTriple(), TM->getMCAsmInfo(),
TM->getMCRegisterInfo(), TM->getMCSubtargetInfo(),
Expand Down Expand Up @@ -150,15 +139,8 @@ FunctionPass *llvm::createFreeMachineFunctionPass() {
return new FreeMachineFunction();
}

MachineModuleInfoWrapperPass::MachineModuleInfoWrapperPass(
const TargetMachine *TM)
: ImmutablePass(ID), MMI(TM) {
initializeMachineModuleInfoWrapperPassPass(*PassRegistry::getPassRegistry());
}

MachineModuleInfoWrapperPass::MachineModuleInfoWrapperPass(
const TargetMachine *TM, MCContext *ExtContext)
: ImmutablePass(ID), MMI(TM, ExtContext) {
MachineModuleInfoWrapperPass::MachineModuleInfoWrapperPass()
: ImmutablePass(ID) {
initializeMachineModuleInfoWrapperPassPass(*PassRegistry::getPassRegistry());
}

Expand Down Expand Up @@ -193,6 +175,7 @@ static uint64_t getLocCookie(const SMDiagnostic &SMD, const SourceMgr &SrcMgr,
}

bool MachineModuleInfoWrapperPass::doInitialization(Module &M) {
MachineModuleInfo &MMI = getMMI();
MMI.initialize();
MMI.TheModule = &M;
LLVMContext &Ctx = M.getContext();
Expand All @@ -210,7 +193,7 @@ bool MachineModuleInfoWrapperPass::doInitialization(Module &M) {
}

bool MachineModuleInfoWrapperPass::doFinalization(Module &M) {
MMI.finalize();
getMMI().finalize();
return false;
}

Expand Down
7 changes: 3 additions & 4 deletions llvm/lib/ExecutionEngine/MCJIT/MCJIT.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,8 @@ MCJIT::MCJIT(std::unique_ptr<Module> M, std::unique_ptr<TargetMachine> TM,
std::shared_ptr<MCJITMemoryManager> MemMgr,
std::shared_ptr<LegacyJITSymbolResolver> Resolver)
: ExecutionEngine(TM->createDataLayout(), std::move(M)), TM(std::move(TM)),
Ctx(nullptr), MemMgr(std::move(MemMgr)),
Resolver(*this, std::move(Resolver)), Dyld(*this->MemMgr, this->Resolver),
ObjCache(nullptr) {
MemMgr(std::move(MemMgr)), Resolver(*this, std::move(Resolver)),
Dyld(*this->MemMgr, this->Resolver), ObjCache(nullptr) {
// FIXME: We are managing our modules, so we do not want the base class
// ExecutionEngine to manage them as well. To avoid double destruction
// of the first (and only) module added in ExecutionEngine constructor
Expand Down Expand Up @@ -163,7 +162,7 @@ std::unique_ptr<MemoryBuffer> MCJIT::emitObject(Module *M) {

// Turn the machine code intermediate representation into bytes in memory
// that may be executed.
if (TM->addPassesToEmitMC(PM, Ctx, ObjStream, !getVerifyModules()))
if (TM->addPassesToEmitMC(PM, ObjStream, !getVerifyModules()))
report_fatal_error("Target does not support MC emission!");

// Initialize passes.
Expand Down
1 change: 0 additions & 1 deletion llvm/lib/ExecutionEngine/MCJIT/MCJIT.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,6 @@ class MCJIT : public ExecutionEngine {
};

std::unique_ptr<TargetMachine> TM;
MCContext *Ctx;
std::shared_ptr<MCJITMemoryManager> MemMgr;
LinkingSymbolResolver Resolver;
RuntimeDyld Dyld;
Expand Down
3 changes: 1 addition & 2 deletions llvm/lib/ExecutionEngine/Orc/CompileUtils.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,7 @@ Expected<SimpleCompiler::CompileResult> SimpleCompiler::operator()(Module &M) {
raw_svector_ostream ObjStream(ObjBufferSV);

legacy::PassManager PM;
MCContext *Ctx;
if (TM.addPassesToEmitMC(PM, Ctx, ObjStream))
if (TM.addPassesToEmitMC(PM, ObjStream))
return make_error<StringError>("Target does not support MC emission",
inconvertibleErrorCode());
PM.run(M);
Expand Down
20 changes: 14 additions & 6 deletions llvm/lib/Target/DirectX/DirectXTargetMachine.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,7 @@ void DirectXTargetMachine::registerPassBuilderCallbacks(PassBuilder &PB) {

bool DirectXTargetMachine::addPassesToEmitFile(
PassManagerBase &PM, raw_pwrite_stream &Out, raw_pwrite_stream *DwoOut,
CodeGenFileType FileType, bool DisableVerify,
MachineModuleInfoWrapperPass *MMIWP) {
CodeGenFileType FileType, bool DisableVerify, MachineModuleInfo *MMI) {
TargetPassConfig *PassConfig = createPassConfig(PM);
PassConfig->addCodeGenPrepare();

Expand All @@ -150,8 +149,17 @@ bool DirectXTargetMachine::addPassesToEmitFile(
// globals don't pollute the DXIL.
PM.add(createDXContainerGlobalsPass());

if (!MMIWP)
MMIWP = new MachineModuleInfoWrapperPass(this);
MachineModuleInfoWrapperPass *MMIWP;
if (MMI) {
MMIWP = new NonOwningMachineModuleInfoWrapperPass(*MMI);
} else {
MMIWP = new OwningMachineModuleInfoWrapperPass(*this);
MMI = &MMIWP->getMMI();
}
// Check (for the case of externally-managed MMI) if the TM of MMI is
// the same as this target machine
if (&MMI->getTarget() != this)
return true;
PM.add(MMIWP);
if (addAsmPrinter(PM, Out, DwoOut, FileType,
MMIWP->getMMI().getContext()))
Expand All @@ -166,9 +174,9 @@ bool DirectXTargetMachine::addPassesToEmitFile(
}

bool DirectXTargetMachine::addPassesToEmitMC(PassManagerBase &PM,
MCContext *&Ctx,
raw_pwrite_stream &Out,
bool DisableVerify) {
bool DisableVerify,
MachineModuleInfo *MMI) {
return true;
}

Expand Down
Loading