[RISCV] Implement the TargetLowering::getRegisterByName hook

[luismarques]

Summary

The hook should work for any RISC-V register. Non-allocatable registers do not need to be reserved, for the remaining the hook will only succeed if you pass clang the -ffixed-xX flag. This builds upon D67185, which currently only allows reserving GPRs.

[luismarques]

NFC code tweak.

[lenary]

Nice. This is looking good! One correctness issue, and one low priority issue.

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

2902

I think you need to be using the Type information in VT to check the register we're returning has the expected width/type

For instance, @llvm.register_read.i32("sp") on RISC-V 64 should absolutely fail. The same @llvm.register_read.i64("sp") should succeed on 64-bit RISC-V. I don't know if this test is done by the register_read intrinsic lowering or has to be done in this code.

llvm/lib/Target/RISCV/RISCVISelLowering.h

150

Low Priority: It would be nice to have a docstring that explains the implementation in RISC-V (including that this is used by @llvm.read_register.<ty>()/@llvm.write_register.<ty>() intrinsics).

[luismarques]

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

2902

If the type doesn't match the lowering will fail, complaining about not knowing how to promote READ_REGISTER. The hook is never reached, so it's not possible to do such check at the hook.

[luismarques]

• Add method documentation comment;
• Tweak test formatting and var naming.

[luismarques]

• Tweak method doc.

[luismarques]

• Use getReservedRegs instead of hardcoding the reserved registers.
• Update summary.

[lenary]

Please can you test using this with the register named fp. This is another alternate name for x8 (aka s0) which is supported in some places, and should be supported here. I'm not sure if MatchRegisterAltName will pick it up. I think x8 is the only register with three names (the rest all only have two).

[luismarques]

Tweaks the get-register-reserve.ll test, including addressing @lenary's concern of testing that the fp register name is properly accepted.

[lenary]

LGTM

[luismarques]

Rebase, update test that had not be incorrectly updated.

[thakis]

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

2899

This seems a bit strange to me from a dependency point of view. GenAsmMatcher conceptually belongs to RISCVAsmParser, but RISCVCodeGen doesn't depend on RISCVAsmParser. I believe other targets don't include GenAsmMatcher.inc from their codegen libraries (except aarch64, but it's strange there too -- added in D56305, +phosek).

[lenary]

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

2899

It seems reasonable to use the canonical register name matching, here, rather than a big duplicated StringSwitch. Is there a more reasonable way of avoiding duplication but still using the canonical register information from tablegen?

llvm/lib/Target/RISCV/RISCVISelLowering.h

public:
 
   /// If a physical register, this returns the register that receives the
   /// exception typeid on entry to a landing pad.
   unsigned
   getExceptionSelectorRegister(const Constant *PersonalityFn) const override;
 
   bool shouldExtendTypeInLibCall(EVT Type) const override;
 
+  /// Returns the register with the specified architectural or ABI name. This
+  /// method is necessary to lower the llvm.read_register.* and
+  /// llvm.write_register.* intrinsics. Allocatable registers must be reserved
+  /// with the clang -ffixed-xX flag for access to be allowed.
+  Register getRegisterByName(const char *RegName, EVT VT,
+                             const MachineFunction &MF) const override;
+
 private:
   void analyzeInputArgs(MachineFunction &MF, CCState &CCInfo,
                         const SmallVectorImpl<ISD::InputArg> &Ins,
                         bool IsRet) const;
   void analyzeOutputArgs(MachineFunction &MF, CCState &CCInfo,
                          const SmallVectorImpl<ISD::OutputArg> &Outs,
                          bool IsRet, CallLoweringInfo *CLI) const;
   // Lower incoming arguments, copy physregs into vregs

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

bool RISCVTargetLowering::shouldExtendTypeInLibCall(EVT Type) const {
   // Return false to suppress the unnecessary extensions if the LibCall
   // arguments or return value is f32 type for LP64 ABI.
   RISCVABI::ABI ABI = Subtarget.getTargetABI();
   if (ABI == RISCVABI::ABI_LP64 && (Type == MVT::f32))
     return false;
 
   return true;
 }
+
+#define GET_REGISTER_MATCHER
+#include "RISCVGenAsmMatcher.inc"
+
+Register
+RISCVTargetLowering::getRegisterByName(const char *RegName, EVT VT,
+                                       const MachineFunction &MF) const {
+  Register Reg = MatchRegisterAltName(RegName);
+  if (Reg == RISCV::NoRegister)
+    Reg = MatchRegisterName(RegName);
+  if (Reg == RISCV::NoRegister)
+    report_fatal_error(
+        Twine("Invalid register name \"" + StringRef(RegName) + "\"."));
+  BitVector ReservedRegs = Subtarget.getRegisterInfo()->getReservedRegs(MF);
+  if (!ReservedRegs.test(Reg) && !Subtarget.isRegisterReservedByUser(Reg))
+    report_fatal_error(Twine("Trying to obtain non-reserved register \"" +
+                             StringRef(RegName) + "\"."));
+  return Reg;
+}

llvm/test/CodeGen/RISCV/get-register-invalid.ll

+; RUN: not llc < %s -mtriple=riscv32 2>&1 | FileCheck %s
+
+define i32 @get_invalid_reg() nounwind {
+entry:
+; CHECK: Invalid register name "notareg".
+  %reg = call i32 @llvm.read_register.i32(metadata !0)
+  ret i32 %reg
+}
+
+declare i32 @llvm.read_register.i32(metadata) nounwind
+
+!0 = !{!"notareg\00"}

llvm/test/CodeGen/RISCV/get-register-noreserve.ll

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=riscv32 | FileCheck %s
+
+define i32 @get_stack() nounwind {
+; CHECK-LABEL: get_stack:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    mv a0, sp
+; CHECK-NEXT:    ret
+entry:
+  %sp = call i32 @llvm.read_register.i32(metadata !0)
+  ret i32 %sp
+}
+
+define void @set_stack(i32 %val) nounwind {
+; CHECK-LABEL: set_stack:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    mv sp, a0
+; CHECK-NEXT:    ret
+entry:
+  call void @llvm.write_register.i32(metadata !0, i32 %val)
+  ret void
+}
+
+define i32 @get_tp_arch_name() nounwind {
+; CHECK-LABEL: get_tp_arch_name:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    mv a0, tp
+; CHECK-NEXT:    ret
+entry:
+  %sp = call i32 @llvm.read_register.i32(metadata !1)
+  ret i32 %sp
+}
+
+declare i32 @llvm.read_register.i32(metadata) nounwind
+declare void @llvm.write_register.i32(metadata, i32) nounwind
+
+!0 = !{!"sp\00"}
+!1 = !{!"x4\00"}

llvm/test/CodeGen/RISCV/get-register-reserve.ll

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: not llc < %s -mtriple=riscv32 -mattr +reserve-x8 2>&1 \
+; RUN:   | FileCheck -check-prefix=NO-RESERVE-A1 %s
+; RUN: not llc < %s -mtriple=riscv32 -mattr +reserve-x11 2>&1 \
+; RUN:   | FileCheck -check-prefix=NO-RESERVE-FP %s
+; RUN: llc < %s -mtriple=riscv32 -mattr +reserve-x8 -mattr +reserve-x11 \
+; RUN:   | FileCheck -check-prefix=RESERVE %s
+
+define i32 @get_reg_a1() nounwind {
+; NO-RESERVE-A1: Trying to obtain non-reserved register "a1".
+; RESERVE-LABEL: get_reg_a1:
+; RESERVE:       # %bb.0: # %entry
+; RESERVE-NEXT:    mv a0, a1
+; RESERVE-NEXT:    ret
+entry:
+  %a1 = call i32 @llvm.read_register.i32(metadata !0)
+  ret i32 %a1
+}
+
+define i32 @get_reg_fp() nounwind {
+; NO-RESERVE-FP: Trying to obtain non-reserved register "fp".
+; RESERVE-LABEL: get_reg_fp:
+; RESERVE:       # %bb.0: # %entry
+; RESERVE-NEXT:    mv a0, s0
+; RESERVE-NEXT:    ret
+entry:
+  %fp = call i32 @llvm.read_register.i32(metadata !1)
+  ret i32 %fp
+}
+
+declare i32 @llvm.read_register.i32(metadata) nounwind
+
+!0 = !{!"a1\00"}
+!1 = !{!"fp\00"}