Avoids using a plain unsigned for registers throughoug codegen.
Doesn't attempt to change every register use, just something a little
more than the set needed to build after changing the return type of
MachineOperand::getReg().
Details
Diff Detail
Event Timeline
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?
include/llvm/CodeGen/Register.h | ||
---|---|---|
28 | I am not sure I like those, since it still allows using Reg as plain unsigned. | |
57 | Ditto. |
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
include/llvm/CodeGen/Register.h | ||
---|---|---|
28 | I missed the parameter types here. Do you think trying to preserve the isStackSlot is important? | |
57 | 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 |
include/llvm/CodeGen/Register.h | ||
---|---|---|
28 | I guess this can be made private |
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
Sounds like a plan.
include/llvm/CodeGen/Register.h | ||
---|---|---|
23–25 | Any particular reason to have static methods that take an unsigned instead of just implementing these directly using this's Reg in isVirtual and friends? |
include/llvm/CodeGen/Register.h | ||
---|---|---|
23–25 | Not really, but this was easier to copy-paste from the existing functions |
Agreed. I like the direction this is going
include/llvm/CodeGen/Register.h | ||
---|---|---|
57 | I agree that operator unsigned() will need to be removed to complete the migration but I also see the point about limiting the size of the patch. I'm ok with it being removed in a follow-up step. We can get an idea of how big a job removing it is by marking it deprecated and seeing how many warnings that causes. |
Any particular reason to have static methods that take an unsigned instead of just implementing these directly using this's Reg in isVirtual and friends?