Comments (15)
theemathas
commented on July 29, 2024
2
Minimized:
fn main() {
let tmp = if true { -0.0 } else { 1.0 };
if tmp == 0.0 {
println!("ok");
} else {
println!("bug")
}
}This incorrectly prints "bug" in release mode.
from rust.
nikic
commented on July 29, 2024
1
LLVM IR already includes an unconditional bug print. Also reproduces with -Z mir-opt-level=2 (without -O), so this is probably a mir opt bug (https://godbolt.org/z/Eo4j5Y1qW).
from rust.
saethlin
commented on July 29, 2024
1
Ah. You always need to stabilize the MIR-opt pipeline by carefully selecting specifically the passes you need when bisecting. You should bisect with the flags I mentioned above, -Zmir-opt-level=0 -Zmir-enable-passes=+JumpThreading.
from rust.
Noratrieb
commented on July 29, 2024
1
It looks like the bug here is that jump threading deals in ScalarInts, meaning it only looks at the bit pattern when checking if two values are equal. But floats are not compared by their bitpattern, they have more complicated rules!
fn main() {
let tmp = if true { f32::NAN } else { 1.0 };
if tmp == f32::NAN {
println!("true");
} else {
println!("false");
}
}same problem here, this prints true with optimizations enabled.
Either jump threading needs to be changed to handle float comparisons correctly (by storing extra state in its Condition) or it should just be disabled for floats alltogether.
from rust.
Noratrieb
commented on July 29, 2024
1
reopen to track potential backports
from rust.
tgross35
commented on July 29, 2024
To minimize this, you should try to inline the logic from inari into your code. E.g. type F64X2 = __m128d;, struct Interval { rep: F64X2 }, const_interval is a transumte, etc.
However, what is the reason you believe this to be a compiler/std bug rather than inari? The return -0 logic comes from inari.
from rust.
Chris00
commented on July 29, 2024
I tried but inlining the code, the program behaves correctly:
use std::{arch::x86_64::*, mem::transmute};
#[derive(Copy, Clone)]
struct I {
rep: __m128d,
}
impl I {
fn new(a: f64, b: f64) -> Self {
Self { rep: unsafe { transmute([-a, b]) } }
}
fn inf(self) -> f64 {
unsafe { transmute::<_, [f64; 2]>(self.rep)[0] }
}
}
fn f(u: I) -> f64 {
let t = u.inf() == 0.;
if t { 1. }
else { println!("{} {}", u.inf(), u.inf() == 0.);
u.inf() }
}
fn main() {
println!("{}", f(I::new(0., 0.)));
}from rust.
tgross35
commented on July 29, 2024
If I am understanding the code correctly then transmute::<_, [f64; 2]>(self.rep)[0] is only extract0, the logic from https://github.com/unageek/inari/blob/3660fa09392b90d320f7e6961e52922ed4b0587c/src/numeric.rs#L27-L37 and here https://github.com/unageek/inari/blob/3660fa09392b90d320f7e6961e52922ed4b0587c/src/interval.rs#L65-L67 seems to be missing.
from rust.
Chris00
commented on July 29, 2024
You are correct, I oversimplified the inlining. The bug can be reproduced with:
use std::{arch::x86_64::*, mem::transmute};
#[derive(Copy, Clone)]
struct I {
rep: __m128d,
}
impl I {
fn new(a: f64, b: f64) -> Self {
Self { rep: unsafe { transmute([-a, b]) } }
}
pub(crate) fn inf_raw(self) -> f64 {
unsafe { - transmute::<_, [f64; 2]>(self.rep)[0] }
}
fn inf(self) -> f64 {
let x = self.inf_raw();
if x.is_nan() {
// The empty interval.
f64::INFINITY
} else if x == 0.0 {
-0.0
} else {
x
}
}
}
fn f(u: I) -> f64 {
let t = u.inf() == 0.;
if t { 1. }
else { println!("{} {}", u.inf(), u.inf() == 0.);
u.inf() }
}
fn main() {
println!("{}", f(I::new(0., 0.)));
}from rust.
tgross35
commented on July 29, 2024
Reduced some
#![allow(unused)]
use std::{arch::x86_64::*, mem::transmute};
use std::hint::black_box;
fn check(x: f64) -> f64 {
if x == 0.0 {
-0.0
} else {
1234.0
}
}
fn main() {
let f = 0.0;
let simty: __m128d = unsafe { transmute([-f, f]) };
let as_f = unsafe { -transmute::<_, [f64; 2]>(simty)[0] };
if check(as_f) == 0.0 {
println!("ok");
} else {
println!("bug")
}
}https://godbolt.org/z/Whh4sh4bn
from rust.
tgross35
commented on July 29, 2024
What, why is this code fragile?? No simd types required.
pub fn main() {
let f: f64 = 0.0;
let as_f = -f;
println!("{:#018x}", as_f.to_bits());
let tmp = if -0.0 == 0.0 {
-0.0
} else {
0.0
};
if tmp == 0.0 {
println!("ok");
} else {
println!("bug")
}
}Prints "ok" by default, "bug" with -O https://godbolt.org/z/soz3jTThr.
Spot checking some versions it seems to show up at 1.78, which aligns with the LLVM18 version bump. Still reproduces on nightly.
This might be a known issue and it might be fixed with the LLVM19 update. Needs some more digging but I'm going to guess LLVM for now.
from rust.
saethlin
commented on July 29, 2024
This is a bug in JumpThreading. The bad codegen reproduces with -Zmir-opt-level=0 -Zmir-enable-passes=+JumpThreading.
The MIR diff for the bad optimization:
-// MIR for `main` before JumpThreading
+// MIR for `main` after JumpThreading
fn main() -> () {
let mut _0: ();
@@ -165,7 +165,7 @@ fn main() -> () {
bb7: {
_29 = const -0f64;
- goto -> bb9;
+ goto -> bb17;
}
bb8: {
@@ -179,7 +179,7 @@ fn main() -> () {
StorageLive(_32);
_32 = _29;
_31 = Eq(move _32, const 0f64);
- switchInt(move _31) -> [0: bb13, otherwise: bb10];
+ goto -> bb10;
}
bb10: {
@@ -242,4 +242,13 @@ fn main() -> () {
StorageDead(_1);
return;
}
+
+ bb17: {
+ StorageDead(_30);
+ StorageLive(_31);
+ StorageLive(_32);
+ _32 = _29;
+ _31 = Eq(move _32, const 0f64);
+ goto -> bb13;
+ }
}from rust.
tgross35
commented on July 29, 2024
$cargo bisect-rustc --start=1.74.0 --end=1.79.0 -- run --release
...
********************************************************************************
Regression in nightly-2024-02-12
********************************************************************************
fetching https://static.rust-lang.org/dist/2024-02-11/channel-rust-nightly-git-commit-hash.txt
nightly manifest 2024-02-11: 40 B / 40 B [======================================================================================================================================================] 100.00 % 694.44 KB/s converted 2024-02-11 to 6cc4843512d613f51ec81aba689180c31b0b28b6
fetching https://static.rust-lang.org/dist/2024-02-12/channel-rust-nightly-git-commit-hash.txt
nightly manifest 2024-02-12: 40 B / 40 B [======================================================================================================================================================] 100.00 % 498.67 KB/s converted 2024-02-12 to 1a648b397dedc98ada3dd3360f6d661ec2436c56
looking for regression commit between 2024-02-11 and 2024-02-12
fetching (via remote github) commits from max(6cc4843512d613f51ec81aba689180c31b0b28b6, 2024-02-09) to 1a648b397dedc98ada3dd3360f6d661ec2436c56
ending github query because we found starting sha: 6cc4843512d613f51ec81aba689180c31b0b28b6
get_commits_between returning commits, len: 9
commit[0] 2024-02-10: Auto merge of #119614 - RalfJung:const-refs-to-static, r=oli-obk
commit[1] 2024-02-10: Auto merge of #16527 - Veykril:salsa-no-self-ref, r=Veykril
commit[2] 2024-02-10: Auto merge of #117206 - cjgillot:jump-threading-default, r=tmiasko
commit[3] 2024-02-11: Auto merge of #120232 - c272:json-buildstd, r=Mark-Simulacrum
commit[4] 2024-02-11: Auto merge of #120405 - cjgillot:gvn-pointer, r=oli-obk
commit[5] 2024-02-11: Auto merge of #120914 - ehuss:downgrade-xcode, r=Mark-Simulacrum
commit[6] 2024-02-11: Auto merge of #120920 - matthiaskrgr:rollup-jsryr8e, r=matthiaskrgr
commit[7] 2024-02-11: Auto merge of #120903 - matthiaskrgr:rollup-tmsuzth, r=matthiaskrgr
commit[8] 2024-02-11: Auto merge of #120803 - onur-ozkan:fix-compilation-cache, r=Mark-Simulacrum
validated commits found, specifying toolchains
checking the start range to verify it passes
installing 0cbef48150e1fab161b5fd147b57ceb3f9272a52
rust-std-nightly-aarch64-apple-darwin: 25.33 MB / 25.33 MB [=====================================================================================================================================] 100.00 % 20.82 MB/s testing...
RESULT: 0cbef48150e1fab161b5fd147b57ceb3f9272a52, ===> Compile error
uninstalling 0cbef48150e1fab161b5fd147b57ceb3f9272a52
ERROR: the commit at the start of the range (0cbef48150e1fab161b5fd147b57ceb3f9272a52) includes the regression
from rust.
tgross35
commented on July 29, 2024
Well, #117206. Must have been a preexisting bug
from rust.
tgross35
commented on July 29, 2024
Ah, thanks.
********************************************************************************
Regression in nightly-2023-10-24
********************************************************************************
fetching https://static.rust-lang.org/dist/2023-10-23/channel-rust-nightly-git-commit-hash.txt
nightly manifest 2023-10-23: 40 B / 40 B [======================================================================================================================================================] 100.00 % 585.21 KB/s converted 2023-10-23 to 54b0434cead71e33bb4ddb52acde7767452b276d
fetching https://static.rust-lang.org/dist/2023-10-24/channel-rust-nightly-git-commit-hash.txt
nightly manifest 2023-10-24: 40 B / 40 B [======================================================================================================================================================] 100.00 % 398.93 KB/s converted 2023-10-24 to cd674d61790607dfb6faa9d754bd3adfa13aea7c
looking for regression commit between 2023-10-23 and 2023-10-24
fetching (via remote github) commits from max(54b0434cead71e33bb4ddb52acde7767452b276d, 2023-10-21) to cd674d61790607dfb6faa9d754bd3adfa13aea7c
ending github query because we found starting sha: 54b0434cead71e33bb4ddb52acde7767452b276d
get_commits_between returning commits, len: 14
commit[0] 2023-10-22: Auto merge of #117000 - weihanglo:update-cargo, r=weihanglo
commit[1] 2023-10-22: Auto merge of #117062 - Kobzol:update-rustc-perf, r=Mark-Simulacrum
commit[2] 2023-10-23: Auto merge of #115324 - francorbacho:master, r=davidtwco
commit[3] 2023-10-23: Auto merge of #117066 - calebcartwright:rustfmt-sync, r=calebcartwright
commit[4] 2023-10-23: Auto merge of #116606 - ChrisDenton:empty, r=dtolnay
commit[5] 2023-10-23: Auto merge of #117071 - matthiaskrgr:rollup-1tcxdgj, r=matthiaskrgr
commit[6] 2023-10-23: Auto merge of #116849 - oli-obk:error_shenanigans, r=cjgillot
commit[7] 2023-10-23: Auto merge of #116835 - oli-obk:evaluated_static_in_metadata2, r=RalfJung
commit[8] 2023-10-23: Auto merge of #116837 - oli-obk:smir_run_macro, r=spastorino
commit[9] 2023-10-23: Auto merge of #117087 - matthiaskrgr:rollup-08kkjkz, r=matthiaskrgr
commit[10] 2023-10-23: Auto merge of #107009 - cjgillot:jump-threading, r=pnkfelix
commit[11] 2023-10-23: Auto merge of #116033 - bvanjoi:fix-116032, r=petrochenkov
commit[12] 2023-10-23: Auto merge of #117103 - matthiaskrgr:rollup-96zuuom, r=matthiaskrgr
commit[13] 2023-10-24: Auto merge of #116300 - cjgillot:split-move, r=petrochenkov
ERROR: no CI builds available between 54b0434cead71e33bb4ddb52acde7767452b276d and cd674d61790607dfb6faa9d754bd3adfa13aea7c within last 167 days
Seems like it was from the original implementation at #107009.
Cc @cjgillot
from rust.
Related Issues (20)
- Should `Mutex` and `Condvar` respect priorities? HOT 2
- ICE: `unable to construct a valtree for the unevaluated constant` HOT 3
- fs::read_to_string(config.src.join("src/ci/channel")) failed with No such file or directory (os error 2) HOT 4
- Regression in `type annotations needed for` check between 1.79 and 1.80 HOT 2
- ICE: `trait takes 1 generic argument but 0 generic arguments were supplied` HOT 2
- Bug when using `cargo check` (my program works with `cargo run`) HOT 1
- zero-sized slices are sometimes placed at 0x1 (1.79+) HOT 15
- Documentation of default value for config.toml's `rust.channel` is inaccurate
- Rustdoc: struct fields are spaced too closely HOT 3
- [Enhancement]: `must_use_xor` to Force Using One of the Functions but not all in a Struct HOT 1
- false positive `dead_code` "never constructed" warning on 1.80.0 for a `struct` where the constructor takes `&'static self` HOT 11
- rustc_contracts: differentiate between exhaustive and non-exhaustive contracts
- ICE: The Rust Compiler Panicked After I Removed A Windows SDK HOT 3
- rustc book does not document RUST_TARGET_PATH HOT 1
- Tracking Issue for `nonzero_bitwise`
- Linking with `rust-lld` causes SIGSEGV in FFI code HOT 7
- Effective breakage to `jiff` due to `ambiguous_negative_literals` HOT 3
- Tracking Issue for `float_semantics` RFC 3514 HOT 1
- Spurious CI failures on x86_64-msvc-ext: "failed to remove file `C:\a\rust\rust\build\x86_64-pc-windows-msvc\stage2-tools\x86_64-pc-windows-msvc\release\cargo.exe`" HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from rust.