This is an archive of the discontinued LLVM Phabricator instance.

[Xtensa 7/10] Add Xtensa instruction printer.
ClosedPublic

Authored by andreisfr on Jul 16 2019, 3:27 PM.

Details

Summary

Add printer for current instructions and operands subsets.
Also add basic tests of the Xtensa instructions.

Diff Detail

Event Timeline

andreisfr created this revision.Jul 16 2019, 3:27 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 16 2019, 3:27 PM
andreisfr updated this revision to Diff 212691.Jul 31 2019, 4:00 PM

Register names are capitalized.

andreisfr updated this revision to Diff 242217.Feb 3 2020, 3:18 PM

Patch is updated according to latest upstream version. Updated licenses.

andreisfr updated this revision to Diff 328699.Mar 5 2021, 4:46 PM

Patch is updated according to latest upstream version

andreisfr updated this revision to Diff 329499.Mar 9 2021, 4:51 PM

Patch is updated according to LLVM upstream version and latest Xtensa backend version.

andreisfr updated this revision to Diff 335956.Apr 7 2021, 4:55 PM

The llvm_unreachable is substituted to report_fatal_error

andreisfr updated this revision to Diff 351603.Jun 11 2021, 5:40 PM

Remove redundant whitespaces.

Patch is updated according to LLVM upstream version.

bero added a subscriber: bero.Apr 14 2022, 2:00 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 14 2022, 2:00 PM
andreisfr updated this revision to Diff 425076.Apr 25 2022, 5:17 PM

Patch is updated according to LLVM upstream version

saugustine added inline comments.
llvm/test/MC/Xtensa/xtensa-invalid.s
14

It would be good to check instruction arguments out of order, and invalid register names.

llvm/test/MC/Xtensa/xtensa-valid.s
5

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.

62

Spaces after the commas in this file is inconsistent.

andreisfr updated this revision to Diff 455444.Aug 24 2022, 6:00 PM

Minor changes: corrected comment

andreisfr added inline comments.Aug 24 2022, 6:19 PM
llvm/test/MC/Xtensa/xtensa-invalid.s
14

You mean for each instruction implement extended tests with argument out of order and invalid register names?

saugustine added inline comments.Aug 30 2022, 1:47 PM
llvm/test/MC/Xtensa/xtensa-invalid.s
14

Probably just once for each format, but yes.

phosek added a subscriber: phosek.Sep 1 2022, 11:44 AM
tpambor added a subscriber: tpambor.Sep 4 2022, 3:08 AM
arsenm added a subscriber: arsenm.Sep 12 2022, 6:42 AM
arsenm added inline comments.
llvm/lib/Target/Xtensa/MCTargetDesc/XtensaInstPrinter.cpp
34–35

Just use cast<>

MaskRay added inline comments.
llvm/test/MC/Xtensa/elf-header.s
6

Add -NEXT when applicable.

llvm/test/MC/Xtensa/xtensa-valid.s
2

Split this into multiple files.

MC/AArch64/armv8* and MC/LoongArch/Basic can give some insights how tests are organized.

MaskRay added inline comments.Sep 12 2022, 10:24 PM
llvm/lib/Target/Xtensa/MCTargetDesc/XtensaInstPrinter.cpp
90

isInt<8> from MathExtras.h

94

Add braces. then uses braces and else should use, too.

101

isInt<16> from MathExtras.h

andreisfr updated this revision to Diff 461114.Sep 18 2022, 6:20 PM

Minor code corrections. Test file xtensa-valid.s is splitted to several files by instructions groups.

andreisfr updated this revision to Diff 461118.Sep 18 2022, 6:35 PM

Remove xtensa-valid.s

andreisfr added inline comments.Sep 18 2022, 6:37 PM
llvm/lib/Target/Xtensa/MCTargetDesc/XtensaInstPrinter.cpp
34–35

Corrected

90

Corrected

94

Corrected

101

Corrected

llvm/test/MC/Xtensa/elf-header.s
6

Corrected

llvm/test/MC/Xtensa/xtensa-invalid.s
14

I added more tests for different instruction formats.

llvm/test/MC/Xtensa/xtensa-valid.s
2

Thank you for advice, I've splitted file to several files by commands groups.

5

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?

JKRhb added a subscriber: JKRhb.Oct 9 2022, 2:30 PM
arsenm accepted this revision.Dec 13 2022, 8:59 AM

LGTM with nit

llvm/lib/Target/Xtensa/MCTargetDesc/XtensaInstPrinter.cpp
35

Also means you can get rid of the assert(false) and if

This revision is now accepted and ready to land.Dec 13 2022, 8:59 AM
saugustine accepted this revision.Dec 13 2022, 10:40 AM
jrtc27 added a subscriber: jrtc27.Dec 13 2022, 12:06 PM
jrtc27 added inline comments.
llvm/test/MC/Xtensa/elf-header.s
3

Why not just use CHECK? And please keep these uppercase so they stand out.

andreisfr updated this revision to Diff 484120.Dec 19 2022, 3:41 PM

Fixed "elf-header.s" test according to comments

andreisfr marked an inline comment as done.Dec 19 2022, 3:42 PM
andreisfr added inline comments.
llvm/test/MC/Xtensa/elf-header.s
3

Thank you for comment, test is fixed.

MaskRay accepted this revision.Dec 19 2022, 3:57 PM
MaskRay added inline comments.
llvm/lib/Target/Xtensa/MCTargetDesc/XtensaInstPrinter.h
53
llvm/test/MC/Xtensa/elf-header.s
6

Not done.

llvm/test/MC/Xtensa/xtensa-invalid.s
2

llvm-mc doesn't need <

delete excess spaces

10

Add locations for all diagnostics ( [[#@LINE+1]])

20

[[#LINE]] is legacy syntax. Use [[#@LINE]]. Fix this everywhere.

barannikov88 accepted this revision.Dec 20 2022, 2:31 AM
barannikov88 added a subscriber: barannikov88.
barannikov88 added inline comments.
llvm/lib/Target/Xtensa/Xtensa.td
56

I think this bit does not affect anything.

MaskRay added a comment.EditedDec 20 2022, 1:14 PM

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.

andreisfr updated this revision to Diff 485252.Dec 25 2022, 4:32 PM
andreisfr marked an inline comment as done.

Added fixes according to comments

andreisfr added inline comments.Dec 25 2022, 4:33 PM
llvm/lib/Target/Xtensa/Xtensa.td
56

Fixed

@MaskRay , I changed Xtensa tests structure according to your suggestions, PTAL.

This revision was landed with ongoing or failed builds.Dec 26 2022, 4:39 AM
This revision was automatically updated to reflect the committed changes.