Add printer for current instructions and operands subsets.
Also add basic tests of the Xtensa instructions.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Patch is updated according to LLVM upstream version and latest Xtensa backend version.
llvm/test/MC/Xtensa/xtensa-invalid.s | ||
---|---|---|
13 ↗ | (On Diff #425076) | It would be good to check instruction arguments out of order, and invalid register names. |
llvm/test/MC/Xtensa/xtensa-valid.s | ||
4 ↗ | (On Diff #425076) | It would be good here to comments marking which instruction formats (RRR, RRI4, RRI8 and so on). re covered, as the difference between them is a common source of bugs. Perhaps next to one of the instructions. I assume that eventually there will be comprehensive coverage of all the different formats. |
61 ↗ | (On Diff #425076) | Spaces after the commas in this file is inconsistent. |
llvm/test/MC/Xtensa/xtensa-invalid.s | ||
---|---|---|
13 ↗ | (On Diff #425076) | You mean for each instruction implement extended tests with argument out of order and invalid register names? |
llvm/test/MC/Xtensa/xtensa-invalid.s | ||
---|---|---|
13 ↗ | (On Diff #425076) | Probably just once for each format, but yes. |
llvm/lib/Target/Xtensa/MCTargetDesc/XtensaInstPrinter.cpp | ||
---|---|---|
34–35 | Just use cast<> |
Minor code corrections. Test file xtensa-valid.s is splitted to several files by instructions groups.
llvm/lib/Target/Xtensa/MCTargetDesc/XtensaInstPrinter.cpp | ||
---|---|---|
34–35 | Corrected | |
90 | Corrected | |
94 | Corrected | |
101 | Corrected | |
llvm/test/MC/Xtensa/elf-header.s | ||
5 ↗ | (On Diff #455444) | Corrected |
llvm/test/MC/Xtensa/xtensa-invalid.s | ||
13 ↗ | (On Diff #425076) | I added more tests for different instruction formats. |
llvm/test/MC/Xtensa/xtensa-valid.s | ||
1 ↗ | (On Diff #455444) | Thank you for advice, I've splitted file to several files by commands groups. |
4 ↗ | (On Diff #425076) | Added format information to each instruction. |
@MaskRay , may I ask you about new test structure of the instructions set? I divided test file xtensa-valid.s to several files by instructions groups. Whether such division matches to your suggestions?
LGTM with nit
llvm/lib/Target/Xtensa/MCTargetDesc/XtensaInstPrinter.cpp | ||
---|---|---|
35 | Also means you can get rid of the assert(false) and if |
llvm/test/MC/Xtensa/elf-header.s | ||
---|---|---|
2 ↗ | (On Diff #461118) | Why not just use CHECK? And please keep these uppercase so they stand out. |
llvm/test/MC/Xtensa/elf-header.s | ||
---|---|---|
2 ↗ | (On Diff #461118) | Thank you for comment, test is fixed. |
llvm/lib/Target/Xtensa/MCTargetDesc/XtensaInstPrinter.h | ||
---|---|---|
53 | ||
llvm/test/MC/Xtensa/elf-header.s | ||
5 ↗ | (On Diff #455444) | Not done. |
llvm/test/MC/Xtensa/xtensa-invalid.s | ||
1 ↗ | (On Diff #484120) | llvm-mc doesn't need < delete excess spaces |
9 ↗ | (On Diff #484120) | Add locations for all diagnostics ( [[#@LINE+1]]) |
19 ↗ | (On Diff #484120) | [[#LINE]] is legacy syntax. Use [[#@LINE]]. Fix this everywhere. |
llvm/lib/Target/Xtensa/Xtensa.td | ||
---|---|---|
56 | I think this bit does not affect anything. |
The xtensa- prefix in test filenames like llvm/test/MC/Xtensa/xtensa-arith.s is unnecessary.
You may look at M68k and LoongArch for how ISA tests are organized in subdirectories. I think the two newer ports have more organized hierarchy.
llvm/lib/Target/Xtensa/Xtensa.td | ||
---|---|---|
56 | Fixed |