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

Conversion from&to float<->integer #147

Merged
merged 6 commits into from
Apr 8, 2017
Merged

Conversion from&to float<->integer #147

merged 6 commits into from
Apr 8, 2017

Conversation

japaric
Copy link
Member

@japaric japaric commented Mar 6, 2017

this is a rebased version of #139

cc @ithinuel

@mattico
Copy link
Contributor

mattico commented Mar 6, 2017

Can close #106 also once this is merged.

We could probably use a macro to impl the Float trait just like the Int trait, but that can be done in a followup PR.

@japaric
Copy link
Member Author

japaric commented Mar 14, 2017

I can reproduce the powerpc segfault (inside QEMU) locally and consistently. It could be a QEMU bug as I have had segfaults like this with QEMUlated powerpc targets before; though, those were spurious.

@japaric
Copy link
Member Author

japaric commented Mar 14, 2017

FWIW, I can repro the segfault with Cross, which uses QEMU 2.7.1. The CI here is using QEMU 2.6.1. Interestingly, I don' see the segfault if I run the test suite in release mode.

Also given the error message

test float::conv::tests::_test::__fixdfdi ... Invalid instruction

This could be a problem with the default cpu / architecture-level that QEMU uses. Perhaps, we just have to pick a different cpu model in QEMU.

@oli-obk
Copy link
Contributor

oli-obk commented Apr 5, 2017

Does the architecture need to be changed here or in cross?

@japaric
Copy link
Member Author

japaric commented Apr 5, 2017

@oli-obk Here, this repo doesn't use Cross. I tried a few different QEMU_CPUs last time and none fixed the problem.

@japaric
Copy link
Member Author

japaric commented Apr 7, 2017

rust-lang/rust#41080 appears to have fixed the powerpc segfault (according to my local test). I'll retry once a new nightly becomes available.

@japaric
Copy link
Member Author

japaric commented Apr 8, 2017

Nightly's out. Let's try this.

@bors r+

@bors
Copy link
Contributor

bors commented Apr 8, 2017

📌 Commit 4c556dc has been approved by japaric

@bors
Copy link
Contributor

bors commented Apr 8, 2017

⌛ Testing commit 4c556dc with merge dbfcc16...

bors added a commit that referenced this pull request Apr 8, 2017
Conversion from&to float<->integer

this is a rebased version of #139

cc @ithinuel
@bors
Copy link
Contributor

bors commented Apr 8, 2017

💔 Test failed - status-travis

@oli-obk
Copy link
Contributor

oli-obk commented Apr 8, 2017

Getting undefined symbols now

@japaric
Copy link
Member Author

japaric commented Apr 8, 2017

I think that was a cache problem as I can't repro locally and AppVeyor passed plus I think I have seen this kind of error beofer. I have cleared the caches. Let's try again.

@bors retry

@bors
Copy link
Contributor

bors commented Apr 8, 2017

⌛ Testing commit 4c556dc with merge 9db9a8f...

bors added a commit that referenced this pull request Apr 8, 2017
Conversion from&to float<->integer

this is a rebased version of #139

cc @ithinuel
@bors
Copy link
Contributor

bors commented Apr 8, 2017

💔 Test failed - status-travis

@japaric
Copy link
Member Author

japaric commented Apr 8, 2017

---- float::conv::tests::_test::__fixsfdi stdout ----
__fixsfdi - Args: 9223372000000000000
compiler-builtins: Some(9223372036854775807)
compiler_rt: Some(-9223372036854775808)
gcc_s: Some(Some(-9223372036854775808))

It seems that we indeed are getting the wrong answer on powerpc64le.

use std::mem;

fn main() {
    let x: u32 = 9223372000000000000;
    let y: f32 = unsafe { mem::transmute(x) };
    let z = y as i64;
    println!("f32: {}", y);
    println!("i64: {}", z);
}
# x86_64
$ cargo run
f32: 242691860000000000000000000
i64: -9223372036854775808

But it seems that the intrinsics we are currently shipping with today's std are wrong too (?)

$ cross run --target aarch64-unknown-linux-gnu
f32: 242691860000000000000000000
i64: 9223372036854775807

$ cross run --target powerpc64le-unknown-linux-gnu
f32: 242691860000000000000000000
i64: 9223372036854775807

I'm not sure what's going on there.

@japaric
Copy link
Member Author

japaric commented Apr 8, 2017

Oh, wait. That value doesn't fit in the i64 so this is undef in LLVM. We shouldn't be testing undef conversions.

to avoid imprecision due to rounding to f32
@japaric
Copy link
Member Author

japaric commented Apr 8, 2017

@bors r+

@bors
Copy link
Contributor

bors commented Apr 8, 2017

📌 Commit 109c33e has been approved by japaric

@bors
Copy link
Contributor

bors commented Apr 8, 2017

⌛ Testing commit 109c33e with merge 28ac490...

bors added a commit that referenced this pull request Apr 8, 2017
Conversion from&to float<->integer

this is a rebased version of #139

cc @ithinuel
@japaric
Copy link
Member Author

japaric commented Apr 8, 2017

Cancelling Travis builds that won't be used by bors.

@bors
Copy link
Contributor

bors commented Apr 8, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: japaric
Pushing 28ac490 to master...

@bors bors merged commit 109c33e into master Apr 8, 2017
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

Successfully merging this pull request may close these issues.

5 participants