[PATCH] D63496: [WIP] CodeGen: Prototype class for registers

[Matt Arsenault via Phabricator via llvm-commits]

arsenm marked 2 inline comments as done.
arsenm added a comment.

In D63496#1548951 <https://reviews.llvm.org/D63496#1548951>, @qcolombet wrote:

> Thanks for doing this Matt, this is a long due clean-up.
>  I haven't looked carefully at all the changes, but the general direction LGTM.
>
> One question: right now, we already have `MCPhysReg` for physical registers (unfortunately it is not used consistently, but that's a different story), should we replace those as well with `Register` or be explicitly about what is expected in the related passes?
>
> I am torn because MCPhysReg gives more information on what is expected, but on the other hand it will be pretty easy to slip `Register` here and there over time and we would end up with an inconsistent code base again.
>  What do you think?


I'm not sure. I didn't encounter that many MCPhysRegs. Maybe adding an MCRegister that's nearly identical might be a future improvement? That way it could enforce MCRegister->Register is OK, but Register->MCRegister is not



================
Comment at: include/llvm/CodeGen/Register.h:27
+    //assert(!isStackSlot(Reg) && "Not a register! Check isStackSlot() first.");
+    return int(Reg) > 0;
+  }
----------------
qcolombet wrote:
> I am not sure I like those, since it still allows using Reg as plain unsigned.
I missed the parameter types here. Do you think trying to preserve the isStackSlot is important?


================
Comment at: include/llvm/CodeGen/Register.h:56
+    return Reg;
+  }
+
----------------
qcolombet wrote:
> Ditto.
I think this is necessary to make this a manageable sized patch. I was hoping to minimize the amount of code touched at once, so this can possibly be removed later


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

https://reviews.llvm.org/D63496