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 implemented. I think this one is what most plugins > > provide. > > > > One way to figure this out is to figure out what actually needs this. > > Return false from both and try a bunch of things to see if it fails, run an > > expression, step in and out etc. > > > > I'd be more comfortable having one not implemented if we know how the other > > one gets used. > The first one is used for calling functions via JIT. The second is used for > calling functions via the IR Interpreter. I didn't want to enable JIT, so I > took the Hexagon implementation (Hexagon doesn't support JIT in lldb, but can > call functions with the IR interpreter) and reworked it for RISC-V. > > Here's a function call on riscv64: > (lldb) re r pc > pc = 0x00000000000106b2 factrv64`main + 28 at factorial.c:32:8 > factrv64`main + 28 at factorial.c:32:8 > (lldb) p factorial(3) > (int) 6 > Ok cool, as long as one of us knows what these do :) The other way to kinda chaos engineer this is to take your host's target, make a function fail and see what parts of the test suite fail. Then that can tell you what to test manually for riscv's version of the same functions. ================ 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,"; } ---------------- ted wrote: > DavidSpickett wrote: > > You might want to take the lead from AArch64 here: > > ``` > > // If any AArch64 variant, enable latest ISA with all extensions. > > ``` > > If "+all" doesn't already work for riscv then you don't have to go and make > > that work right now. > > > > But in general we decided that much like llvm-objdump, we'll try to > > disassemble any possible encoding. If the user happens to point the > > disassembler at garbage that looks like a fancy extension on a cpu from 20 > > years ago, that's on them. > While I like the "turn on the latest" philosophy in general, for RISC-V we > don't want to do that. It's modular architecture means features can be turned > on and off when a core is designed, so one core might have +d (floating point > double), while an newer core might not have any floating point at all. I'm > inclined to leave the features as they are now, with a and m turned on. Fair enough, I see that +all does not in fact work for risc-v llvm-objdump which backs that up. I guess you'd have to pass through some kind of object attributes to detect some of them. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159101/new/ https://reviews.llvm.org/D159101 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
