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

[Ted Woodward via Phabricator via lldb-commits]

ted added inline comments.


================
Comment at: lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.cpp:133
+ABISysV_riscv::CreateInstance(ProcessSP process_sp, const ArchSpec &arch) {
+  if (process_sp) {
+    process_sp->SetCanInterpretFunctionCalls(true);
----------------
DavidSpickett wrote:
> Do you know why this check is needed? Is there a specific test that hits this?
> 
> The other ABI plugins seem to sidestep this by not having to `->` on it, but that could just be luck.
Without that check, certain tests crashed when they instantiated the ABI plugin without a process.


================
Comment at: lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.cpp:135
+    process_sp->SetCanInterpretFunctionCalls(true);
+    process_sp->SetCanJIT(false);
+  }
----------------
jasonmolenda wrote:
> These should not be set in the ABI, please remove these.  These are probably correct for the qemu environment you're testing against, but if we're connected to a remote stub that can allocate memory (_M packet), we can JIT code. 
I can remove these, but for the current environment they're correct. We don't have function calling for JIT, or RuntimeDyld set up for RISC-V.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159101