[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
Mon Aug 19 19:00:14 PDT 2024


weiweichen wrote:

> > This function is not used by anyone in the LLVM monorepo AFAIK. I have a related PR (#98770) to this, which exposes the move constructor on the MMIWP side, so that I can inspect its contents after I run passes on it without requiring the PM to stay alive.
> > However, as discussions in the PR shows, when I used it in the use case I showed [here](https://github.com/llvm/llvm-project/pull/98770#issue-2407150383), I would get segfaults. I suspected this was because MFs had references to the old, unmoved MMI fields deleted by the PM destructor, which @arsenm has fixed since then. I have not tested the MMIWP constructor since then due to time constraints.
> > TLDR; I think there should be a test for this constructor in LLVM proper (something along the lines of my example use case with the PM explicitly deleted should be enough), to prove that it is working correctly in the first place.
> 
> Thank you for context and suggestion on adding a test! I agree that a test is good to have, though do you have any suggestions where should it be added to? [Your PR](https://github.com/llvm/llvm-project/pull/98770) doesn't seem to have a real one doing the testing in the code but only code snippet in the comment, and I can't find any in `llvm/test` at all that's remotely testing this sort of thing?

Actually, nvm, found something in `llvm/unittests/CodeGen`. Will push a commit to test the change in this PR. 

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


More information about the llvm-commits mailing list