jasonmolenda wrote:

> > Doesn't seem the ideal format given that we have a known size, today most 
> > often 16/32/64, and I guess 48 for funsies.
> 
> Standard instructions are right now only 16/32, but custom instructions can 
> be any multiple of 16. This was the change to llvm-objdump to group bytes 
> like gnu objdump does: 
> [b27f86b](https://github.com/llvm/llvm-project/commit/b27f86b40b20942c0e809128214b43d6edde365a)
>  which was only a bit over a year ago.

Thanks again for the helpful pointers and explanations.  I'm surprised they 
gave up so many bits to help us distinguish opcodes!  But we really should be 
using this in lldb as long as they're there.

I don't think there's any good reason to print riscv instructions like we do 
with the intel variable length instructions in lldb's disassembler.  Printing 
as a UInt16, UInt32, UInt16 + UInt16 + UInt16, or UInt32 + UInt32 etc seems 
like a good change to make to lldb's disassembler style for riscv + `-b`.

I'm only skimming the patch to Opcode::Dump, but it looks like the existing 
code is relying on an enum when the Opcode object is constructed,
```
  enum Type {
    eTypeInvalid,
    eType8,
    eType16,
    eType16_2, // a 32-bit Thumb instruction, made up of two words
    eType32,
    eType64,
    eTypeBytes
  };
```

and the riscv instructions are constructed with the `Opcode(uint8_t *bytes, 
size_t length)` ctor so it gets `eTypeBytes` and it's printed that way.  It 
looks like the `InstructionLLVMC::Decode()` function in DisassemblerLLVMC.cpp 
is creating these?  

I think changing the way we print the bytes for the riscv disassembler -b is a 
very reasonable change, instead of printing an array of UInt8's, and it doesn't 
seem like we should need to break this apart into a separate Opcode::DumpRISCV 
method?  Adding `opcode_name = "<unknown>"` i'm less immediately thrilled by, 
but I don't know what we'd print here that was useful as an alternative, maybe 
it's fine, would like to hear other opinions.

https://github.com/llvm/llvm-project/pull/145793
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to