Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
llvm/lib/Target/Xtensa/Disassembler/XtensaDisassembler.cpp | ||
---|---|---|
29 | using is more common in newer code. This doesn't have to copy from other targets. |
Patch is updated according to LLVM upstream version and latest Xtensa backend version.
Correct instruction descriptions, format descriptions and instruction operands according to common style for *.td files. The llvm_unreachable is substituted to report_fatal_error.
This is largely mechanical, so mostly looks good with a minor nit.
Disassembly is likely to run into issues with invalid instructions in the stream because the llvm disassembler doesn't honor .xt.insn section notes. For example, the nop padding from D64831 is going to trip this up if it runs into a single-byte pad, or the two-byte density nops, for that matter. This is one of the caveats of Xtensa's 2 and 3 byte instructions when you need to pad a single byte--there isn't any valid instruction to do that.
The alternative would be to never pad with illegal instructions. On a density machine, if you need a single-byte of padding, you can five-bytes (fetch width + remainder). On a non-density machine, it gets more complicated.
Still, this is not exactly a correctness issue, but it will make using this disassembler for real work tricky. Honoring .xt.insn sections is likely beyond the scope here.
Worse, the gnu assembler won't be able to properly disassemble these situations either, as this implementation doesn't generate .xt.insn sections.
I suppose people who care can use the gnu assembler and disassembler rather than llvm's built in version.
llvm/lib/Target/Xtensa/Disassembler/XtensaDisassembler.cpp | ||
---|---|---|
172 | this only reads three bytes, does it not? And no need to comment about the endianness--it isn't supported. |
Corrected comment. The "array_lengthof" changed to "std::size".
llvm/lib/Target/Xtensa/Disassembler/XtensaDisassembler.cpp | ||
---|---|---|
172 | Corrected |
@saugustine , we added to the our plans implementation of the ".xt.insn" section. Is it suitable solution to resolve the issue with "nop" operation in this patch?
llvm/lib/Target/Xtensa/Disassembler/XtensaDisassembler.cpp | ||
---|---|---|
205 | This should probably be done inside readInstruction24. |
Added fixes according to comments
llvm/lib/Target/Xtensa/Disassembler/XtensaDisassembler.cpp | ||
---|---|---|
205 | Fixed |
llvm/lib/Target/Xtensa/Disassembler/XtensaDisassembler.cpp | ||
---|---|---|
179 | (cosmetic) If you have only two bytes in the buffer, this method will be called twice, fail twice, and <unknown> will be printed twice, which is probably not what you want. |
using is more common in newer code. This doesn't have to copy from other targets.