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

[Quentin Colombet via Phabricator via llvm-commits]

qcolombet requested changes to this revision.
qcolombet added a comment.
This revision now requires changes to proceed.

Hi,

I like that we take the iterator path, but I don't think the current patch is clearer than the previous implementation.

First we duplicate some logic from `AllocationOrder` to `AllocationOrder::Iterator` (see my inline comment), second, I found the `AllocationOrder::getIterator` not iterator friendly.
What I mean here is that I would have expected a `AllocationOrder::begin` and  `AllocationOrder::end`, so that we can use range based loop and so on.

As is I don't really see the value of that refactoring.

Cheers,
-Quentin



================
Comment at: llvm/lib/CodeGen/AllocationOrder.h:58
+        unsigned Reg = Order[Pos++];
+        if (!is_contained(Hints, Reg))
+          return Reg;
----------------
I don't like that we duplicate the logic of `AllocationOrder::isHint` here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88256