[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