[PATCH] D143753: [MachineInstr] Introduce TII buildCopy helper functions (NFC).

[Quentin Colombet via Phabricator via llvm-commits]

qcolombet added a comment.

Hey,

What's the long term plan for this helper?

I am not mad at the refactoring itself, but I'm wondering what kind of criteria we should use to decide when we add such helper.

For instance, should we do the same thing for `PHI`, `INSERT_SUBREG`, etc. and why or why not.
Also, should these helper be virtual or not?

I guess where I am going is what do you intend to simplify/achieve with them?

The bottom line is without a proper rationale on why we need this helper, I feel that it gets difficult to know which method should be used to construct a copy.
Right now we have `BuildMI` (and the underlying `MachineInstrBuilder` object that can be called directly), `MachineFunction::CreateMachineInstr`, `MachineIRBuilder::buildInstr` (BTW `MachineIRBuilder` has its own `MachineIRBuilder::buildCopy`) and we add a new method here.

I believe we need to disambiguate when this one should be used and why.

Admittedly I'm playing the devil's advocate here (I actually like the patch) but I think it is important to flush out the goals and intended usages to be able to better guide future users of this (and the other) API(s).

Cheers,
-Quentin


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143753