[PATCH] D80249: CodeGen: Don't lazily construct MachineFunctionInfo

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added a comment.

In D80249#2158886 <https://reviews.llvm.org/D80249#2158886>, @nhaehnle wrote:

> In D80249#2156889 <https://reviews.llvm.org/D80249#2156889>, @arsenm wrote:
>
>> In D80249#2156869 <https://reviews.llvm.org/D80249#2156869>, @nhaehnle wrote:
>>
>>> Do you really need to change the signature of `MachineFunctionInfo::create`? That seems orthogonal to the problem you're trying to solve, or perhaps a redundant solution.
>>>
>>> The problem is: due to lazy construction and being able to refer to the MachineFunction, the state of the MachineFunctionInfo can have surprising accidental differences depending on when exactly it was created. There are two possible solutions:
>>>
>>> 1. Prevent the MachineFunctionInfo from accessing the MachineFunction.
>>> 2. Always create the MachineFunctionInfo in the same place.
>>>
>>> This change does both, but either solution should be sufficient, and I personally find #1 counter-intuitive: it seems to me that the additional target-specific info about an object should be allowed to access that object.
>>
>> You can't really do anything with the empty MachineFunction on construction. At the construction point, there isn't any information to access and it would always be a mistake to inspect it or make any construction decisions based on it (it would also be bad for the constructor to modify the MachineFunction).
>
> MFI is mutable, and so referring back to the MachineFunction as part of the mutation is potentially something reasonable to do. So the point isn't to use data from the MachineFunction during construction of the MFI, but rather to allow the "child" object to have a backlink to its "parent".

But you can just as well pass in the machine function in these cases. I don't think the minor convenience of not passing in the function is worth the footgun this gives you


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D80249/new/

https://reviews.llvm.org/D80249