[flang-commits] [clang] [flang] [llvm] [mlir] Make Ownership of MachineModuleInfo in Its Wrapper Pass External (PR #110443)

[Matin Raayai via flang-commits]

matinraayai wrote:

I dug up the commit that introduced `LLVMTargetMachine`: https://github.com/llvm/llvm-project/commit/12e97307a10bbac6bf9e6733833b84faf06dee88. It dates back to before when the MC layer was created. It seems the motivation was to allow a hypothetical target to generate code using whatever code generator it wants internally (be it the one provided by LLVM at the time or some other external library). The MC stuff was later on added on top of it around 2010, which I think made this abstraction a bit pointless, since MC and CodeGen integrate tightly together, and there's no point to have support for only one layer.

Given the issues faced when joining `TM` and `LLVMTM`, I think this abstraction should be respected. The key takeaway from this abstraction is that `TM` __must__ have a function that generates object files/MC; It's just that those interface functions should be void of any CodeGen related constructs (even the `MMIWP`). TLDR: We should follow this rule of thumb: __If it uses LLVM's CodeGen it belongs to the `LLVMTM` class, otherwise it belongs to `TM`__ (The same goes for all the `MachineFunctionInfo` stuff; They should be moved to `LLVMTM`).

This is a cause of concern for managing the lifetime of `MMI`: For `TM` interfaces, `MMI`'s lifetime should be managed by the `MMIWP` pass, otherwise it will get deleted when it goes out of the scope of the pass building function. For `LLVMTM` interface, however, `MMI` should be managed externally by the interface user. I think both should exist.

This also relates to a question that I had regarding the new PM codegen interface `buildCodeGenPipeline` @aeubanks: how exactly is the `MMI`'s lifetime managed after calling this function? If it's possible I want to talk more about it offline (You can find me on LLVM's Discord).

> I see that MMI really is a Codegen concept and not a Target concept, so that's why it takes an LLVMTargetMachine instead of TargetMachine. However, the `static_cast`s everywhere are extremely unfortunate. And it doesn't make sense to make the return type of `Target::createTargetMachine()` `LLVMTargetMachine` instead of `TargetMachine`. Perhaps alternatively we make the MMI constructor take `TargetMachine` and cast it inside the constructor to `LLVMTargetMachine`, so we only pay the cost of this weird distinction in one place rather than everywhere? wdyt?

To get back to your question @aeubanks  I don't think we should force any casts in the `MMI` constructor; Instead we should address the `TM`/`LLVMTM` abstraction issue. The casting will then take care of itself. Also there should be a `Target::createLLVMTargetMachine()` for those who want to explicitly manage `MMI`'s lifetime and want to use LLVm CodeGen.

https://github.com/llvm/llvm-project/pull/110443