[LoongArch] Implement the TargetLowering::getRegisterByName hook

[gonglingqin]

Summary

Only reserved registers can be read.

[SixWeining]

llvm/test/CodeGen/LoongArch/get-noreg.ll

2 ↗

(On Diff #473576)

crash may slow down the test. Could we not add such tests?

llvm/test/CodeGen/LoongArch/get-reg-error.ll

2 ↗

(On Diff #473576)

Ditto.

llvm/test/CodeGen/LoongArch/get-reg-reserve.ll

1 ↗

(On Diff #473576)

I suggest to rename get-reg-reserve.ll to get-reg.ll since get-reg-reserve.ll may imply we support getting non-reserved registers.

[gonglingqin]

llvm/test/CodeGen/LoongArch/get-noreg.ll

2 ↗

(On Diff #473576)

Thanks, I will remove this test.

llvm/test/CodeGen/LoongArch/get-reg-error.ll

2 ↗

(On Diff #473576)

Thanks, I will remove this test.

llvm/test/CodeGen/LoongArch/get-reg-reserve.ll

1 ↗

(On Diff #473576)

Thanks, I will rename this file.

[gonglingqin]

Address @SixWeining's comments.

[SixWeining]

LGTM.

[xen0n]

I'm curious what usage case motivated the change? Not to say it's unnecessary but it's better to have people know a bit more background behind this work.

[gonglingqin]

In D137532#3923952, @xen0n wrote:

I'm curious what usage case motivated the change? Not to say it's unnecessary but it's better to have people know a bit more background behind this work.

This interface is required by the code in https://github.com/loongson/linux/blob/master/arch/loongarch/include/asm/percpu.h#L12-L18. This patch can avoid crashes when compiling the kernel with llvm in the future.

[xen0n]

In D137532#3924037, @gonglingqin wrote:

In D137532#3923952, @xen0n wrote:

I'm curious what usage case motivated the change? Not to say it's unnecessary but it's better to have people know a bit more background behind this work.

This interface is required by the code in https://github.com/loongson/linux/blob/master/arch/loongarch/include/asm/percpu.h#L12-L18. This patch can avoid crashes when compiling the kernel with llvm in the future.

Very nice to know.

BTW, are the normal GPRs already supported like this too (things like global register long foo __asm__("$a0")), even without this change, or are such usages invalid in the first place? Memory is failing me.

[xry111]

In D137532#3924134, @xen0n wrote:

In D137532#3924037, @gonglingqin wrote:

In D137532#3923952, @xen0n wrote:

I'm curious what usage case motivated the change? Not to say it's unnecessary but it's better to have people know a bit more background behind this work.

This interface is required by the code in https://github.com/loongson/linux/blob/master/arch/loongarch/include/asm/percpu.h#L12-L18. This patch can avoid crashes when compiling the kernel with llvm in the future.

Very nice to know.

BTW, are the normal GPRs already supported like this too (things like global register long foo __asm__("$a0")), even without this change, or are such usages invalid in the first place? Memory is failing me.

I've not built LLVM and Clang for LoongArch recently, but on gcc400.fsffrance.org (with Arch Linux for LoongArch installed) I got:

$ clang -v 2>&1 | head -n1
clang version 14.0.6
$ cat t.c
register int y __asm__("$s0");
int o() { register int x __asm__("$t0"); return y; }
$ clang t.c -S
fatal error: error in backend: Invalid register name global variable
... ... (stack trace)

But if we change it to return x;, the code will be compiled successfully. So it looks like local register variables with assigned GPR have been supported, but the global ones are not.

[gonglingqin]

In D137532#3924134, @xen0n wrote:

In D137532#3924037, @gonglingqin wrote:

In D137532#3923952, @xen0n wrote:

I'm curious what usage case motivated the change? Not to say it's unnecessary but it's better to have people know a bit more background behind this work.

This interface is required by the code in https://github.com/loongson/linux/blob/master/arch/loongarch/include/asm/percpu.h#L12-L18. This patch can avoid crashes when compiling the kernel with llvm in the future.

Very nice to know.

BTW, are the normal GPRs already supported like this too (things like global register long foo __asm__("$a0")), even without this change, or are such usages invalid in the first place? Memory is failing me.

As @xry111 said, any local register variables with assigned GPR can be read (the analysis process is completed by clang), but only reserved registers can be read when reading global registers.

[thakis]

llvm/lib/Target/LoongArch/LoongArchISelLowering.cpp

2462

This is a bit strange to me from a dependency point of view. GenAsmMatcher conceptually belongs to LoongArchAsmParser, but LoongArchCodeGen doesn't depend on LoongArchAsmParser. I believe other targets don't include GenAsmMatcher.inc from their codegen libraries (except aarch64 and riscv, but it's strange there too -- see also https://reviews.llvm.org/D69130#inline-627969).

Any chance this could be layered nicer?

[gonglingqin]

llvm/lib/Target/LoongArch/LoongArchISelLowering.cpp

2462

Thanks for your advice. We considered using StringSwitch for matching, but we chose the current implementation because of code reuse. Do you think using stringswitch is a better way to do this, or do you have any suggestions?

llvm/lib/Target/LoongArch/LoongArchISelLowering.h

public:
 
   Register
   getExceptionSelectorRegister(const Constant *PersonalityFn) const override;
 
   ISD::NodeType getExtendForAtomicOps() const override {
     return ISD::SIGN_EXTEND;
   }
 
+  Register getRegisterByName(const char *RegName, LLT VT,
+                             const MachineFunction &MF) const override;
+
 private:
   /// Target-specific function used to lower LoongArch calling conventions.
   typedef bool LoongArchCCAssignFn(const DataLayout &DL, LoongArchABI::ABI ABI,
                                    unsigned ValNo, MVT ValVT,
                                    CCValAssign::LocInfo LocInfo,
                                    ISD::ArgFlagsTy ArgFlags, CCState &State,
                                    bool IsFixed, bool IsReg, Type *OrigTy);
 

llvm/lib/Target/LoongArch/LoongArchISelLowering.cpp

case 'K':
       }
       return;
     default:
       break;
     }
   }
   TargetLowering::LowerAsmOperandForConstraint(Op, Constraint, Ops, DAG);
 }
+
+#define GET_REGISTER_MATCHER
+#include "LoongArchGenAsmMatcher.inc"
+
+Register
+LoongArchTargetLowering::getRegisterByName(const char *RegName, LLT VT,
+                                           const MachineFunction &MF) const {
+  std::pair<StringRef, StringRef> Name = StringRef(RegName).split('$');
+  std::string NewRegName = Name.second.str();
+  Register Reg = MatchRegisterAltName(NewRegName);
+  if (Reg == LoongArch::NoRegister)
+    Reg = MatchRegisterName(NewRegName);
+  if (Reg == LoongArch::NoRegister)
+    report_fatal_error(
+        Twine("Invalid register name \"" + StringRef(RegName) + "\"."));
+  BitVector ReservedRegs = Subtarget.getRegisterInfo()->getReservedRegs(MF);
+  if (!ReservedRegs.test(Reg))
+    report_fatal_error(Twine("Trying to obtain non-reserved register \"" +
+                             StringRef(RegName) + "\"."));
+  return Reg;
+}

llvm/test/CodeGen/LoongArch/get-reg.ll

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