[PATCH] D141372: MachineIRBuilder: Rename buildMerge. NFC

Scott Linder via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 11 11:40:31 PST 2023


scott.linder added a comment.

In D141372#4044590 <https://reviews.llvm.org/D141372#4044590>, @aemerson wrote:

> In D141372#4042763 <https://reviews.llvm.org/D141372#4042763>, @rovka wrote:
>
>> In D141372#4042701 <https://reviews.llvm.org/D141372#4042701>, @foad wrote:
>>
>>> In D141372#4040553 <https://reviews.llvm.org/D141372#4040553>, @scott.linder wrote:
>>>
>>>> Tiny driveby bikeshed (feel free to ignore): `buildMergeLikeInstr`? It is longer, but `Op` seems to more commonly mean "Operand" here (save for some stuff like `buildAssertOp`).
>>>
>>> I'm not a huge fan of the "Op" part either, but there is a precedent for it in the GMergeLikeOp wrapper class.
>>
>> There could be an argument made that both GMergeLikeOp and buildAssertOp should use Instr instead of Op, too... Should I just rename all of them?
>
> As a separate commit yes that seems fine to me.

+1 to renaming in a pre-patch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141372



More information about the llvm-commits mailing list