[PATCH] D65552: Move llvm::Register from CodeGen to MC to correct a layering issue

[Daniel Sanders via Phabricator via llvm-commits]

dsanders added a comment.

In D65552#1609520 <https://reviews.llvm.org/D65552#1609520>, @arsenm wrote:

> I had been planning on introducing a distinct "MCRegister" or "MCReg" for this purpose (to mostly replace MCPhysReg usage), with promotion from MCRegister->Register being allowed, but not the inverse.


I agree with having a class wrapping MCPhysReg but I think MCRegister needs to know about the namespacing, vregs, etc. as there's places in the MC layer API's where unsigned isn't limited to MCPhysReg. Once it knows about all that, I doubt a CodeGen-layer Register class would be different from MCRegister.

MCRegisterClass::contains() is the main example I have at the moment. It's often called with virtual register arguments and returns false for this case. You can check this by adding `assert(Reg == (Reg & 0xFFFF))` to this function and you'll see it fail on lots of tests as the vregs are passed in (I originally found it by changing unsigned to MCPhysReg in MCRegisterInfo's API's and found that two AMDGPU tests started failing). It happens because TargetRegisterClass::contains() forwards on to MCRegisterClass::contains().


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65552