[PATCH] D153122: [MC] Use regunits instead of MCRegUnitIterator. NFC.

[Sergei Barannikov via Phabricator via llvm-commits]

barannikov88 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);
----------------
foad wrote:
> 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.
[[ https://en.cppreference.com/w/cpp/iterator/istream_iterator/istream_iterator | std::istreambuf_iterator ]] behaves similarly.


================
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.");
----------------
foad wrote:
> barannikov88 wrote:
> > There is `range_size` just for this.
> Thanks! But why don't we have `iterator_range::size` like Boost does?
Probably no one took care of implementing it. There is only a handful uses of this function.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153122