[Lldb-commits] [PATCH] D62732: [RISCV] Add SystemV ABI

Thu Oct 1 07:32:07 PDT 2020

labath added a comment.

ABI plugins are one of the hardest things to test in lldb, particularly without actual hardware. That's why we've let them be added in the past without any accompanying tests. The situation is not ideal though, because we've accumulated various ABI plugins which are hard to change without breaking (or even to know if they work) because they only work with third-party (possibly proprietary) stubs.

I'm not sure what's the state of risc-v hardware these days and how much resources do you have available, but if it's at all possible, I'd definitely recommend adding the lldb-server bits for risc-v and adding a builtbot for testing this configuration.

Even if there's no hardware which could run the lldb test suite in a reasonable amount of time, the availability of an lldb-server implementation would enable anyone to test this via qemu -- that's the strategy we're pursuing for the ARM SVE stuff, and we're about to add instructions on how to run lldb+sve+qemu here: D82064 <https://reviews.llvm.org/D82064>.

================
Comment at: lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.cpp:37-276
+enum riscv_dwarf_regnums {
+  dwarf_x0 = 0,
+  dwarf_x1,
+  dwarf_x2,
+  dwarf_x3,
+  dwarf_x4,
+  dwarf_x5,
----------------
It would be good to make this an MCBasedABI (that's a new concept, introduced after the inception of this patch). That should make all of this goo disappear.

================
Comment at: lldb/source/Utility/ArchSpec.cpp:460-463
+    {ArchSpec::eCore_riscv32, llvm::ELF::EM_RISCV, LLDB_INVALID_CPUTYPE,
+     0xFFFFFFFFu, 0xFFFFFFFFu}, // riscv32
+    {ArchSpec::eCore_riscv64, llvm::ELF::EM_RISCV, LLDB_INVALID_CPUTYPE,
+     0xFFFFFFFFu, 0xFFFFFFFFu}, // riscv64
----------------
These bits should go in as a part of D86292.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62732