Skip to content
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

Enable AVR as a Tier 3 target upstream #69478

Merged
merged 9 commits into from
Jun 12, 2020

Conversation

dylanmckay
Copy link
Contributor

@dylanmckay dylanmckay commented Feb 26, 2020

Tracking issue: #44052.

Things intentionally left out of the initial upstream:

  • The target_cpu flag

I have made the cleanup suggestions by @jplatte and @jplatte in avr-rust@043550d.

Anybody feel free to give the branch a test and see how it fares, or make suggestions on the code patch itself.

@rust-highfive
Copy link
Collaborator

r? @estebank

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 26, 2020
@dylanmckay dylanmckay closed this Feb 26, 2020
@dylanmckay
Copy link
Contributor Author

Dammit, meant to raise this on avr-rust/rust first, I always do this

@rust-highfive

This comment has been minimized.

@dylanmckay
Copy link
Contributor Author

Actually, come to think of it, might as well keep it here for tracking.

Code reviews welcome, but let's hold off merging until some more testing has been done,

@dylanmckay dylanmckay reopened this Feb 26, 2020
@rust-highfive

This comment has been minimized.

@dylanmckay dylanmckay changed the title Preliminary Pull Request: AVR support upstream WIP: Enable AVR as a Tier 3 target upstream Feb 26, 2020
@dylanmckay dylanmckay force-pushed the avr-support-upstream branch 2 times, most recently from 5d9b2ba to e3b36d7 Compare February 26, 2020 11:04
@rust-highfive

This comment has been minimized.

@shepmaster
Copy link
Member

added some commits almost 4 years ago

How time flies!

@rust-highfive

This comment has been minimized.

@dylanmckay dylanmckay force-pushed the avr-support-upstream branch 2 times, most recently from 9522529 to 37b32f0 Compare March 3, 2020 06:38
@rust-highfive

This comment has been minimized.

@dylanmckay
Copy link
Contributor Author

Any suggestions for where to put an AVR README or just AVR-specific documentation like build/usage instructions?

@rust-highfive

This comment has been minimized.

@dylanmckay dylanmckay force-pushed the avr-support-upstream branch from 110ca92 to 99f6776 Compare March 3, 2020 10:03
@dylanmckay
Copy link
Contributor Author

The only remaining failing automated test is related to name mangling, seems odd.

@rust-highfive

This comment has been minimized.

@shepmaster
Copy link
Member

shepmaster commented Mar 3, 2020

We may need to re-bless the test output for the ABI failure tests (ui/feature-gates/feature-gate-abi-avr-interrupt.r)

Patch generated with `./x.py test --stage 1 src/test/ui/feature-gates --bless`.
It is not possible to compile libstd for AVR anyway.
… ABI

This patch brings the AVR calling convention argument classification
logic in line with AVR Clang's behaviour.

AVR-Clang currently uses the `clang::DefaultABIInfo` ABI implementation.
This calling convention promotes all aggregates to indirect, no matter their
size.

It is also unnecessary to perform any integer width extension for AVR as
the minimum argument size matches the minimum describable size of
abi::Primitive::Int - 8 bits.

At some point in the future, an AVR-GCC compatible argument
classification implementation should be adopted in both Clang and Rust.
@dylanmckay dylanmckay force-pushed the avr-support-upstream branch from 491bf8c to 0340359 Compare June 9, 2020 05:49
@dylanmckay
Copy link
Contributor Author

Rebased, and the calling convention re-blessing removed from the git history.

@jonas-schievink
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Jun 9, 2020

📌 Commit 0340359 has been approved by jonas-schievink

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 9, 2020
@bors
Copy link
Contributor

bors commented Jun 11, 2020

⌛ Testing commit 0340359 with merge 0b208d1125cca703f2f834db83a8a41988ffa813...

@Dylan-DPC-zz
Copy link

@bors retry yield

@Dylan-DPC-zz
Copy link

@bors p=1

@bors
Copy link
Contributor

bors commented Jun 12, 2020

⌛ Testing commit 0340359 with merge e91bf6c...

@bors
Copy link
Contributor

bors commented Jun 12, 2020

☀️ Test successful - checks-azure
Approved by: jonas-schievink
Pushing e91bf6c to master...

@Restioson
Copy link

I have made the cleanup suggestions by @jplatte and @jplatte in avr-rust/rust-legacy-fork@043550d.

(from the original post)

Typo?

@shepmaster shepmaster deleted the avr-support-upstream branch June 12, 2020 14:09
@jplatte
Copy link
Contributor

jplatte commented Jun 12, 2020

@Restioson the wording of that commit is a bit weird, but I did make two suggestions, so it was likely intentional. (not meant to mention somebody else, at least).

@Restioson
Copy link

Ah, okay.

dylanmckay added a commit to dylanmckay/rust that referenced this pull request Jun 19, 2020
Adding a new ABI changes the hashes of all previous ABIs.

Fix suggested by @shepmaster in
rust-lang#69478 (comment).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. O-AVR Target: AVR processors (ATtiny, ATmega, etc.) relnotes Marks issues that should be documented in the release notes of the next release. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. WG-embedded Working group: Embedded systems
Projects
None yet
Development

Successfully merging this pull request may close these issues.