[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