Skip to content

Conversation

@taiki-e
Copy link
Member

@taiki-e taiki-e commented Sep 22, 2025

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:

    • addsubiw
    • break
    • eijmpcall
    • elpm
    • elpmx
    • ijmpcall
    • jmpcall
    • lpm
    • lpmx
    • movw
    • mul
    • rmw
    • spm
    • spmx

    Of these, all except addsubiw, break, ijmpcall, lpm, rmw, spm, and spmx have corresponding conditional codes in avr-libc. LLVM also has des feature, 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, and memmappedregs features, 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

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Sep 22, 2025
@rustbot
Copy link
Collaborator

rustbot commented Sep 22, 2025

workingjubilee is currently at their maximum review capacity.
They may take a while to respond.

@rustbot rustbot added A-target-feature Area: Enabling/disabling target features like AVX, Neon, etc. O-AVR Target: AVR processors (ATtiny, ATmega, etc.) labels Sep 22, 2025
@taiki-e
Copy link
Member Author

taiki-e commented Sep 23, 2025

As for sram, this is the minimum requirement for C/C++ in both avr-gcc and clang (llvm/llvm-project@4bf6e83), and likely for Rust as well, so it is probably better to handle this not as a target feature here, but another way (such as rejecting compilation if it is not enabled).

static AVR_FEATURES: &[(&str, Stability, ImpliedFeatures)] = &[
// tidy-alphabetical-start
("addsubiw", Unstable(sym::avr_target_feature), &[]),
("break", Unstable(sym::avr_target_feature), &[]),
Copy link
Contributor

@Patryk27 Patryk27 Sep 23, 2025

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.

Copy link
Member Author

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.)

@taiki-e
Copy link
Member Author

taiki-e commented Sep 24, 2025

As for sram, this is the minimum requirement for C/C++ in both avr-gcc and clang (llvm/llvm-project@4bf6e83), and likely for Rust as well, so it is probably better to handle this not as a target feature here, but another way (such as rejecting compilation if it is not enabled).

Looking at the LLVM issue, in Rust, I guess it might work if we use no_main and put all processing inside global_asm and/or external assemblies...

@workingjubilee
Copy link
Member

workingjubilee commented Sep 27, 2025

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 -Ctarget-feature=-sram to the compiler. It is a rare case of "Here's a nickel, kid, go buy yourself a real computer" being an actually literally valid suggestion, even with 2025's inflation. The newest "AVR1" chips are so old that even products with 25-year lifecycles should have killed them off by now.

@bors
Copy link
Collaborator

bors commented Nov 5, 2025

☔ The latest upstream changes (presumably #147645) made this pull request unmergeable. Please resolve the merge conflicts.

@workingjubilee
Copy link
Member

This is waiting on "-sram should error", I think?

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 15, 2025
@taiki-e taiki-e force-pushed the avr-target-feature branch from 18d8866 to 7763f18 Compare January 9, 2026 01:43
@rustbot
Copy link
Collaborator

rustbot commented Jan 9, 2026

Some changes occurred in src/doc/rustc/src/platform-support

cc @Noratrieb

@rustbot

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@taiki-e taiki-e force-pushed the avr-target-feature branch from 7763f18 to e3f2d28 Compare January 9, 2026 01:51
@rust-log-analyzer

This comment has been minimized.

@taiki-e taiki-e force-pushed the avr-target-feature branch 2 times, most recently from 0d13763 to 75de66b Compare January 9, 2026 13:38
@taiki-e
Copy link
Member Author

taiki-e commented Jan 9, 2026

Changed sram to Forbidden and added it to abi_required_features, to report future-incompatible warning (#116344) for -C target-feature=-sram and -C target-cpu=<device_without_sram> cases.

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 9, 2026
@rust-bors
Copy link
Contributor

rust-bors bot commented Jan 15, 2026

☔ The latest upstream changes (presumably #151144) made this pull request unmergeable. Please resolve the merge conflicts.

@taiki-e taiki-e force-pushed the avr-target-feature branch from 75de66b to e940165 Compare January 15, 2026 02:06
@rustbot
Copy link
Collaborator

rustbot commented Jan 15, 2026

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.

@taiki-e taiki-e force-pushed the avr-target-feature branch from e940165 to febe008 Compare January 15, 2026 02:16
@rust-log-analyzer

This comment has been minimized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-target-feature Area: Enabling/disabling target features like AVX, Neon, etc. O-AVR Target: AVR processors (ATtiny, ATmega, etc.) S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants