[PATCH] D81461: GlobalISel: Fix double printing new instructions in legalizer

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added a comment.

In D81461#2082923 <https://reviews.llvm.org/D81461#2082923>, @arsenm wrote:

> In D81461#2082557 <https://reviews.llvm.org/D81461#2082557>, @aditya_nandakumar wrote:
>
> > In D81461#2082149 <https://reviews.llvm.org/D81461#2082149>, @arsenm wrote:
> >
> > > It seems both the MachineFunction and MachineIRBuilder have insertion observers; this seems redundant. I think the machine function observers should be removed?
> >
> >
> > MachineFunction observers are useful even if instructions are built without MIRBuilders (say using BuildMI) so CSE can be aware of it. Similar for deletions. To me, since the MF observer is always called, the one in MachineIRBuilder seems redundant.
>
>
> I don't think I would expect CSE or any magic when using BuildMI or generally touching the function. I think anything before selection should be going through MachineIRBuilder?


I also don't see how CSE would work with BuildMI, since you don't see the full set of operands at construction as CSE requires. Even with MachineIRBuilder, you only get CSE if you construct the instruction with all of its operands initially. Trying to handle this for target instructions also generally starts to break down with implicit operands etc.


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

https://reviews.llvm.org/D81461