[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