[PATCH] D150868: [CodeGen] Rename `MachineInstr::defs` to `MachineInstr::explicit_defs` (NFC)

Jay Foad via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri May 19 03:14:42 PDT 2023


foad added a comment.

In D150868#4355843 <https://reviews.llvm.org/D150868#4355843>, @barannikov88 wrote:

> In D150868#4355814 <https://reviews.llvm.org/D150868#4355814>, @foad wrote:
>
>> Sure, I can help, but I can't promise how much time I can spend on it.
>
> Thanks! There is no hurry.
>
>> I see this sort of pattern a few times:
>>
>>   for (auto &MO : I->uses()) {
>>     if (MO.isReg() && MO.isUse()) {
>>       MRI.clearKillFlags(MO.getReg());
>>     }
>>   }
>>
>> Here we could really use a proper `uses` (or maybe `all_uses`) iterator that does the filtering for us, so it returns all use operands and //only// the use operands.
>>
>> Do you think it would be worth renaming the current `uses` to something ugly like `uses_and_implicit_operands`, in the hope of migrating all users to something less ugly and then removing it?
>
> SGTM assuming we provide a replacement for cases where we only need uses (explicit + implicit).
> `uses()` would be a natural choise for this, but changing its behavior silently will cause problems to downstream users.
> Either some time should pass before we can re-introduce `uses()` or a different name should be picked (`all_uses()` sounds good to me as well).
>
> BTW should `uses()` be deprecated instead of removed in this patch?

If we implement `all_uses` (and `all_defs`?) then maybe there is no need to rename `uses` after all. In the future hopefully we can deprecate and then remove it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150868



More information about the llvm-commits mailing list