Page MenuHomePhabricator

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

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

Details

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.

Diff Detail

Event Timeline

andreisfr created this revision.Jul 16 2019, 3:21 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 16 2019, 3:21 PM
arsenm added a subscriber: arsenm.Jul 16 2019, 3:24 PM
arsenm added inline comments.
llvm/lib/Target/Xtensa/XtensaRegisterInfo.td
41–46

Usually the register names and corresponding generated enums are capitalized

appcs added a reviewer: appcs.Jul 16 2019, 4:03 PM

Where is the Xtensa instruction set architecture (ISA) document on which this patch is based on?

andreisfr updated this revision to Diff 212684.Jul 31 2019, 3:51 PM

Register names are capitalized.

As for Xtensa instruction set architecture (ISA) document , currently public ISA description mainly available in opensource projects like gcc/qemu/binutils:
https://github.com/gcc-mirror/gcc/tree/master/gcc/config/xtensa
https://github.com/qemu/qemu/blob/master/target/xtensa/translate.c
https://github.com/bminor/binutils-gdb/blob/master/bfd/xtensa-modules.c

I don’t think I saw an RFC for this target? Was there one I missed?

As for Xtensa instruction set architecture (ISA) document , currently public ISA description mainly available in opensource projects like gcc/qemu/binutils:
https://github.com/gcc-mirror/gcc/tree/master/gcc/config/xtensa
https://github.com/qemu/qemu/blob/master/target/xtensa/translate.c
https://github.com/bminor/binutils-gdb/blob/master/bfd/xtensa-modules.c

So the only documentation is the existing implementations?
To me personally that does not sound like there is documentation..

Doing a websearch for "xtensa isa", I do see that documentation does seem to exist: https://github.com/eerimoq/hardware-reference/blob/master/esp32/xtensa%20Instruction%20Set%20Architecture%20(ISA)%20Reference%20Manual.pdf.

It would sure be nice if it was hosted in a more official location, though.

I do not think there is a reason why the Xtensa architecture shouldn't be included in LLVM. As a comparison, there is also the proprietary (Google) Lanai architecture which was added at a time when there was little public information on it and to this time nobody can actually buy Lanai hardware (there is only a simulator). As a comparison, Xtensa has easy to get hardware (in the form of the ESP8266/ESP32) and is already supported in GCC/binutils.

Looking back at the email thread, there were very few responses. I wonder what should be done next?
Tagging Chris just in case: @lattner

I do not think there is a reason why the Xtensa architecture shouldn't be included in LLVM. As a comparison, there is also the proprietary (Google) Lanai architecture which was added at a time when there was little public information on it and to this time nobody can actually buy Lanai hardware (there is only a simulator). As a comparison, Xtensa has easy to get hardware (in the form of the ESP8266/ESP32) and is already supported in GCC/binutils.

I'd note that Lanai did publish a set of ISA docs as part of upstreaming the llvm backend.

Having something like that perhaps doesn't need to be a hard-blocker, but I'd love to see a real response from the xtensa folks, since AFAICT it looks as if like there *IS* ISA documentation, just perhaps not -- officially -- publicly available. It would make things much easier to understand if xtensa published what they had. Without that, nobody else can really review or understand the backend.

I'd note that Lanai did publish a set of ISA docs as part of upstreaming the llvm backend.

Having something like that perhaps doesn't need to be a hard-blocker, but I'd love to see a real response from the xtensa folks, since AFAICT it looks as if like there *IS* ISA documentation, just perhaps not -- officially -- publicly available. It would make things much easier to understand if xtensa published what they had. Without that, nobody else can really review or understand the backend.

@jyknight , as I'm understood, Cadence doesn;t want to publish Xtensa ISA. But it seems that documentation from the link that you provided is quite actual, probably it was published when Xtensa was owned by Tensilica. Also, the code can be reviewed by people from companies that use Xtensa IP in their chips.

BTW there is also an ARC backend (from Synopsys), and I have not found any public documentation with ARC ISA, so probably encoding of ARC instructions also could be verified only by Synopsys engineers.

Also we could prepare summary about Xtensa ISA based on public resources like GCC, Binutils, QEMU with quality enough for code review. What do you think?

@jyknight , as I'm understood, Cadence doesn;t want to publish Xtensa ISA. But it seems that documentation from the link that you provided is quite actual, probably it was published when Xtensa was owned by Tensilica. Also, the code can be reviewed by people from companies that use Xtensa IP in their chips.

That seems really silly -- why wouldn't someone want all users of their CPUs to be able to easily access the ISA docs. Oh well.

Anyways, are there some folks who know LLVM and do have access to the official documentation who can help review the patches? (Maybe asking on the mailing list thread would be useful.)

BTW there is also an ARC backend (from Synopsys), and I have not found any public documentation with ARC ISA, so probably encoding of ARC instructions also could be verified only by Synopsys engineers.

Also we could prepare summary about Xtensa ISA based on public resources like GCC, Binutils, QEMU with quality enough for code review. What do you think?

I think a short summary of the general characteristics of the CPU/ISA would be useful. I do not think that it would be a productive use of time to attempt to reverse-engineer/rewrite an entire ISA doc -- having such a reverse-engineered and possibly incorrect or outdated doc might even be worse than not having it. But I do not think lack of public documentation should be a blocker for inclusion, especially if there are folks who do have access to the privately-available documentation who can help review.

@andreisfr @jyknight @ivanbaev Ping? Are there any news concerning these pull requests?

@andreisfr @jyknight @ivanbaev Ping? Are there any news concerning these pull requests?

From my point of view, this is waiting for the submitter to follow up (or someone may take over).

andreisfr added a comment.EditedJan 17 2020, 4:07 AM

@codehippo, @jyknight, @ivanbaev , we still active develop and improve Xtensa backend, for example this is recent version https://github.com/espressif/llvm-project/tree/xtensa_release_9.x currently we maintain mainstream version and we plan to publish one based on release 10. Current review is paused on *.td files with Xtensa ISA description after the question about ISA documentation, we have been tried to find a way how to help reviewers to aprove this code. We interested to integrate Xtensa backend to upstream and make it available to LLVM community, so we are going to prepare Xtensa ISA description based on opensource projects like binutils, gcc etc. It seems to be the only way to continue review process. What do you think?

andreisfr updated this revision to Diff 242205.Feb 3 2020, 3:14 PM

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

@jyknight, @ivanbaev , currently is available Xtensa backend based on latest 9.0.1 release https://github.com/espressif/llvm-project/tree/xtensa_release_9.0.1

@appcs Do you perhaps have access to the ISA documentation so you can review these patches?

andreisfr marked an inline comment as done.Feb 9 2020, 4:54 PM