[PATCH] D153122: [MC] Use regunits instead of MCRegUnitIterator. NFC.
Jay Foad via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Jun 16 03:52:48 PDT 2023
foad added inline comments.
================
Comment at: llvm/lib/MC/MCRegisterInfo.cpp:138
+ auto RangeA = regunits(RegA);
+ MCRegUnitIterator IA = RangeA.begin(), EA = RangeA.end();
+ auto RangeB = regunits(RegB);
----------------
barannikov88 wrote:
> foad wrote:
> > This still mentions `MCRegUnitIterator` but it gets away from the non-standard `isValid` style of iteration, towards the standard `iterator_range` style.
> > This still mentions `MCRegUnitIterator`
> I think this is fine.
>
> (suggestion) The default constructor of MCRegUnitIterator creates an end iterator, it can be used to simplify code a bit:
> ```
> ...
> MCRegUnitIterator IA = regunits(RegA).begin();
> MCRegUnitIterator IB = regunits(RegB).begin();
> MCRegUnitIterator E;
> ...
> } while (*IA < *IB ? ++IA != E : ++IB != E);
> ```
>
That is certainly simpler, but less standard. I'm not sure which I prefer.
================
Comment at: llvm/lib/Target/SystemZ/SystemZInstrInfo.cpp:1023
+ auto CCUnits = TRI->regunits(MCRegister::from(SystemZ::CC));
+ assert(std::distance(CCUnits.begin(), CCUnits.end()) == 1 &&
+ "CC only has one reg unit.");
----------------
barannikov88 wrote:
> There is `range_size` just for this.
Thanks! But why don't we have `iterator_range::size` like Boost does?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D153122/new/
https://reviews.llvm.org/D153122
More information about the llvm-commits
mailing list