[PATCH] D140964: [GlobalISel] Don't switch opcodes in MIRBuilder::buildInstr

Diana Picus via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 4 03:52:53 PST 2023


rovka added a comment.

In D140964#4025539 <https://reviews.llvm.org/D140964#4025539>, @foad wrote:

>> This patch simplifies the API of MachineIRBuilder so that the
>> buildInstr method does the least surprising thing (i.e. builds an instruction
>> with the specified opcode) and only the convenience buildX methods
>> (buildMerge etc) are allowed freedom over which opcode to use.
>
> I like this!
>
> Does this patch actually fix any missed CSE opportunities?

No, we also need to add G_BUILD_VECTOR to CSE <https://reviews.llvm.org/D140965>.

> I wonder if we could get rid of buildMerge altogether, in favour of separate buildMergeValues / buildConcatVectors / buildBuildVector functions.

We already have buildBuildVector and buildConcatVectors and they seem to have a decent number of uses. I haven't really audited all the uses of buildMerge to see if it would be onerous to replicate the checks for vectors everywhere :) I could have a look and send a follow-up patch if the existing uses aren't too sloppy.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140964



More information about the llvm-commits mailing list