[libunwind][RISCV] Support reading of VLENB CSR register

[kachkov98]

Summary

Support read of VLENB (vector byte length) control register (CSR number: 0xC22, DWARF register number: 0x1C22 according to RISC-V DWARF specification: https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-dwarf.adoc). This support is needed for correct unwinding of RVV objects on stack.

Required for fix of https://github.com/llvm/llvm-project/issues/58356

[kito-cheng]

LGTM, just one nit :)

libunwind/src/Registers.hpp

4106

nit: __asm__("csrr %0, 0xC22" : "=r"(vlenb));

[kachkov98]

Review changes

[kito-cheng]

Still LGTM, I guess we need some libunwind guy to approve that?

[MaskRay]

Can you write a test? Run tests with ninja check-unwind

Changing _LIBUNWIND_HIGHEST_DWARF_REGISTER to such a large integer makes
RegisterLocation savedRegisters[kMaxRegisterNumber + 1]; consume a lot of stack space and harms performance.

libunwind/include/libunwind.h

1026

[MaskRay]

Please mention in the summary the official document defining UNW_RISCV_VLENB. Why is it so large?

[kachkov98]

Add test and review fixes

[kachkov98]

In D136264#3881329, @MaskRay wrote:

Please mention in the summary the official document defining UNW_RISCV_VLENB. Why is it so large?

Link to the documentation: https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-dwarf.adoc - it's already placed before RISC-V registers enum, I've also added it to the commit message and copied DWARF numbers from the table to the enum comment

[kachkov98]

In D136264#3881323, @MaskRay wrote:

Can you write a test? Run tests with ninja check-unwind

Changing _LIBUNWIND_HIGHEST_DWARF_REGISTER to such a large integer makes
RegisterLocation savedRegisters[kMaxRegisterNumber + 1]; consume a lot of stack space and harms performance.

Thank you, I didn't noticed that. Fortunately, this register is read-only and we don't need to save it, so in this particular case it looks ok to not change MaxRegisterNumber at all. Looks like NEC's Vector Engine target in libunwind already does the same thing for the UNW_VE_VIXR, UNW_VE_VL registers.

[kachkov98]

Ping

[kachkov98]

Ping

[MaskRay]

Support read of VLENB (vector byte length) control register (CSR number: 0xC22, DWARF register number: 0x1C22 according to RISC-V DWARF specification: https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-dwarf.adoc). This support is needed for correct unwinding of RVV objects on stack.

I don't find 0x1C22 on https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-dwarf.adoc

No, please don't share false information.

[craig.topper]

In D136264#3939348, @MaskRay wrote:

Support read of VLENB (vector byte length) control register (CSR number: 0xC22, DWARF register number: 0x1C22 according to RISC-V DWARF specification: https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-dwarf.adoc). This support is needed for correct unwinding of RVV objects on stack.

I don't find 0x1C22 on https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-dwarf.adoc

No, please don't share false information.

The DWARF spec generically says CSRs are assigned their CSR address plus 4096.

[MaskRay]

In D136264#3939352, @craig.topper wrote:

In D136264#3939348, @MaskRay wrote:

Support read of VLENB (vector byte length) control register (CSR number: 0xC22, DWARF register number: 0x1C22 according to RISC-V DWARF specification: https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-dwarf.adoc). This support is needed for correct unwinding of RVV objects on stack.

I don't find 0x1C22 on https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-dwarf.adoc

No, please don't share false information.

The DWARF spec generically says CSRs are assigned their CSR address plus 4096.

This information helps but is still unsatisfactory.

The RISC-V specification defines a total of 4096 CSRs (see <<riscv-priv>>).
Each CSR is assigned a DWARF register number corresponding to its specified CSR
number plus 4096.

[bibliography]
== References

* [[[riscv-priv]]] "The RISC-V Instruction Set Manual, Volume II: Privileged
Architecture, Document", Editors Andrew Waterman, Krste Asanovi´c, and
John Hauser, RISC-V International.

I don't think [[riscv-priv] assigns 0xc22 to VLENB. Referencing v-spec.adoc in riscv-elf-psabi-doc will probably help.

[kachkov98]

Added link to v-spec with VLENB definition

[kachkov98]

Ping

[MaskRay]

libunwind/test/unwind_scalable_vectors.pass.cpp

16

static

16

Add a comment what this function does.

[kachkov98]

Addressing review comments

[kachkov98]

Fix diff

[MaskRay]

libunwind/test/unwind_scalable_vectors.pass.cpp

33

This is: DW_OP_breg 0 DW_OP_bregx 0x3822 0 DW_OP_plus.

Why is 0x3822 used?

[MaskRay]

Looks great!

libunwind/test/unwind_scalable_vectors.pass.cpp

33

Ah, sorry, it's DW_OP_breg2 0 DW_OP_bregx 0x1C22 0 DW_OP_plus. This is correct.

[MaskRay]

libunwind/test/unwind_scalable_vectors.pass.cpp

27

Since we use // REQUIRES: linux && target={{riscv64-.+}}, the #if is unneeded.

[whitequark]

I think this revision introduces a bug: namely, without Zicsr enabled, the code would not compile.

The code in getRegister should be wrapped in #if defined(__riscv_v) and if it's not enabled then 0 would be returned.

libunwind/include/libunwind.h

enum {
   UNW_RISCV_F24 = 56,
   UNW_RISCV_F25 = 57,
   UNW_RISCV_F26 = 58,
   UNW_RISCV_F27 = 59,
   UNW_RISCV_F28 = 60,
   UNW_RISCV_F29 = 61,
   UNW_RISCV_F30 = 62,
   UNW_RISCV_F31 = 63,
+  // 65-95 -- Reserved for future standard extensions
+  // 96-127 -- v0-v31 (Vector registers)
+  // 128-3071 -- Reserved for future standard extensions
+  // 3072-4095 -- Reserved for custom extensions
+  // 4096-8191 -- CSRs
+  //
+  // VLENB CSR number: 0xC22 -- defined by section 3 of v-spec:
+  // https://github.com/riscv/riscv-v-spec/blob/master/v-spec.adoc#3-vector-extension-programmers-model
+  // VLENB DWARF number: 0x1000 + 0xC22
+  UNW_RISCV_VLENB = 0x1C22,
 };
 
 // VE register numbers
 enum {
   UNW_VE_S0   = 0,
   UNW_VE_S1   = 1,
   UNW_VE_S2   = 2,
   UNW_VE_S3   = 3,

libunwind/src/Registers.hpp

 
 inline bool Registers_riscv::validRegister(int regNum) const {
   if (regNum == UNW_REG_IP)
     return true;
   if (regNum == UNW_REG_SP)
     return true;
   if (regNum < 0)
     return false;
+  if (regNum == UNW_RISCV_VLENB)
+    return true;
   if (regNum > UNW_RISCV_F31)
     return false;
   return true;
 }
 
 inline reg_t Registers_riscv::getRegister(int regNum) const {
   if (regNum == UNW_REG_IP)
     return _registers[0];
   if (regNum == UNW_REG_SP)
     return _registers[2];
   if (regNum == UNW_RISCV_X0)
     return 0;
   if ((regNum > 0) && (regNum < 32))
     return _registers[regNum];
+  if (regNum == UNW_RISCV_VLENB) {
+    reg_t vlenb;
+    __asm__("csrr %0, 0xC22" : "=r"(vlenb));
+    return vlenb;
+  }
   _LIBUNWIND_ABORT("unsupported riscv register");
 }
 
 inline void Registers_riscv::setRegister(int regNum, reg_t value) {
   if (regNum == UNW_REG_IP)
     _registers[0] = value;
   else if (regNum == UNW_REG_SP)
     _registers[2] = value;
inline const char *Registers_riscv::getRegisterName(int regNum) {
   case UNW_RISCV_F28:
     return "ft8";
   case UNW_RISCV_F29:
     return "ft9";
   case UNW_RISCV_F30:
     return "ft10";
   case UNW_RISCV_F31:
     return "ft11";
+  case UNW_RISCV_VLENB:
+    return "vlenb";
   default:
     return "unknown register";
   }
 }
 
 inline bool Registers_riscv::validFloatRegister(int regNum) const {
 # if defined(__riscv_flen)
   if (regNum < UNW_RISCV_F0)

libunwind/test/unwind_scalable_vectors.pass.cpp

+// -*- C++ -*-
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// REQUIRES: linux && target={{riscv64-.+}}
+
+#undef NDEBUG
+#include <assert.h>
+#include <libunwind.h>
+
+__attribute__((naked)) void foo() {
+#if defined(__riscv)
+  __asm__(".cfi_startproc\n"
+          "mv s0, ra\n"
+          "csrr  s1, vlenb\n"
+          "sub sp, sp, s1\n"
+          "# .cfi_def_cfa_expression sp + vlenb\n"
+          ".cfi_escape 0x0f, 0x07, 0x72, 0x00, 0x92, 0xa2, 0x38, 0x00, 0x22\n"
+          "call stepper\n"
+          "add sp, sp, s1\n"
+          "mv ra, s0\n"
+          "ret\n"
+          ".cfi_endproc\n");
+#else
+#error This test is only supported on riscv64
+#endif
+}
+
+extern "C" void stepper() {
+  unw_cursor_t cursor;
+  unw_context_t uc;
+  unw_getcontext(&uc);
+  unw_init_local(&cursor, &uc);
+  // Stepping into foo() should succeed.
+  assert(unw_step(&cursor) > 0);
+  // Stepping past foo() should succeed, too.
+  assert(unw_step(&cursor) > 0);
+}
+
+int main() { foo(); }