[llvm] clang crash assigning to a global named register variable #109778 (PR #113105)

[David Spickett via llvm-commits]

================
@@ -11519,17 +11519,14 @@ SDValue AArch64TargetLowering::LowerSPONENTRY(SDValue Op,
 Register AArch64TargetLowering::
 getRegisterByName(const char* RegName, LLT VT, const MachineFunction &MF) const {
   Register Reg = MatchRegisterName(RegName);
-  if (AArch64::X1 <= Reg && Reg <= AArch64::X28) {
-    const AArch64RegisterInfo *MRI = Subtarget->getRegisterInfo();
-    unsigned DwarfRegNum = MRI->getDwarfRegNum(Reg, false);
-    if (!Subtarget->isXRegisterReserved(DwarfRegNum) &&
-        !MRI->isReservedReg(MF, Reg))
-      Reg = 0;
-  }
-  if (Reg)
-    return Reg;
-  report_fatal_error(Twine("Invalid register name \""
-                              + StringRef(RegName)  + "\"."));
+  if (Reg == AArch64::NoRegister)
+    report_fatal_error(
+        Twine("Invalid register name \"" + StringRef(RegName) + "\"."));
+  BitVector ReservedRegs = Subtarget->getRegisterInfo()->getReservedRegs(MF);
+  if (!ReservedRegs.test(Reg) && !Subtarget->isRegisterReservedByUser(Reg))
----------------
DavidSpickett wrote:

This is how other targets do it but AArch64 does not separate user reserved vs. reserved for ABI reasons (x18 is usually reserved for the OS to use).

I think you can still improve the error message but it'll need to check ReserveXRegister.

The tell tale sign this doesn't work is that nothing in AArch64 writes to UserReservedRegister but in RISCV and M68K they do (find in files or grep in `llvm/`, even if you don't understand what they're doing, it'll tell you something interacts with it at least).

I don't think it's worth moving AArch64 to the same style just for a better error though.

I think the tests pass because for AArch64 `-ffixed-xN` does set a bit in `ReservedRegs.test` and so the fact that `isRegisterReservedByUser` will always be False doesn't change anything.

I think if this becomes:
if (!ReservedRegs.test(Reg))

That should work. Then remove the new bitset you added.

Another way to do this is to keep the original code but instead of:
```
if (!Subtarget->isXRegisterReserved(DwarfRegNum) &&
        !MRI->isReservedReg(MF, Reg))
      Reg = 0;
```
report_fatal_error here with the improved message.

https://github.com/llvm/llvm-project/pull/113105