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

[AVR] result of i32 as f32 is incorrect #77131

Closed
jsen- opened this issue Sep 24, 2020 · 4 comments
Closed

[AVR] result of i32 as f32 is incorrect #77131

jsen- opened this issue Sep 24, 2020 · 4 comments
Labels
A-floating-point Area: Floating point numbers and arithmetic C-bug Category: This is a bug. E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example O-AVR Target: AVR processors (ATtiny, ATmega, etc.) requires-nightly This issue requires a nightly compiler in some way.

Comments

@jsen-
Copy link
Contributor

jsen- commented Sep 24, 2020

I tried this code: https://github.com/jsen-/rust-i32-as-f32-issue-repro

I expected to see this happen:
Every i32 value converted to f32 should have equal decimal part and zeros-only after decimal point.

Instead, this happened:

i32   manual   using "as"
      conv.
----------------------------
0     0.0000   0.0000
1     1.0000   1.0078
2     2.0000   2.0313
3     3.0000   2.0469
4     4.0000   4.1250
5     5.0000   4.1563
6     6.0000   4.1875
7     7.0000   4.2188
8     8.0000   8.5000
9     9.0000   8.5625
10   10.0000   8.6250
11   11.0000   8.6875
12   12.0000   8.7500
13   13.0000   8.8125
14   14.0000   8.8750
15   15.0000   8.9375
16   16.0000  18.0000
...

full output

Meta

rustc --version --verbose:

rustc 1.48.0-nightly (0da580074 2020-09-22)
binary: rustc
commit-hash: 0da58007451a154da2480160429e1604a1f5f0ec
commit-date: 2020-09-22
host: x86_64-unknown-linux-gnu
release: 1.48.0-nightly
LLVM version: **11.0**

tested on arduino nano

Note: I updated compiler-builtins to 0.1.36, because without this commit I was getting undefined symbols __ashlsi3 and __lshrsi3

Note2:: Debug builds don't link due to missing references core::panicking::panic::hc30e67a739c4d8d1, but that's a separate issue. Release builds fine.

@jsen- jsen- added the C-bug Category: This is a bug. label Sep 24, 2020
@ecstatic-morse ecstatic-morse added O-AVR Target: AVR processors (ATtiny, ATmega, etc.) A-floating-point Area: Floating point numbers and arithmetic E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example labels Sep 24, 2020
@jonas-schievink jonas-schievink added the requires-nightly This issue requires a nightly compiler in some way. label Nov 7, 2020
@Rahix
Copy link

Rahix commented Feb 9, 2021

Note2:: Debug builds don't link due to missing references core::panicking::panic::hc30e67a739c4d8d1, but that's a separate issue. Release builds fine.

Off-topic but this is caused by rust-lang/compiler-builtins#347 ...

@aykevl
Copy link

aykevl commented May 9, 2021

I believe I've hit this issue too in TinyGo, if I remember correctly one of these patches would fix it:
https://reviews.llvm.org/D86546
https://reviews.llvm.org/D86547

@Patryk27
Copy link
Contributor

Patryk27 commented Dec 21, 2021

Currently the original code with .to_utf8chars() crashes inside simavr:

Loaded 19156 .text at address 0x0
Loaded 5062 .data
CORE: *** Invalid write address PC=017c SP=08ff O=920d Address 0000=78 low registers
avr_sadly_crashed
avr_gdb_init listening on port 1234

... but changing the impl ufmt::uDisplay to:

impl<const S: usize> ufmt::uDisplay for Buffer<S> {
    fn fmt<W: ufmt::uWrite + ?Sized>(
        &self,
        f: &mut ufmt::Formatter<'_, W>,
    ) -> Result<(), W::Error> {
        for ch in self.0.iter() {
            f.write_char(*ch as char);
        }

        Ok(())
    }
}

... seems to render a correct code, one that yields the expected results:

0  0.0000.........................  0.0000...........................
1  1.0000.........................  1.0000...........................
2  2.0000.........................  2.0000...........................
3  3.0000.........................  3.0000...........................
4  4.0000.........................  4.0000...........................
5  5.0000.........................  5.0000...........................
6  6.0000.........................  6.0000...........................
7  7.0000.........................  7.0000...........................
8  8.0000.........................  8.0000...........................
9  9.0000.........................  9.0000...........................
10 10.0000......................... 10.0000...........................
11 11.0000......................... 11.0000...........................
12 12.0000......................... 12.0000...........................
13 13.0000......................... 13.0000...........................
14 14.0000......................... 14.0000...........................
15 15.0000......................... 15.0000...........................
16 16.0000......................... 16.0000...........................
17 17.0000......................... 17.0000...........................
18 18.0000......................... 18.0000...........................
19 19.0000......................... 19.0000...........................
20 20.0000......................... 20.0000...........................

(requires https://reviews.llvm.org/D114611 + #82242 (comment))

@workingjubilee
Copy link
Member

As I understand it, this issue is resolved and adding simulation testing for functional correctness (as opposed to merely building correctly) for a tier 3 target may constitute a significant policy change, so I am closing this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-floating-point Area: Floating point numbers and arithmetic C-bug Category: This is a bug. E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example O-AVR Target: AVR processors (ATtiny, ATmega, etc.) requires-nightly This issue requires a nightly compiler in some way.
Projects
None yet
Development

No branches or pull requests

7 participants