[PATCH] D59444: [GlobalISel] Change MachineIRBuilder's SrcOp to contain subregister info

[Aditya Nandakumar via Phabricator via llvm-commits]

aditya_nandakumar added a comment.

In general I'm not too fond of building very target specific instructions involving subregs with the MachineIRBuilder (As these are only likely to be used in a few places in the selector). Specifically for this case of building a subreg, you can say

  Builder.buildInstr(COPY, {Dst},{})
    .addReg(Src, 0, SubReg);

which is not too much larger than (and doesn't require making every SrcOp bigger).

  Builder.buildInstr(COPY, {Dst}, {{Src, SubReg}});

While adding CSE support for COPYs with Subregs is great, I'm not sure how often it'll trigger and be useful (If it does occur then great). Also should we start adding support for building other kinds of target instructions - Adding immediate operands/target index nodes etc?
My approach for building non generic instructions have been with using the .addImm(..).addReg(...)... pattern - but if others strongly feel the need to add the ability to pass in all of the operands at once to the builder interfaces, and enabling CSE for them, then we can go ahead with this change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59444