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

Serial writes aren't working on rustc 1.82.0-nightly (8e86c9567 2024-08-01) #571

Open
hankertrix opened this issue Aug 2, 2024 · 12 comments · May be fixed by #585
Open

Serial writes aren't working on rustc 1.82.0-nightly (8e86c9567 2024-08-01) #571

hankertrix opened this issue Aug 2, 2024 · 12 comments · May be fixed by #585
Labels
compiler-bug Not a bug in avr-hal, but a bug in the rust compiler/LLVM

Comments

@hankertrix
Copy link
Contributor

hankertrix commented Aug 2, 2024

After updating the rustc compiler to rustc 1.82.0-nightly (8e86c9567 2024-08-01) from rustc 1.82.0-nightly (c1a6199e9 2024-07-24), serial writes don't work properly any more.

Here is the minimal reproducible example for the Arduino Mega 2560:

#![no_std]
#![no_main]

use panic_halt as _;

#[arduino_hal::entry]
fn main() -> ! {
    let peripherals = arduino_hal::Peripherals::take().unwrap();
    let pins = arduino_hal::pins!(peripherals);

    let mut serial = arduino_hal::default_serial!(peripherals, pins, 57600);
    ufmt::uwrite!(serial, "Serial initialised!\n").unwrap();
    loop {}
}

The console prints out Serial initialised! with a new line at the back as expected on rustc 1.82.0-nightly (c1a6199e9 2024-07-24), but only prints S on rustc 1.82.0-nightly (8e86c9567 2024-08-01). Nothing is printed to the console if other things are being initialised before the serial handler, such as a timer or a stepper motor driver.

I believe this should also be reproducible on the Arduino Uno as well.

@CoolSlimbo
Copy link

By comparing a diff of the generated assembly, you can see the difference.

- cpi r25, 0
- breq .LBB1_1
+ cpi r25, 1
+ brne .LBB1_1

The prior rust version (commit 8e86c9567) changes it to a "branch if not equal" op. I am by no means an assembly expert (first time ever looking at AVR instruction set), but, as it seems, this is saying to branch is the operation before (the cpi r25, 1) is not true.

Odds are, I've very much misinterpreted that, however. So, in place of the fact I have no clue what I'm doing, I'm going to call the one person who I believe can help. @Patryk27. This seems like a you kind of thing, if you don't mind.

@Patryk27
Copy link
Contributor

Patryk27 commented Aug 2, 2024

Thanks for pinging, I'll take a look later!

@Patryk27
Copy link
Contributor

Patryk27 commented Aug 2, 2024

Alright, so that comparison is actually correct - it's like if r25 == 0 { ... } vs if r25 != 1 { ... }, which do the same thing (given that r25 ∈ {0, 1}).

This codegen difference stems from some changes within rustc / llvm optimizations, because it used to generate:

  %_9.not = icmp eq i8 %1, 0
  br i1 %_9.not, label %bb11, label %bb10

... but since 2024-08-01 it generates this instead:

  %_9 = trunc nuw i8 %1 to i1
  br i1 %_9, label %bb10, label %bb11

I think both snippets are correct and what caught my attention were the jumps - all brne etc. in the invalid binary are offset by two bytes, which looks very wrong. I'll try to narrow the issue down.

@Rahix Rahix added the compiler-bug Not a bug in avr-hal, but a bug in the rust compiler/LLVM label Aug 2, 2024
@Patryk27
Copy link
Contributor

Patryk27 commented Aug 7, 2024

Status: I've confirmed it's a regression with LLVM (building newest rustc locally with LLVM 18 yields a correct binary), will try to git bisect it.

@Patryk27
Copy link
Contributor

Patryk27 commented Aug 8, 2024

Alright, it looks like the breaking commit was llvm/llvm-project@6859685 (or, more precisely, llvm/llvm-project@84428da, because the former has a small compilation issue fixed by this commit).

Now, onto discovering why an unrelated change broke AVR 🔍

@Patryk27
Copy link
Contributor

Patryk27 commented Aug 8, 2024

Got a lead, the difference between both versions boils down to this check:

https://github.com/llvm/llvm-project/blob/1919db907bc3e0541839992d903b6b875da98b04/llvm/lib/Target/AVR/MCTargetDesc/AVRAsmBackend.cpp#L521

This function returns true for the working version, but false nowadays - and, actually, judging by what it's doing, it was supposed to return false before as well, it was only now that this check became correct 🔍

@Patryk27
Copy link
Contributor

Status: patch is ready (llvm/llvm-project#102936), waiting for review.

@Tellurian-Ul
Copy link

Unrelated to the fix. I'm working on a project that's encountering this bug.

How do I rebuild using the old version of rustc nightly?

@Vollbrecht
Copy link
Contributor

Vollbrecht commented Sep 1, 2024

in your project you can define a rust-toolchain.toml file like its done here. Instead of using "nightly" you can specify a nightly version with a date of that nightly version.

@hankertrix
Copy link
Contributor Author

@Tellurian-Ul, navigate to your project root folder and run the command below:

rustup override set nightly-2024-07-25

@Patryk27
Copy link
Contributor

Patryk27 commented Sep 12, 2024

Status: fix got merged into rust-lang/llvm-project as a part of regular cross-repo merge, should be present in newest rustc soon.

@Patryk27
Copy link
Contributor

Alright, fixed in the newest toolchain 🙂

@Rahix Rahix linked a pull request Sep 25, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler-bug Not a bug in avr-hal, but a bug in the rust compiler/LLVM
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants