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

[Kito Cheng via Phabricator via lldb-commits]

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:
> > 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.
RISC-V have ELF attribute* to record which arch used for this binary, so you may need to add something at `lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp` to read that and record something in ArchSpec like what `ObjectFileELF::ParseARMAttributes` do.

* https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-elf.adoc#tag_riscv_arch-5-ntbssubarch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159101