[Xtensa 4/10] Add basic *td files with Xtensa architecture description.

[andreisfr]

Summary

Add initial Xtensa.td file with target machine description. Add XtensaInstrInfo.td,
currently describe just susbet of Core Instructions like ALU, Processor control,
memory barrier and some move instructions. Add descriptions of the instructions
formats(XtensaInstrInfo.td) and some immediate instruction operands(XtensaOperands.td).
Add General Registers and Special Registers classes.

[andreisfr]

@brainstorm , @tonic , @tstellar , thank you very much for your help!

I support and improve Xtensa backend now, current version of the backend is rebased on release 11.0.0 https://github.com/espressif/llvm-project/tree/xtensa_release_11.0.0 .

@tstellar , so I can just update these 10 patches according to latest changes in LLVM, add myself as code owner in the first patch and send patches again to list (I will recheck outstanding issues)?

[tstellar]

In D64830#2518675, @andreisfr wrote:

@brainstorm , @tonic , @tstellar , thank you very much for your help!

I support and improve Xtensa backend now, current version of the backend is rebased on release 11.0.0 https://github.com/espressif/llvm-project/tree/xtensa_release_11.0.0 .

@tstellar , so I can just update these 10 patches according to latest changes in LLVM, add myself as code owner in the first patch and send patches again to list (I will recheck outstanding issues)?

@andreisfr Yes, that sounds good.

[aykevl]

@andreisfr any update on this?

It would be great if you could update these patches and send a new RFC to llvm-dev, like you did here: https://lists.llvm.org/pipermail/llvm-dev/2019-March/130796.html

[aykevl]

Also, there is an emulator here: https://github.com/espressif/qemu/wiki
I have successfully used it in the past. Noting it here as it can make backend review/development easier for those who don't have the hardware.

[andreisfr]

@aykevl , I prepared new version of patches and I will update them in 1 day.

[andreisfr]

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

[aykevl]

@andreisfr awesome! Thanks a lot!

Can you please also write a new RFC and send it to llvm-dev like @tstellar proposed? An RFC is the next step in getting this backend accepted.

In D64830#2514379, @tstellar wrote:

If you think you meet those requirements, please re-send the RFC to the list, so we can discuss getting this added.

[craig.topper]

llvm/lib/Target/Xtensa/Xtensa.td

2

Please limit line length to 80 characters

llvm/lib/Target/Xtensa/XtensaInstrFormats.td

2

Please limit line length to 80 characters

15

I think the prevailing style in LLVM has the open curly brace at the end of the previous line.

32

It might just be me, but I find this lining the XtensaInst up with template parameters hard to read. I think a common style is to put at the start of the line with only a small indentation. For example

class PseudoI<dag oops, dag iops, list<dag> pattern>                                                                                                                                                                                                                                   
  : X86Inst<0, Pseudo, NoImm, oops, iops, ""> {                                                                                                                                                                                                                                        
  let Pattern = pattern;                                                                                                                                                                                                                                                               
}

llvm/lib/Target/Xtensa/XtensaOperands.td

2

This line seems shorter than 80 characters.

[andreisfr]

Correct instruction descriptions, format descriptions and instruction operands according to common style for *.td files.

[andreisfr]

llvm/lib/Target/Xtensa/Xtensa.td

2

Corrected

llvm/lib/Target/Xtensa/XtensaInstrFormats.td

2

Corrected

15

Corrected for instruction descriptions, format descriptions and instruction operands.

32

@craig.topper , I corrected instruction formats and instruction descriptions, do you think that this code now in common style or it must be corrected bit more?

llvm/lib/Target/Xtensa/XtensaOperands.td

2

Corrected

[andreisfr]

@craig.topper , thank you very much for comments!

[craig.topper]

llvm/lib/Target/Xtensa/XtensaInstrFormats.td

32

I think this looks better. Unfortunately, we don't have a formatting tool for tablegen and no rules are written down. I think this is good enough.

167

Drop the second blank line?

llvm/lib/Target/Xtensa/XtensaRegisterInfo.td

31

This is overindented relative to the style of the rest of the file.

[andreisfr]

Corrected indentation, removed blank line

llvm/lib/Target/Xtensa/XtensaInstrFormats.td

167

Corrected

llvm/lib/Target/Xtensa/XtensaRegisterInfo.td

31

Corrected

[kairoswater-jason]

Who is needed to review and approve this patch?

[andreisfr]

Patch is updated according to LLVM upstream version.

[luojia]

What's the current state of this patch? :)

[andreisfr]

In D64830#3474080, @luojia wrote:

What's the current state of this patch? :)

HI @luojia, thank you for your interest to the Xtensa backend.

Currently, probably all suggestions in comments for this patch were implemented. If you can help with further reviewing or approving this patch that would be great.
Brief status of the whole series of the Xtensa backend patches :

1. All 10 patches are updated according to the latest upstream changes . Patches 1-3 are approved.
2. We have submitted an RFC to the mailing list https://lists.llvm.org/pipermail/llvm-dev/2021-March/149090.html .
3. We have written the ISA documentation https://github.com/espressif/xtensa-isa-doc .
4. Also we have a branch on Github with patches rebased to llvm-project main branch https://github.com/espressif/llvm-project/tree/main_xtensa .

[saugustine]

I have access to an up-to-date copy of the isa book and can review these for technical correctness. I'm less familiar with style issues.

I have checked the bitfields, encodings, and field names. They are all correct.

Some comments below, especially on the xSYNC instructions. Nothing too serious, but they do have side effects of one form or another. Typically a compiler wouldn't generate them unless as part of a larger hard-coded, unsplittable block, but if they are present, they ought to be correct.

llvm/lib/Target/Xtensa/XtensaInstrFormats.td

53

These fields are reverse order in big endian versions of the core. Is the ESP32 implementation of Xtensa always little endian?

It would be nice to handle both variants, but that isn't a block for this patch.

llvm/lib/Target/Xtensa/XtensaInstrInfo.td

102

This is quite a bit more conservative than necessary. MEMW only serializes memory operations, it is fine to move arithmetic ops across it.

109

This instruction is a superset of MEMW and serializes all externally visible operations.

hasSideEffects is necessary on this one (at least in a base ISA implementation).

116

DSYNC is the least intrusive processor-state sync instruction, used to serialize loads and stores against WSR and XSR instructions that may update the page table.

It could be modeled quite similarly to MEMW, but with a more complete memory-barrier model, it should include side effects.

123

ISYNC synchronizes the instruction fetch unit against updates to the icache. Needs hasSideEffects set.

130

This synchronizes reading register fields out of instructions against WSR and XSR modifying the register window. Needs hasSideEffects set.

137

This is quite similar to RSYNC, but synchronizes the exact read from the register window, rather than the fields out of the instruction. Needs hasSideEffects set.

[andreisfr]

Corrected MEMW, EXTW, DSYNC, ESYNC, ISYNC and RSYNC instruction descriptions

[andreisfr]

@saugustine , thank you very much for your comments!

llvm/lib/Target/Xtensa/XtensaInstrFormats.td

53

Yes, the ESP32 implementation is always little endian, and I didn't observed any public available big endian implementations. So, initial idea was to implement just little endian at first and maybe implement support of the big endian in future.

llvm/lib/Target/Xtensa/XtensaInstrInfo.td

102

Corrected

109

Corrected

116

Corrected

123

Corrected

130

Corrected

137

Corrected

[andreisfr]

@aykevl , @jyknight , @ivanbaev , I have a good news about Xtensa ISA documentation. Cadence makes documentation publicly available https://www.cadence.com/content/dam/cadence-www/global/en_US/documents/tools/ip/tensilica-ip/isa-summary.pdf

[jscott]

What is the current state of this patch? As the Debian maintainer of ath9k_htc, the flagship libre wireless firmware for USB adapters, I eagerly wait for this to be merged so that I may begin exploring using LLVM to build the firmware, which is currently a rather painful process with GCC.

[sstefan1]

Hi everyone. Since this has been accepted for some time now, we're planning to commit this on Monday (December 16th). Please let us know if there is anything else we should address.

[barannikov88]

llvm/lib/Target/Xtensa/XtensaInstrFormats.td

218

• IIRC SoftFail is no longer necessary after this patch (if you don't use this functionality, of course).
• 'field' seems redundant, too (hasn't this keyword been deprecated?).
• Do isCodeGenOnly instructions really need encoding bits?

[andreisfr]

Added fixes according to comments

[andreisfr]

llvm/lib/Target/Xtensa/XtensaInstrFormats.td

218

Thank you for comments, I removed redundant code.