[PATCH] D88256: [NFC][regalloc] Separate iteration from AllocationOrder

Mircea Trofin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Oct 5 14:42:16 PDT 2020


mtrofin added inline comments.


================
Comment at: llvm/lib/CodeGen/RegAllocGreedy.cpp:1860
   unsigned BestCand = NoCand;
-  Order.rewind();
-  while (unsigned PhysReg = Order.next()) {
+  for (auto PhysReg : Order) {
+    assert(PhysReg);
----------------
qcolombet wrote:
> mtrofin wrote:
> > qcolombet wrote:
> > > Could you call out the type here?
> > > Put differently, I don't like the use of `auto` when the type is not immediately obvious.
> > Done - also changed the type of operator*, in light of our discussion - we could type it as MCPhysReg, but I think MCRegister would also be correct, and help reduce the concept count - wdyt?
> I have a slight preference with MCPhysReg, but MCRegister is fine too.
> That said, it is strange to have `ArrayRef<MCPhysReg> getOrder` returning a different type.
> I would rather be consistent, but I can live with the iterator offering the richer type.
ack - fixed here and at line 2298. Left at 2617, which before was Register, and also in RegAllocBasic, same reason.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88256



More information about the llvm-commits mailing list