-
-
Notifications
You must be signed in to change notification settings - Fork 14.9k
Add avr_target_feature #146900
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 avr_target_feature #146900
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -898,6 +898,28 @@ static M68K_FEATURES: &[(&str, Stability, ImpliedFeatures)] = &[ | |
| // tidy-alphabetical-end | ||
| ]; | ||
|
|
||
| static AVR_FEATURES: &[(&str, Stability, ImpliedFeatures)] = &[ | ||
| // tidy-alphabetical-start | ||
| ("addsubiw", Unstable(sym::avr_target_feature), &[]), | ||
| ("break", Unstable(sym::avr_target_feature), &[]), | ||
| ("eijmpcall", Unstable(sym::avr_target_feature), &[]), | ||
| ("elpm", Unstable(sym::avr_target_feature), &[]), | ||
| ("elpmx", Unstable(sym::avr_target_feature), &[]), | ||
| ("ijmpcall", Unstable(sym::avr_target_feature), &[]), | ||
| ("jmpcall", Unstable(sym::avr_target_feature), &[]), | ||
| ("lowbytefirst", Unstable(sym::avr_target_feature), &[]), | ||
| ("lpm", Unstable(sym::avr_target_feature), &[]), | ||
| ("lpmx", Unstable(sym::avr_target_feature), &[]), | ||
| ("movw", Unstable(sym::avr_target_feature), &[]), | ||
| ("mul", Unstable(sym::avr_target_feature), &[]), | ||
| ("rmw", Unstable(sym::avr_target_feature), &[]), | ||
| ("spm", Unstable(sym::avr_target_feature), &[]), | ||
| ("spmx", Unstable(sym::avr_target_feature), &[]), | ||
| ("sram", Forbidden { reason: "devices that have no SRAM are unsupported" }, &[]), | ||
| ("tinyencoding", Unstable(sym::avr_target_feature), &[]), | ||
| // tidy-alphabetical-end | ||
| ]; | ||
|
|
||
| /// When rustdoc is running, provide a list of all known features so that all their respective | ||
| /// primitives may be documented. | ||
| /// | ||
|
|
@@ -919,6 +941,7 @@ pub fn all_rust_features() -> impl Iterator<Item = (&'static str, Stability)> { | |
| .chain(IBMZ_FEATURES) | ||
| .chain(SPARC_FEATURES) | ||
| .chain(M68K_FEATURES) | ||
| .chain(AVR_FEATURES) | ||
| .cloned() | ||
| .map(|(f, s, _)| (f, s)) | ||
| } | ||
|
|
@@ -996,12 +1019,8 @@ impl Target { | |
| Arch::S390x => IBMZ_FEATURES, | ||
| Arch::Sparc | Arch::Sparc64 => SPARC_FEATURES, | ||
| Arch::M68k => M68K_FEATURES, | ||
| Arch::AmdGpu | ||
| | Arch::Avr | ||
| | Arch::Msp430 | ||
| | Arch::SpirV | ||
| | Arch::Xtensa | ||
| | Arch::Other(_) => &[], | ||
| Arch::Avr => AVR_FEATURES, | ||
| Arch::AmdGpu | Arch::Msp430 | Arch::SpirV | Arch::Xtensa | Arch::Other(_) => &[], | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -1023,11 +1042,11 @@ impl Target { | |
| MIPS_FEATURES_FOR_CORRECT_FIXED_LENGTH_VECTOR_ABI | ||
| } | ||
| Arch::AmdGpu => AMDGPU_FEATURES_FOR_CORRECT_FIXED_LENGTH_VECTOR_ABI, | ||
| Arch::Nvptx64 | Arch::Bpf | Arch::M68k => &[], // no vector ABI | ||
| Arch::Nvptx64 | Arch::Bpf | Arch::M68k | Arch::Avr => &[], // no vector ABI | ||
| Arch::CSky => CSKY_FEATURES_FOR_CORRECT_FIXED_LENGTH_VECTOR_ABI, | ||
| // FIXME: for some tier3 targets, we are overly cautious and always give warnings | ||
| // when passing args in vector registers. | ||
| Arch::Avr | Arch::Msp430 | Arch::SpirV | Arch::Xtensa | Arch::Other(_) => &[], | ||
| Arch::Msp430 | Arch::SpirV | Arch::Xtensa | Arch::Other(_) => &[], | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -1224,6 +1243,12 @@ impl Target { | |
| // because the vector and float registers overlap. | ||
| FeatureConstraints { required: &[], incompatible: &["soft-float"] } | ||
| } | ||
| Arch::Avr => { | ||
| // SRAM is minimum requirement for C/C++ in both avr-gcc and Clang, | ||
| // and backends of them only support assembly for devices have no SRAM. | ||
| // See the discussion in https://github.com/rust-lang/rust/pull/146900 for more. | ||
| FeatureConstraints { required: &["sram"], incompatible: &[] } | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FWIW this function is specifically meant for target features that affect the ABI. This one here seems to be motivated by different concerns. Maybe that's okay for now but we should have an eye on this.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In particular, it is strange yo have target feature constraints here, but not have any AVR-specific ABI flag checks in |
||
| } | ||
| _ => NOTHING, | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| warning: target feature `sram` cannot be disabled with `-Ctarget-feature`: devices that have no SRAM are unsupported | ||
| | | ||
| = note: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! | ||
| = note: for more information, see issue #116344 <https://github.com/rust-lang/rust/issues/116344> | ||
|
|
||
| warning: target feature `sram` must be enabled to ensure that the ABI of the current target can be implemented correctly | ||
| | | ||
| = note: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! | ||
| = note: for more information, see issue #116344 <https://github.com/rust-lang/rust/issues/116344> | ||
|
|
||
| warning: 2 warnings emitted | ||
|
|
||
|
Comment on lines
+1
to
+12
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should probably proactively become a hard error before stabilizing. |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| warning: target feature `sram` must be enabled to ensure that the ABI of the current target can be implemented correctly | ||
| | | ||
| = note: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! | ||
| = note: for more information, see issue #116344 <https://github.com/rust-lang/rust/issues/116344> | ||
|
|
||
| warning: 1 warning emitted | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| //@ revisions: has_sram no_sram disable_sram | ||
| //@ build-pass | ||
| //@[has_sram] compile-flags: --target avr-none -C target-cpu=atmega328p | ||
| //@[has_sram] needs-llvm-components: avr | ||
| //@[no_sram] compile-flags: --target avr-none -C target-cpu=attiny11 | ||
| //@[no_sram] needs-llvm-components: avr | ||
| //@[disable_sram] compile-flags: --target avr-none -C target-cpu=atmega328p -C target-feature=-sram | ||
| //@[disable_sram] needs-llvm-components: avr | ||
| //@ ignore-backends: gcc | ||
| //[no_sram,disable_sram]~? WARN target feature `sram` must be enabled | ||
| //[disable_sram]~? WARN target feature `sram` cannot be disabled with `-Ctarget-feature` | ||
|
|
||
| #![feature(no_core)] | ||
| #![no_core] | ||
| #![crate_type = "lib"] |
Uh oh!
There was an error while loading. Please reload this page.
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.
nit: what about name collisions? 👀
The RFC doesn't seem to mention this, but I can see AVR's
break(or other "generic" word likemul) accidentally colliding with another arch in future - on the other hand, keeping the mapping 1:1 with LLVM (FeatureBREAKin this case) is important as well, IMO.Other than that LGTM.
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.
Target feature name collisions with other architectures are fine. (e.g., the
aestarget feature exists on x86/x86_64 (AES-NI) and arm/aarch64/arm64ec (FEAT_AES) respectively.)