[PATCH] D67423: [RISCV] Consistently use Register arithmetic

[James Clarke via Phabricator via llvm-commits]

jrtc27 added a comment.

Hm, I don't particularly like this (or at least the F parts of it); the tables are clear, whereas this seems very brittle.. e.g. when you add Q you have to also modify the places doing arithmetic on F and D registers.

It's worth noting that other targets (Mips and Sparc) call their floating point registers F0-F31 and D0-D31 (and there's also Q0-Q31 for Sparc) even though they're always referred to as `%fX` in the assembly. That would be an alternative resolution to the sorting issue, that also feels less hacky/surprising compared with the numerical-vs-alphabetical ordering subtleties.



================
Comment at: llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp:745
 static Register convertFPR32ToFPR64(Register Reg) {
-  switch (Reg) {
-  default:
-    llvm_unreachable("Not a recognised FPR32 register");
-  case RISCV::F0_32: return RISCV::F0_64;
-  case RISCV::F1_32: return RISCV::F1_64;
-  case RISCV::F2_32: return RISCV::F2_64;
-  case RISCV::F3_32: return RISCV::F3_64;
-  case RISCV::F4_32: return RISCV::F4_64;
-  case RISCV::F5_32: return RISCV::F5_64;
-  case RISCV::F6_32: return RISCV::F6_64;
-  case RISCV::F7_32: return RISCV::F7_64;
-  case RISCV::F8_32: return RISCV::F8_64;
-  case RISCV::F9_32: return RISCV::F9_64;
-  case RISCV::F10_32: return RISCV::F10_64;
-  case RISCV::F11_32: return RISCV::F11_64;
-  case RISCV::F12_32: return RISCV::F12_64;
-  case RISCV::F13_32: return RISCV::F13_64;
-  case RISCV::F14_32: return RISCV::F14_64;
-  case RISCV::F15_32: return RISCV::F15_64;
-  case RISCV::F16_32: return RISCV::F16_64;
-  case RISCV::F17_32: return RISCV::F17_64;
-  case RISCV::F18_32: return RISCV::F18_64;
-  case RISCV::F19_32: return RISCV::F19_64;
-  case RISCV::F20_32: return RISCV::F20_64;
-  case RISCV::F21_32: return RISCV::F21_64;
-  case RISCV::F22_32: return RISCV::F22_64;
-  case RISCV::F23_32: return RISCV::F23_64;
-  case RISCV::F24_32: return RISCV::F24_64;
-  case RISCV::F25_32: return RISCV::F25_64;
-  case RISCV::F26_32: return RISCV::F26_64;
-  case RISCV::F27_32: return RISCV::F27_64;
-  case RISCV::F28_32: return RISCV::F28_64;
-  case RISCV::F29_32: return RISCV::F29_64;
-  case RISCV::F30_32: return RISCV::F30_64;
-  case RISCV::F31_32: return RISCV::F31_64;
-  }
+  return Reg + 1; // F0_32, F0_64, F1_32, F1_64, ...
 }
----------------
We've lost the assertion here; probably want something like `assert(Reg >= RISCV::F0_32 && Reg <= RISCV::F31_32 && Reg % 2 == RISCV::F0_32 % 2);`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67423