[PATCH] D91160: [NFC] Use [MC]Register for Hexagon target

[Gaurav Jain via Phabricator via llvm-commits]

gjain marked 6 inline comments as done.
gjain added a comment.

I'd definitely prefer not cleaning up RegisterRef as part of this patch. The register/sub tuple pattern is present throughout the code and needs an independant clean-up. Otherwise we risk this not being a NFC.

Regarding const usage, this will delete the implicit copy constructor. Given the earlier statement that those classes need an independant clean-up, are you okay punting on this?



================
Comment at: llvm/lib/Target/Hexagon/BitTracker.h:134
 
-  unsigned Reg;
+  Register Reg;
   uint16_t Pos;
----------------
mtrofin wrote:
> are Reg and Pos const?
See comments.


================
Comment at: llvm/lib/Target/Hexagon/HexagonBlockRanges.h:34
 
   struct RegisterRef {
+    llvm::Register Reg;
----------------
mtrofin wrote:
> weird... there was one of these earlier. Not sure if hoisting it in a common .h may make sense, but otherwise the comments are the same (const-ing Reg and Sub)
Yeah this was very odd and requires clean-up


================
Comment at: llvm/lib/Target/Hexagon/HexagonConstPropagation.cpp:86
   struct RegisterSubReg {
-    unsigned Reg, SubReg;
+    Register Reg;
+    unsigned SubReg;
----------------
mtrofin wrote:
> seems like a popular design
See comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91160