-
Notifications
You must be signed in to change notification settings - Fork 12.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add no_std Xtensa targets support #125141
base: master
Are you sure you want to change the base?
Conversation
r? @davidtwco rustbot has assigned @davidtwco. Use |
These commits modify compiler targets. Some changes occurred in src/doc/rustc/src/platform-support cc @Nilstrieb |
This comment has been minimized.
This comment has been minimized.
33611af
to
e06858f
Compare
This comment has been minimized.
This comment has been minimized.
e06858f
to
bfaeb9d
Compare
This comment has been minimized.
This comment has been minimized.
bfaeb9d
to
e9eeb0d
Compare
This comment has been minimized.
This comment has been minimized.
e9eeb0d
to
41071ae
Compare
This comment has been minimized.
This comment has been minimized.
By the way, you can try to run the $ DEPLOY=1 ENABLE_GCC_CODEGEN=1 src/ci/docker/run.sh x86_64-gnu-llvm-17 |
I think that you should just run |
41071ae
to
4b69f37
Compare
Some changes occurred in tests/ui/check-cfg cc @Urgau |
This comment has been minimized.
This comment has been minimized.
4b69f37
to
21e924e
Compare
This comment has been minimized.
This comment has been minimized.
21e924e
to
2f7dc2a
Compare
This comment has been minimized.
This comment has been minimized.
Hmm. It looks like it's a problem that rustc cannot even create a The existing @davidtwco might know more. |
tier: None, | ||
host_tools: None, | ||
std: None, | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you complete the metadata for each of these targets? It's only Option
for the targets we've not populated these for yet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi! I think I just added it, but CI is still failing :/
2f7dc2a
to
fcd81c1
Compare
This comment has been minimized.
This comment has been minimized.
Yes, I really cant understand why it does complain about Xtensa, it should be the same as |
(just to clarify) It is not the same, because CSky is actually in mainline LLVM, but xtensa is not (if I understand it correctly). So I would definitely expect Xtensa to fail on CI, I'm just not sure why CSky is also in the error log. |
Xtensa is partially upstream, see: https://github.com/llvm/llvm-project/tree/main/llvm/lib/Target/Xtensa, It's just not complete enough to build useful things with it yet hence why we still need our fork. Maybe we could try enabling Xtensa as an experimental target in the LLVM build and see if it helps? Not sure if that's something we can do here in this PR or if it needs some kind of MCP. |
I think it's okay to do this, I don't expect it would impact any other target. |
Adding xtensa (whatever the name of the target is in LLVM) here should hopefully do the trick. |
851e60d
to
ad82128
Compare
This comment has been minimized.
This comment has been minimized.
FWIW, in my understanding we require one of the actual codegen backends shipped with rustc to be able to generate code. Having patches in a fork is not sufficient. |
Hmm, CI failed again. With the LLVM component change, we do add the Xtensa experimental target, but this specific CI job just uses vanilla LLVM 17 (which we don't build), which probably doesn't enable the target by default. I still don't understand why the test was green before when apparently the same happens for Csky (unless it is enabled in vanilla LLVM 17), it needs investigation. |
The CSky component apparently exists on some runners but not on others. That may explain the strange behavior. |
That makes sense, but I don't understand why wasn't CI failing before. |
ad82128
to
c142b67
Compare
c142b67
to
e0dc9ec
Compare
This comment has been minimized.
This comment has been minimized.
e0dc9ec
to
5b7a886
Compare
5b7a886
to
11f70d7
Compare
CI is green 🎉
Upstream LLVM has landed enough to generate some code/asm for Xtensa targets, however, the LLVM used by Rust currently doesn't have these patches in it yet. Would we still be okay to merge given that it's already landed in LLVM, or would we have to wait? If we have to wait, do you know if there is a timescale for LLVM updates? (I tried looking in the rustc dev guide and didn't see anything there). |
This PR changes how LLVM is built. Consider updating src/bootstrap/download-ci-llvm-stamp. |
We update to major versions (so next update will be LLVM 19), and we support the two latest major versions. But as long as we can get at least basic support working (xtensa already is an experimental target, after all) if user brings their own LLVM, and CI is green, I don't think that we need to wait. |
Adds no_std Xtensa targets. This enables using Rust on ESP32, ESP32-S2 and ESP32-S3 chips.
Tier 3 policy:
@MabezDev and I (@SergioGasquez) will maintain the targets.
The target triple is consistent with other targets.
We follow the same naming convention as other targets.
The target does not introduce any legal issues.
There are no license incompatibilities
Everything added is under that licenses
Requirements are not changed for any other target.
The linker used by the targets is the GCC linker from the GCC toolchain cross-compiled for Xtensa. GNU GPL.
No such terms exist for this target
Understood
The target already implements core.
Here is how to build for the target https://docs.esp-rs.org/book/installation/riscv-and-xtensa.html and it also covers how to run binaries on the target.
Understood
No other targets should be affected
It can produce assembly, but it requires a custom LLVM with Xtensa support (https://github.com/espressif/llvm-project/). The patches are trying to be upstreamed (espressif/llvm-project#4)