Make saturating u128 -> f32 casts the default behavior #45900
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Nov 9, 2017
|
LGTM. cc @alexcrichton @nikomatsakis |
hanna-kruppe
force-pushed
the
u128-to-f32-saturation-by-default
branch
from
91ac0fa to
2d5c892
Compare
Check compiletest suite=codegen mode=codegen (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
[01:00:12]
[01:00:12] running 54 tests
[01:00:16] thread 'main' panicked at 'Some tests failed', /checkout/src/tools/compiletest/src/main.rs:329:21
[01:00:16] i...i.ii...i.............i.....ii......i...i..i..i.F..
[01:00:16] failures:
[01:00:16]
[01:00:16] ---- [codegen] codegen/unchecked-float-casts.rs stdout ----
[01:00:16]
[01:00:16] error: verification with 'FileCheck' failed
[01:00:16] status: exit code: 1
[01:00:16] command: "/usr/lib/llvm-3.9/bin/FileCheck" "--input-file" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/codegen/unchecked-float-casts.ll" "/checkout/src/test/codegen/unchecked-float-casts.rs"
[01:00:16] stdout:
[01:00:16] ------------------------------------------
[01:00:16]
[01:00:16] ------------------------------------------
[01:00:16] stderr:
[01:00:16] ------------------------------------------
[01:00:16] /checkout/obj/build/x86_64-unknown-linux-gnu/test/codegen/unchecked-float-casts.ll:39:7: error: CHECK-NOT: string occurred!
[01:00:16] %2 = select i1 %0, float 0x7FF0000000000000, float %1
[01:00:16] ^
[01:00:16] /checkout/src/test/codegen/unchecked-float-casts.rs:63:16: note: CHECK-NOT: pattern specified here
[01:00:16] // CHECK-NOT: select
[01:00:16] ^
[01:00:16]
[01:00:16] ------------------------------------------
[01:00:16]
[01:00:16] thread '[codegen] codegen/unchecked-float-casts.rs' panicked at 'explicit panic', /checkout/src/tools/compiletest/src/runtest.rs:2499:8 |
hanna-kruppe
force-pushed
the
u128-to-f32-saturation-by-default
branch
from
2d5c892 to
4f29b1c
Compare
|
Oh, I forgot all about that test. Fixed 😅 |
kennytm
added
the
S-waiting-on-review
|
Nice! r=me but looks like tidy failed: |
... rather than being gated by -Z saturating-float-casts. There are several reasons for this: 1. Const eval already implements this behavior. 2. Unlike with float->int casts, this behavior is uncontroversially the right behavior and it is not as performance critical. Thus there is no particular need to make the bug fix for u128->f32 casts opt-in. 3. Having two orthogonal features under one flag is silly, and never should have happened in the first place. 4. Benchmarking float->int casts with the -Z flag should not pick up performance changes due to the u128->f32 casts (assuming there are any). Fixes rust-lang#41799
hanna-kruppe
force-pushed
the
u128-to-f32-saturation-by-default
branch
from
4f29b1c to
5952441
Compare
|
@bors r=alexcrichton |
|
📌 Commit 5952441 has been approved by |
kennytm
added
S-waiting-on-bors
bors
added a commit
that referenced
this pull request
Nov 12, 2017
…lexcrichton Make saturating u128 -> f32 casts the default behavior ... rather than being gated by `-Z saturating-float-casts`. There are several reasons for this: 1. Const eval already implements this behavior. 2. Unlike with float->int casts, this behavior is uncontroversially the right behavior and it is not as performance critical. Thus there is no particular need to make the bug fix for u128->f32 casts opt-in. 3. Having two orthogonal features under one flag is silly, and never should have happened in the first place. 4. Benchmarking float->int casts with the -Z flag should not pick up performance changes due to the u128->f32 casts (assuming there are any). Fixes #41799
|
☀️ Test successful - status-appveyor, status-travis |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
... rather than being gated by
-Z saturating-float-casts. There are several reasons for this:Fixes #41799