[Lldb-commits] [PATCH] D62732: [WIP][RISCV] Initial port of LLDB for RISC-V

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda added inline comments.


================
Comment at: lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp:624-635
+    case llvm::Triple::riscv32:
+    case llvm::Triple::riscv64:
+      for (auto &reg : m_regs) {
+        if (strcmp(reg.name, "x1") == 0)
+          reg.kinds[eRegisterKindGeneric] = LLDB_REGNUM_GENERIC_RA;
+        else if (strcmp(reg.name, "x2") == 0)
+          reg.kinds[eRegisterKindGeneric] = LLDB_REGNUM_GENERIC_SP;
----------------
clayborg wrote:
> Seems like you filled all of this info in the registers defs already in ABISysV_riscv.cpp? Do we need this here? Some platforms weren't setting these correctly and this code was meant to correct that.
There's AugmentRegisterInfoViaABI() over in ProcessGDBRemote which does fill in eh_frame, dwarf, and generic register numbers based on the ABI's register table.  It seems like that should be sufficient - although I see a bunch of other architectures doing the exact same thing here in DynamicRegisterInfo::AddRegister. 


================
Comment at: lldb/source/Target/Thread.cpp:2060-2084
     switch (machine) {
     case llvm::Triple::x86_64:
     case llvm::Triple::x86:
     case llvm::Triple::arm:
     case llvm::Triple::aarch64:
     case llvm::Triple::thumb:
     case llvm::Triple::mips:
----------------
clayborg wrote:
> Seems like the original logic before your mods is not correct. Not sure the default case ever gets used here. The arch of x86_64, x86, arm, aarch64 or thumb should all be caught before we get to the default statement for apple targets. I will add Jason Molenda to the change to get a comment on this.
The default case shouldn't ever be taken - it was the unwinder we used (I think you wrote it during bringup) before UnwindLLDB existed.  I think we've kept it around in case people need an initial unwinder during bringup of new architectures, but maybe it's not serving any purpose at all any more.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62732