Conversation
100c507 to
7f0c1bc
Compare
7230b09 to
d2d4f22
Compare
CI in [1] seems to indicate that there are cases where the `f16` infinite recursion bug ([2], [3]) can make its way into what gets called during tests, even though this doesn't seem to be the usual case. In order to make sure that we avoid these completely, just unset `f16_enabled` on any platforms that have the recursion problem. This also refactors the `match` statement to be more in line with `library/std/build.rs`. [1]: rust-lang#729 [2]: llvm/llvm-project#97981 [3]: rust-lang#651
1a86a8d to
f6a5429
Compare
|
Includes #730 since PPC crashes without it. |
f6a5429 to
d9168a0
Compare
CI in [1] seems to indicate that there are cases where the `f16` infinite recursion bug ([2], [3]) can make its way into what gets called during tests, even though this doesn't seem to be the usual case. In order to make sure that we avoid these completely, just unset `f16_enabled` on any platforms that have the recursion problem. This also refactors the `match` statement to be more in line with `library/std/build.rs`. [1]: rust-lang/compiler-builtins#729 [2]: llvm/llvm-project#97981 [3]: rust-lang/compiler-builtins#651
CI in [1] seems to indicate that there are cases where the `f16` infinite recursion bug ([2], [3]) can make its way into what gets called during tests, even though this doesn't seem to be the usual case. In order to make sure that we avoid these completely, just unset `f16_enabled` on any platforms that have the recursion problem. This also refactors the `match` statement to be more in line with `library/std/build.rs`. [1]: rust-lang/compiler-builtins#729 [2]: llvm/llvm-project#97981 [3]: rust-lang/compiler-builtins#651
CI in [1] seems to indicate that there are cases where the `f16` infinite recursion bug ([2], [3]) can make its way into what gets called during tests, even though this doesn't seem to be the usual case. In order to make sure that we avoid these completely, just unset `f16_enabled` on any platforms that have the recursion problem. This also refactors the `match` statement to be more in line with `library/std/build.rs`. [1]: #729 [2]: llvm/llvm-project#97981 [3]: #651
d9168a0 to
d95f341
Compare
These are not present in LLVM's `compiler-rt` but LLVM does emit them in some cases [1]. [1]: rust-lang/rust#132614 (comment)
d95f341 to
9291a00
Compare
|
@quaternic would you mind double checking some of my logic here? With this current implementation, we are getting errors like the following: Which is coming from the bit of code here https://github.com/tgross35/compiler-builtins/blob/c462b7b5aa56a2ff3cdbfaf7c19143bbdee17b00/builtins-test/tests/conv.rs#L44-L79. I think the implementation is correct, especially considering the tests against apfloat seem okay. So I think this is an error in the validation algorithm here, which only shows up for f16 because As I'm typing it out that sounds pretty reasonable, just need to figure out how to patch this error algorithm. |
|
I think the validation logic is broken in that the apfloat-results that overflowed to infinity are converted back to an integer, specifically the maximum for the type, which should be irrelevant for the casting from integer to float. The overflow threshold is specifically when the exponent after rounding exceeds the maximum. But the rounding itself is done as if the exponent was unbounded. So for
|
|
Isn't that represented correctly? The apfloat bit at https://github.com/tgross35/compiler-builtins/blob/c462b7b5aa56a2ff3cdbfaf7c19143bbdee17b00/builtins-test/tests/conv.rs#L29-L37 should be the same as this: |
|
Lines 48-50 |
|
Ah, by
I thought you were referring to the test above that uses |
These are not present in LLVM's
compiler-rtbut LLVM does emit them in some cases 1.