WIP: Implement signed/unsigned Integer convertion to single/double precision float

[ithinuel]

Implements :

• floatsisf
• floatsidf
• floatdidf
• floatunsisf
• floatunsidf
• floatundidf

add the "alias" __aeabi_ui2d -> __floatunsidf

[japaric]

Awesome! LGTM. Travis is having some problems today but I'll r+ this tomorrow or whenever that's fixed.

P.S. The aeabi_ui2d function should use the AAPCS calling convention but we can do that change in a follow up PR (there are a bunch of symbols that use the wrong calling convention on ARM cf #116. We should do all those in a single PR)

[japaric]

Could you add a comment here? "Unsigned version of Self" or something

[japaric]

Could you add a comment here as well? What does this do and where it's used.

[ithinuel]

Hi,
I also renamed convert to a shorter name to match pow, div, mul etc.
This implementation is based on the gcc's one and has been tested on a x86_64 for all 32bits values (signed and unsigned) in a sandbox.

I am currently testing the float->(u)int conversions and will push them on another PR.

[mattico]

This implementation is based on the gcc's one

Won't this cause issues with licensing and GPL? :/

https://www.gnu.org/licenses/gpl-faq.html#TranslateCode

The reason this project is under the "University of Illinois/NCSA Open Source License" is because we're translating code from compiler-rt which has that license.

[ithinuel]

Hm, well, I learnt how the conversion is made the hard way (by reading the macro-ception in GCC).
I just looked at the implementation from compiler-rt's and it is almost the same as what I have done.

I can still make an update to remove any ambiguity and even use the same variables' names.

EDIT: I noticed that I also forgot to check u64 to f32 conversion.

[bors]

☔ The latest upstream changes (presumably #128) made this pull request unmergeable. Please resolve the merge conflicts.

[ithinuel]

Hi, sorry, I'm working on this impl : https://github.com/ithinuel/compiler-builtins/tree/conv_from_llvm?files=1
I'm adding convertion from float to int
I just moved to belgium and don't have an internet connection yet.
I'll update this asap.
We can close this pr if you want me to make a new one.

[ithinuel]

Hi there, just so you know, I'm back online.
I still need to tune a little bit the code to fix issues with NaN handling.
It should be done for this week end.

[japaric]

@ithinuel Welcome back!

I still need to tune a little bit the code to fix issues with NaN handling.
It should be done for this week end.

Excellent!

cc @est31 IIRC, you wanted to extend these to work with i128 as well.

[est31]

@japaric yes I do! Right now this PR adds int -> float which is great (Can't tell yet whether it works yet, will know when I've tried...). I would need float -> int as well.

[ithinuel]

@est31 That is actually what I am adding to this PR (with an approch closer to the compiler-rt's algorithm).

Though, i am facing an issue with float->int.
What is the protocol to follow when compiler-rt & gcc does not agree on the result ? My tests can't pass.

[japaric]

What is the protocol to follow when compiler-rt & gcc does not agree on the result ? My tests can't pass.

Is that on some specific architecture? ARM? And how off is the result? few bits?

[ithinuel]

It is my VM Linux debian 4.8.0-2-amd64 #1 SMP Debian 4.8.11-1 (2016-12-02) x86_64 GNU/Linux running in a virtual box.
cargo test returns things like :

---- float::conv::tests::_test::__fixunssfdi stdout ----
	__fixunssfdi - Args: -56086066000000000000000000 
 compiler-builtins: Some(0)
 compiler_rt:       Some(0)
 gcc_s:             Some(Some(9223372036854775808))

(( I still can't use ci/run-docker.sh for some reason, probably a missing package on my setup )).

EDIT: Actually I dont understand (yet) what are the rules to handle values that are out of (u)integer's ranges.
EDIT2: This is an issue with the fact that my VM has a hardfloat ! I still have to deal with gcc & compiler-rt not giving the same result in some cases though.

[japaric]

fixunssfdi

signature: fn(f32) -> u64

-56086066000000000000000000

This value doesn't fit in a 64-bit integer. I don't know how your implementation handles it but this kind of conversion is currently UB in Rust. I'm not sure how GCC handles it; it seems it's returing u64::max_value()?

I think that we should reduce the input test range to [i64::max_value(), i64::max_value()] for now.

[ithinuel]

According to C99 clause 6.3.1.4

6.3.1.4 Real floating and integer
1 When a finite value of real floating type is converted to an integer type other than _Bool, the fractional
part is discarded (i.e., the value is truncated toward zero). If the value of the integral part cannot be
represented by the integer type, the behavior is undefined.

GCC seems to be returning 0 on my machine.
IHMO these cases should be handled like overflowing operations are. Is that "allowed" to panic! from compiler-builtins ?

[japaric]

Is that "allowed" to panic! from compiler-builtins ?

panic!s cause linker errors when LTO is used so we avoid them here. We use intrinsics::abort in some unreachable paths, like division by zero, because we know those paths get optimized away by the compiler or because we know they are really unreachable (a division intrinsics will never be called with denominator = zero).

I don't think the intrinsics are the correct place to fix this UB because not all platforms use the intrinsics to do int <-> float conversion so panicking inside the intrinsics would lead to the inconsistency where some platforms are protected and others simply end up hitting UB which can lead to sense of false confidence (e.g. you only test on a protected platform but also deploy on one of the UB platforms without testing it).

This UB problem has to be fixed in rustc and runtime checks have to be added to ensure the intrinsics are never called with invalid inputs (that it's already the case with division by zero). It just hasn't been implemented.

[ithinuel]

New version on #139.