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 | |