[Lldb-commits] [PATCH] D159101: [RISC-V] Add RISC-V ABI plugin

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

In the interests of not being caught by the impeding shutdown at the end of the month, this is where we're at with this:

I'm interested in the test suite results, but mostly for context of where this support is at, and not to block this. If it shows up bugs in
anything that's fully implemented here then of course fix them. If there are large parts that still fail for other reasons, I think those should
be dealt with in follow up patches (it's going to be cleaner that way anyway).

Could you summarise those results in the commit message too? (x mostly works, y all fail but we expect that, z fails for as yet unknown reasons, etc.)

Other than that you have @jasonmolenda  's comment about unwinding. Make sure you're both on the same page there.



================
Comment at: lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.cpp:156
+  size_t reg_size = is_rv64 ? 8 : 4;
+  size_t word_size = is_rv64 ? 8 : 4;
+  size_t total_size = 0;
----------------
Nit: you could just say word_size = reg_size.

The separate names is fine since it helps to explain the calculation below, so keep those.


================
Comment at: lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.cpp:203
+  size_t reg_size = m_is_rv64 ? 8 : 4;
+  size_t word_size = m_is_rv64 ? 8 : 4;
+  // Push host data onto target.
----------------
word_size = reg_size; ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159101