[llvm] [CodeGen] Fix `MachineModuleInfo`'s move constructor to be more safe with `MCContext` ownership. (PR #104834)
weiwei chen via llvm-commits
llvm-commits at lists.llvm.org
Tue Aug 20 08:29:50 PDT 2024
weiweichen wrote:
> @weiweichen @arsenm though a working move constructor will solve the issue here, I think the best solution is to disallow move construction entirely, and treat MMI just like you would an `llvm::Module`.
>
> In other words, just like how an `llvm::Module` can be constructed only by passing a reference to the `llvm::LLVMContext` it uses:
>
> https://github.com/llvm/llvm-project/blob/52bfb2611f8d30fae3306c652af7ba5c7e88c147/llvm/include/llvm/IR/Module.h#L255
>
> An `llvm::MachineModuleInfo` should also be constructed only by passing a reference to the `llvm::MCContext` and the `llvm::LLVMTargetMachine` it will use:
>
> ```c++
> namespace llvm {
> class MachineModuleInfo {
> explicit MachineModuleInfo(LLVMTargetMachine &TM, MCContext &ExtContext);
> }
> ```
>
> Furthermore, `llvm::MachineModuleInfoWrapperPass` should also not manage the lifetime of its `llvm::MachineModuleInfo`, just like the new PM analysis provider of MMI here:
>
> https://github.com/llvm/llvm-project/blob/b443733cdee0c6179f384b41624b3dbcb0bbd8c9/llvm/include/llvm/CodeGen/MachineModuleInfo.h#L196-L224
>
> The only downside of this approach is that this change will probably touch all the LLVM backends, but IMO it makes more sense and should address the use cases we already have. But again, I'm happy either way as long as it works.
The main issue that's unclear to me for the proposed approach is that who will now be the owner of the `MCContext` if we change the API for `llvm::MachineModuleInfo` to just take a reference to `MCContext`.
https://github.com/llvm/llvm-project/pull/104834
More information about the llvm-commits
mailing list