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

Possible rust compilation issue ? #145

Closed
Frankkkkk opened this issue Jan 3, 2024 · 2 comments
Closed

Possible rust compilation issue ? #145

Frankkkkk opened this issue Jan 3, 2024 · 2 comments

Comments

@Frankkkkk
Copy link
Contributor

Hi,

I'm posting here first because I have an unexpected result on some code I wrote. It looks like a compiler issue but as I'm learning Rust, maybe it's just me doing dumb things :D. Could you please tell me is this is a misusage on my end before I raise the issue upstream ?

The code is here.

Basically, it consists of a RefCell that is written in an interrupt, and read on the main loop. The initial value is 0, and set to > 0 on the interrup. The main loop turns on the LED if the value is >0. So it starts off and then turns on.

(sorry for the rather strange code; I started removing logic to make a minimum MVP)

However, when there is no code on the else branch, the LED never turns on. If we put some code (like a nop, or something else) then it works as expected.

For the sake of simplicity, I created two examples:

  • GOOD, with a nop in the else branch -> LED turns on.
  • BAD with nothing on the else branch -> LED stays off.

I tried to debug the compilation, and I see that:

MIR

The runtime optimized after MIR (006.000) looks okay on both cases:

GOOD:

    bb14: {
        StorageLive(_25);
        StorageLive(_26);
        StorageLive(_27);
        _27 = const {alloc1: *mut RefCell<u8>};
        _26 = &mut (*_27);
        _25 = RefCell::<u8>::get_mut(move _26) -> [return: bb15, unwind unreachable];
    }

    bb15: {
        StorageDead(_26);
        _24 = (*_25);
        StorageDead(_27);
        StorageDead(_25);
        StorageLive(_28);
        _28 = Gt(_24, const 0_u8);
        switchInt(move _28) -> [0: bb19, otherwise: bb16];
    }

    bb16: {
        StorageLive(_30);
        StorageLive(_31);
        StorageLive(_32);
        _32 = &(_1.9: avr_device::atmega328p::PORTD);
        _31 = <PORTD as Deref>::deref(move _32) -> [return: bb17, unwind unreachable];
    }

    bb17: {
        StorageDead(_32);
        _30 = &((*_31).2: avr_device::generic::Reg<avr_device::atmega328p::portd::portd::PORTD_SPEC>);
        _29 = Reg::<PORTD_SPEC>::modify::<[closure@src/main.rs:58:35: 58:41]>(move _30, const ZeroSized: [closure@src/main.rs:58:35: 58:41]) -> [return: bb18, unwind unreachable];
    }

    bb18: {
        StorageDead(_30);
        StorageDead(_31);
        goto -> bb20;
    }

    bb19: {
        asm!("444444:", options((empty))) -> [return: bb20, unwind unreachable];
    }

    bb20: {
        StorageDead(_28);
        goto -> bb14;
    }

BAD:

   bb14: {
        StorageLive(_25);
        StorageLive(_26);
        StorageLive(_27);
        _27 = const {alloc1: *mut RefCell<u8>};
        _26 = &mut (*_27);
        _25 = RefCell::<u8>::get_mut(move _26) -> [return: bb15, unwind unreachable];
    }

    bb15: {
        StorageDead(_26);
        _24 = (*_25);
        StorageDead(_27);
        StorageDead(_25);
        StorageLive(_28);
        _28 = Gt(_24, const 0_u8);
        switchInt(move _28) -> [0: bb19, otherwise: bb16];
    }

    bb16: {
        StorageLive(_30);
        StorageLive(_31);
        StorageLive(_32);
        _32 = &(_1.9: avr_device::atmega328p::PORTD);
        _31 = <PORTD as Deref>::deref(move _32) -> [return: bb17, unwind unreachable];
    }

    bb17: {
        StorageDead(_32);
        _30 = &((*_31).2: avr_device::generic::Reg<avr_device::atmega328p::portd::portd::PORTD_SPEC>);
        _29 = Reg::<PORTD_SPEC>::modify::<[closure@src/main.rs:58:35: 58:41]>(move _30, const ZeroSized: [closure@src/main.rs:58:35: 58:41]) -> [return: bb18, unwind unreachable];
    }

    bb18: {
        StorageDead(_30);
        StorageDead(_31);
        goto -> bb19;
    }

    bb19: {
        StorageDead(_28);
        goto -> bb14;
    }

Basically, the diff is in bb18/bb19

LLVM-IR:

GOOD:

  br label %bb14, !dbg !3347
bb14:                                             ; preds = %bb19, %bb16, %"_ZN4core6option15Option$LT$T$GT$6unwrap17h450249f892692cf5E.exit"
  %wait_qty = load i8, ptr @_ZN12mega328_test3CTR17hd094fa9ca712b679E.0, align 1, !dbg !3349, !noundef !358
  call addrspace(1) void @llvm.dbg.value(metadata i8 %wait_qty, metadata !2947, metadata !DIExpression()), !dbg !3350
  %_28.not = icmp eq i8 %wait_qty, 0, !dbg !3351
  br i1 %_28.not, label %bb19, label %bb16, !dbg !3351

BAD:

  %wait_qty.pre1 = load i8, ptr @_ZN12mega328_test3CTR17hd094fa9ca712b679E.0, align 1, !dbg !3349
  br label %bb14, !dbg !3347

bb14:                                             ; preds = %bb14, %bb16, %"_ZN4core6option15Option$LT$T$GT$6unwrap17h450249f892692cf5E.exit"
  %wait_qty = phi i8 [ %wait_qty.pre1, %"_ZN4core6option15Option$LT$T$GT$6unwrap17h450249f892692cf5E.exit" ], [ %wait_qty.pre, %bb16 ], [ 0, %bb14 ], !dbg !3349
  call addrspace(1) void @llvm.dbg.value(metadata i8 %wait_qty, metadata !2947, metadata !DIExpression()), !dbg !3350
  %_28.not = icmp eq i8 %wait_qty, 0, !dbg !3351
  br i1 %_28.not, label %bb14, label %bb16, !dbg !3351

I don't really know LLVM, but the phi i8 that uses [0, %bb14] looks strange to me, making the condition after always true ?

Assembly

BAD:

    loop {
        let wait_qty: u8;
        unsafe { wait_qty = *CTR.get_mut(); };
  24:   80 91 00 00     lds     r24, 0x0000     ; 0x800000 <__SREG__+0x7fffc1>

        if wait_qty > 0 {
  28:   80 30           cpi     r24, 0x00       ; 0
  2a:   81 2d           mov     r24, r1
  2c:   01 f0           breq    .+0             ; 0x2e <_ZN12mega328_test20__avr_device_rt_main17h8ca9fc0fd7a09522E+0x2e>
  2e:   5a 9a           sbi     0x0b, 2 ; 11
  30:   00 c0           rjmp    .+0             ; 0x32 <_ZN12mega328_test20__avr_device_rt_main17h8ca9fc0fd7a09522E+0x32>

To be honest I don't really understand instruction 24; why would it try to load from 0x0000 ?

What do you think ?

Many thanks for your help !

@Rahix
Copy link
Owner

Rahix commented Jan 4, 2024

Hey, I think you should post this issue on the rustc repository, it looks like a compiler topic indeed.

@Frankkkkk
Copy link
Contributor Author

Thanks ! I've created the issue rust-lang/rust#119572 . I'll close this one for now then :-)

Cheers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants