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

Wed Jul 29 14:01:24 PDT 2020

luismarques added a comment.

In D62732#2038309 <https://reviews.llvm.org/D62732#2038309>, @jrtc27 wrote:

> Yeah, I don't think we want to be merging code we can't test even in a non-automated way. Even if this code is completely bug-free, the inability to test it just means we risk having it bit-rot with nobody noticing.

We now have two open-source debug servers that we can use to test this patch: gdbserver and QEMU's gdbstub, both of which now have RISC-V support. I rebased this patch and tested it with both.
My initial tests were with QEMU/gdbstub, and I was quite pleased with the results. I could set breakpoints, continue execution, step line-by-line and instruction-by-instruction and print registers (GPRs and CSRs). The backtrace was only showing the current frame. Still, I would say that LLDB was in a good enough state to be useful. Next, I tested it against gdbserver (compiled from git main) running in a Fedora RISC-V machine. Setting a breakpoint would seemingly succeed (e.g. it reported success, with the correct code address), but the code doesn't actually trap. You can still break with Ctrl-C though, and print registers and so on. So there seem to be some additional issues, but I also had problems when connecting less recent versions of GDB to that debug server, so we might just need to do some adjustments to match the GDB changes.

It seems that, once rebased, this patch is in pretty good shape. lowRISC is keen to see LLDB get RISC-V support, and I'm now ramping up work to help with that. Given the great work that we already have in this patch, I think an important first step would be to land it. None of the issues I encountered were obviously due to the code in this patch (and the things still marked as TODO), so I don't see any major downside to merging it. I will do a more in-depth investigation of the issues I encountered, but that shouldn't be a blocker to this patch. Landing this contribution would make it easier for others to contribute the missing pieces, and fixes for the issues, without having to submit duplicate work.

If it helps, feel free to use this commit [1], it's pretty much just a rebase of this patch.
@simoncook please let me know if you plan to update this patch and get it approved and landed (LGTM, once rebased), or other ways in which it would make sense to coordinate our work.

[1] https://github.com/luismarques/llvm-project/commit/4a0cccb0cfa8cb5c59c8803077a76751498447ec

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62732