-
-
Notifications
You must be signed in to change notification settings - Fork 14.3k
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
base: main
Are you sure you want to change the base?
Add avr_target_feature #146900
Conversation
|
|
|
As for |
| static AVR_FEATURES: &[(&str, Stability, ImpliedFeatures)] = &[ | ||
| // tidy-alphabetical-start | ||
| ("addsubiw", Unstable(sym::avr_target_feature), &[]), | ||
| ("break", Unstable(sym::avr_target_feature), &[]), |
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 like mul) accidentally colliding with another arch in future - on the other hand, keeping the mapping 1:1 with LLVM (FeatureBREAK in 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 aes target feature exists on x86/x86_64 (AES-NI) and arm/aarch64/arm64ec (FEAT_AES) respectively.)
Looking at the LLVM issue, in Rust, I guess it might work if we use |
|
cc @rust-lang/lang informational-only: letting you know about this target's features because it is particularly interesting in terms of it being on the boundary of what Rust can support. I think that we should indeed either ignore it or just error if someone tries to say |
|
☔ The latest upstream changes (presumably #147645) made this pull request unmergeable. Please resolve the merge conflicts. |
|
This is waiting on " @rustbot author |
18d8866 to
7763f18
Compare
|
Some changes occurred in src/doc/rustc/src/platform-support cc @Noratrieb |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
7763f18 to
e3f2d28
Compare
This comment has been minimized.
This comment has been minimized.
0d13763 to
75de66b
Compare
|
☔ The latest upstream changes (presumably #151144) made this pull request unmergeable. Please resolve the merge conflicts. |
75de66b to
e940165
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
e940165 to
febe008
Compare
This adds the following unstable target features (tracking issue: #146889):
The following two are particularly important for properly supporting inline assembly:
tinyencoding: AVR has devices that reduce the number of registers, similar to RISC-V's RV32E. This feature is necessary to support inline assembly in such devices. (see also Support AVRTiny devices in AVR inline assembly #146901)lowbytefirst: AVR's memory access is per 8-bit, and when writing 16-bit ports, the bytes must be written in a specific order. This order depends on devices, making this feature necessary to write proper inline assembly for such use cases. (see also llvm/llvm-project@2a52876)The followings help recognizing whether specific instructions are available:
addsubiwbreakeijmpcallelpmelpmxijmpcalljmpcalllpmlpmxmovwmulrmwspmspmxOf these, all except
addsubiw,break,ijmpcall,lpm,rmw,spm, andspmxhave corresponding conditional codes in avr-libc. LLVM also hasdesfeature, but I excluded it from this PR because DES is insecure.Report future-incompatible warning (The ABI of float types can be changed by
-Ctarget-feature#116344) for -C target-feature=-sram and -C target-cpu=<device_without_sram> cases because SRAM is minimum requirement for non-assembly language in both avr-gcc and LLVM.LLVM also has
smallstack,wrappingrjmp, andmemmappedregsfeatures, but I skipped them because they didn't seem to belong to either of the above categories, but I might have missed something.(The feature names are match with definitions in LLVM.)
cc @Patryk27 @Rahix
r? workingjubilee
@rustbot label +O-AVR +A-target-feature