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

ted created this revision. Herald added subscribers: luke, sunshaoce, VincentWu, vkmr, frasercrmck, luismarques, apazos, sameer.abuasal, s.egerton, Jim, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, jrtc27, shiva0217, kito-cheng, niosHD, sabuasal, simoncook, jo

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

DavidSpickett added a reviewer: DavidSpickett. DavidSpickett added a comment. From what little I know about the ABI this looks ok. Consider adding a riscv expert to check those bits for you. Testing wise riscv is in a strange place without a working lldb-server or a buildbot that could run such

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

DavidSpickett added a comment. Also could you provide a list of things you have tested with qemu. So I can get an idea of how sure we can be any of this works / be a test plan, albeit manual for anyone updating this code in future. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTI

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

ted added a subscriber: labath. ted added a comment. With this change, I'm able to debug RISC-V 32 and 64 bit applications using Linux user space QEMU, using @labath 's QemuUser platform. Simple expression parsing with the IR Interpreter is working. Complex expression parsing with JIT is not te

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

ted added reviewers: jasonmolenda, labath, asb, lewis-revill, compnerd, simoncook, jrtc27, JDevlieghere, aprantl, sven, kasper81, tzb99. ted added a comment. Adding commenters from https://reviews.llvm.org/D62732 as reviewers. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION h

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

ted added a comment. In D159101#4625539 , @DavidSpickett wrote: > Also could you provide a list of things you have tested with qemu. So I can > get an idea of how sure we can be any of this works / be a test plan, albeit > manual for anyone updating th

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

ted marked 13 inline comments as done. ted added inline comments. Comment at: lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.cpp:107 +static size_t word_size = 4U; +static size_t reg_size = word_size; + DavidSpickett wrote: > What's the reason to do this this way? I

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

ted updated this revision to Diff 554505. ted added a comment. Updated with David Spickett's comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159101/new/ https://reviews.llvm.org/D159101 Files: lldb/source/Plugins/ABI/CMakeLists.txt lldb

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

jasonmolenda added inline comments. Comment at: lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.cpp:107 +static size_t word_size = 4U; +static size_t reg_size = word_size; + ted wrote: > DavidSpickett wrote: > > What's the reason to do this this way? It seems like ad

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

DavidSpickett added inline comments. Comment at: lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.cpp:145 + return false; +} + ted wrote: > DavidSpickett wrote: > > Looking at the comments in lldb/include/lldb/Target/ABI.h I'm not sure > > which of these should be i

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

DavidSpickett added a comment. > I've done the following: Great. Could you include that in the commit message? Seems like we get a lot of questions about how mature risc-v support is, so we can point to this as a milestone for that. There is https://lldb.llvm.org/#platform-support but there's

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

ted added a comment. In D159101#4627705 , @DavidSpickett wrote: > I have some vague idea that maybe we could put a hack in the test suite to > use the qemu-user platform instead of lldb-server. To at least give it a go, > but I haven't tried it myself

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

kito-cheng added inline comments. Comment at: lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp:1554 +// Turn them on by default now, since everyone seems to use them +features_str += "+a,+m,"; } DavidSpickett wrote: > ted wrote: > > DavidSp

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

ted added a comment. In D159101#4627705 , @DavidSpickett wrote: > Great. Could you include that in the commit message? Seems like we get a lot > of questions about how mature risc-v support is, so we can point to this as a > milestone for that. Will d

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

ted updated this revision to Diff 555155. ted marked an inline comment as done. ted added a comment. Implement Jason Molenda's request to use an ivar instead of static variables. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159101/new/ https://rev

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

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 any

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

ted added a comment. @DavidSpickett update on testing: I'm running tests. I found an issue in the IR Interpreter when calling class methods. Somehow a 32 bit pointer gets the upper 32 bits in the 64 bit uint it's stored in set to 0x, which causes an assert. The assert should probably be

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

DavidSpickett added a comment. > I can make this change here, but I wonder if it should be a separate change. Sounds like bitness confusion, so yes it's worth making a patch for that. We don't test that scenario on the build bots but people do do this. Repository: rG LLVM Github Monorepo CH

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

ted updated this revision to Diff 557049. ted added a comment. Fix crash when there was no process. Remove bugs in function call setup when using IR interpreter. Change flag tests to use lldb_private::Flags for flags testing Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION http

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

DavidSpickett added inline comments. Comment at: lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.cpp:262 + // NOTE: It's unsafe to iterate through LLDB_REGNUM_GENERICs + // But the a registers re sequential in the RISC-V register space + ++reg_index; S

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

DavidSpickett added a comment. FYI I will be away next week, and the week after that Phab is supposed to be made read-only. So look to @jasonmolenda to approve in that time frame, or we can take this to a PR after that. Comment at: lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.

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

jasonmolenda added inline comments. Comment at: lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.cpp:135 +process_sp->SetCanInterpretFunctionCalls(true); +process_sp->SetCanJIT(false); + } These should not be set in the ABI, please remove these. These are pr

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

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:

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

ted updated this revision to Diff 557321. ted added a comment. Remove disabling JIT and enabling IR Interpreter function calls, as requested by Jason Molenda. Added default unwind plan as suggested by Jason Molenda. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revi

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

ted updated this revision to Diff 557322. ted marked 3 inline comments as done. ted added a comment. Change "word_size = m_is_rv64 ? 8 : 4" to "word_size = reg_size" Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159101/new/ https://reviews.llvm.org

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

ted updated this revision to Diff 557323. ted marked 4 inline comments as done. ted added a comment. Fix comment about "a" registers Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159101/new/ https://reviews.llvm.org/D159101 Files: lldb/source/Pl

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

jasonmolenda accepted this revision. jasonmolenda added a comment. This revision is now accepted and ready to land. This looks good to me, thanks for reviving this and finishing it up. We should land before phabracator is flash frozen, we can iterate issues are found in the future. =

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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG847de9c33277: [RISC-V] Add RISC-V ABI plugin (authored by ted). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.l