Skip to content

Upgrade toolchain and disable LTO #9

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

Upgrade toolchain and disable LTO #9

wants to merge 2 commits into from

Conversation

Patryk27
Copy link
Contributor

(commit messages contain rationale)

Patryk27 added 2 commits July 11, 2022 08:17
@Patryk27
chore: Upgrade toolchain
6494918 
Newest rustc contains an important bugfix to AVR's code generator
(rust-lang/rust#98567).
@Patryk27
Disable LTO
64633a4 
Under certain conditions, it triggers a confusing linking error¹.

¹ rust-lang/compiler-builtins#347
@Rahix Rahix linked an issue Jul 13, 2022 that may be closed by this pull request
upgrade rust-toolchain.toml to reference nightly-07-08 #10
Closed
Rahix added a commit to Rahix/avr-hal that referenced this pull request Jul 28, 2022
@Rahix
Upgrade toolchain
f6c8b31 
Newest rustc contains an important bugfix to AVR's code generator
(rust-lang/rust#98567).

Ref: Rahix/avr-hal-template#9
@Rahix
Copy link
Owner

Rahix commented Aug 4, 2022

Hey, completely missed to also leave a comment here. As you might have noticed, I have already pulled in the toolchain update commit here and in avr-hal. Thank you again for your persistent compiler work, you are bringing the AVR backend forward a great deal.

For the LTO change, I am unsure. From my experience, a lot of programs do not compile at all without LTO as a lot of libraries aren't annotated with #[inline] enough. I think even avr-hal might be an offender of this as it has been used with LTO almost exclusively up to now...

@Patryk27
Copy link
Contributor Author

Patryk27 commented Aug 5, 2022

Fair argument about the LTO - I suggested it mostly because people relatively frequently stumble upon that issue on internet (two times on r/rust already, IIRC) and it's peculiarly nasty, since it gives those "oh no, rust avr is broken again" vibes, when it's not even AVR's codegen fault 😓

But yeah, if from your experience disabing LTO makes things even worse, then maybe it's better to keep it enabled and focus on actually fixing the underlying issue 😄

@Rahix
Copy link
Owner

Rahix commented Aug 5, 2022

I agree, the situation is a bit unfortunate...

I do wonder, does the workaround I posted in that issue not work for the cases where you're hitting it?

@Patryk27
Copy link
Contributor Author

Patryk27 commented Aug 5, 2022

Hmm, in which issue? 👀

@Rahix
Copy link
Owner

Rahix commented Aug 5, 2022

Here: rust-lang/compiler-builtins#347 (comment)

@Patryk27
Copy link
Contributor Author

Patryk27 commented Aug 5, 2022

Ah, I see! I haven't checked that one yet, I'll try the next time I face this error.

@Patryk27
Copy link
Contributor Author

Lemme close this one, to keep it tidy - I'll re-open in case I find anything :-)

@Patryk27 Patryk27 closed this Aug 10, 2022
@Patryk27 Patryk27 deleted the upgrade-toolchain branch August 10, 2022 09:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

upgrade rust-toolchain.toml to reference nightly-07-08
2 participants
@Patryk27 @Rahix