Repository navigation
f32::powi on Windows returns different results between 1.44 and 1.45 beta #73420
Copy link
Copy link
Closed
Labels
A-LLVMArea: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues.Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues.A-floating-pointArea: Floating point numbers and arithmeticArea: Floating point numbers and arithmeticC-bugCategory: This is a bug.Category: This is a bug.O-windowsOperating system: WindowsOperating system: WindowsP-lowLow priorityLow priorityT-compilerRelevant to the compiler team, which will review and decide on the PR/issue.Relevant to the compiler team, which will review and decide on the PR/issue.regression-from-stable-to-stablePerformance or correctness regression from one stable version to another.Performance or correctness regression from one stable version to another.relnotesMarks issues that should be documented in the release notes of the next release.Marks issues that should be documented in the release notes of the next release.
Description
The issue was noticed because typenum::Pow::powi returns consistent results between rustc versions while f32::powi changes.
Given the following code the "std powi" results differ between stable and beta/nightly. Note that code below will run on https://play.rust-lang.org/ but the issue will not reproduce because the host is not Windows. The playground's output for stable/beta/nightly matches stable on my local machine.
extern crate typenum; // 2.12.0
fn main() {
let v = 12.046623229980468750000000000000_f32;
let a = a(v);
let b = b(v);
println!("std powi {:.30?}", a);
println!("typenum {:.30?}", b);
}
#[inline(never)]
fn a(v: f32) -> f32 {
v.powi(3)
}
#[inline(never)]
fn b(v: f32) -> f32 {
typenum::Pow::powi(v, typenum::P3::new())
}On my Windows 10 machine (and the Travis Win 10 VMs) I get the following results. I'm in the process of bisecting down to a specific nightly right now.
/d/Code/Test/rust/tst (master)$ cargo run; rustc --version --verbose
Finished dev [unoptimized + debuginfo] target(s) in 0.07s
Running `target\debug\tst.exe`
std powi 1748.219482421875000000000000000000
typenum 1748.219482421875000000000000000000
rustc 1.44.0 (49cae5576 2020-06-01)
binary: rustc
commit-hash: 49cae55760da0a43428eba73abcb659bb70cf2e4
commit-date: 2020-06-01
host: x86_64-pc-windows-msvc
release: 1.44.0
LLVM version: 9.0
/d/Code/Test/rust/tst (master)$ cargo +beta run; rustc +beta --version --verbose
Compiling tst v0.1.0 (D:\Code\Test\rust\tst)
Finished dev [unoptimized + debuginfo] target(s) in 4.95s
Running `target\debug\tst.exe`
std powi 1748.219604492187500000000000000000
typenum 1748.219482421875000000000000000000
rustc 1.45.0-beta.3359 (b7dc83a3f 2020-06-03)
binary: rustc
commit-hash: b7dc83a3f6ca9746fb3d121761c3605477b77d90
commit-date: 2020-06-03
host: x86_64-pc-windows-msvc
release: 1.45.0-beta.3359
LLVM version: 10.0
1748.2196044921875
1748.219482421875
Metadata
Metadata
Assignees
Labels
A-LLVMArea: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues.Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues.A-floating-pointArea: Floating point numbers and arithmeticArea: Floating point numbers and arithmeticC-bugCategory: This is a bug.Category: This is a bug.O-windowsOperating system: WindowsOperating system: WindowsP-lowLow priorityLow priorityT-compilerRelevant to the compiler team, which will review and decide on the PR/issue.Relevant to the compiler team, which will review and decide on the PR/issue.regression-from-stable-to-stablePerformance or correctness regression from one stable version to another.Performance or correctness regression from one stable version to another.relnotesMarks issues that should be documented in the release notes of the next release.Marks issues that should be documented in the release notes of the next release.
Activity
iliekturtles commentedon Jun 16, 2020
Issue first appears in nightly-2020-05-22-x86_64-pc-windows-msvc - rustc 1.45.0-nightly (9310e3b 2020-05-21)
Commit comparison with the previous nightly: 0aa6751...9310e3b
I'm guessing 9f12823 (Update LLVM submodule) is the culprit since the
f32::powicode just lowers to an LLVM intrisic.ecstatic-morse commentedon Jun 16, 2020
I'm guessing that LLVM 10 started using the MSVC implementation of the
powiintrinsic, which uses a higher precision for intermediate results. The result you see is equivalent to the one you would get from casting to andf64, multiplying and then casting back to andf32.ollie27 commentedon Jun 16, 2020
Looking at the changes in assembly for
powion 1.44.0 we have:callq __powisf2but on beta that's replaced with:
So LLVM has switched from calling the
__powisf2intrinsic from compiler-rt/compiler-builtins to callingpowf. This looks like an intentional change from https://reviews.llvm.org/D69013 but it's not clear to me why that change was made.mati865 commentedon Jun 16, 2020
iliekturtles commentedon Jun 17, 2020
Thanks for digging into the details. Sounds like this issue should be closed as working as intended then?
ollie27 commentedon Jun 17, 2020
I don't know. I think minor changes to the output of float operations between versions are technically allowed breakage but it would be nice to know why this change was made.
Given that
powiseemed to be working fine before, does anyone know why https://reviews.llvm.org/D69013 was done?10 remaining items
ollie27 commentedon Jun 25, 2020
Did anyone actually figure out why this change was made?
Mark-Simulacrum commentedon Jun 25, 2020
My impression is no. I don't think we should close this out before it's a compat note in relnotes and we have some sense of why this is happening (beyond "LLVM patch"), if only to hopefully be better able to explain to users why things broke for them.
eddyb commentedon Jun 25, 2020
Looks like the intention was to avoid calling
__powidf2as it's (sometimes?) not available in MSVC (so this might be for compiling withoutcompiler-rt?), and they assumed the expansion would be compatible. I'm not sure they were aware it might have a different result, or what the deal is with usecases that involvecompiler-rt.spastorino commentedon Jul 15, 2020
Given that this was added to release notes, I think this should be closed now?.
spastorino commentedon Jul 16, 2020
Tagging as
P-lowuntil we find some answers for @Mark-Simulacrum's questionsrust: Update to version 1.45.2.
iliekturtles commentedon Apr 13, 2022
@Mark-Simulacrum @spastorino With this documented in the 1.45.0 compatability notes are there still open questions that need action or are we good to close this issue?
Mark-Simulacrum commentedon Apr 13, 2022
I think we can close. I'm not fully happy about the explanation we have, but it seems good enough for the time being.
rust: Update to version 1.45.2.
rust: Update to version 1.45.2.
rust: Update to version 1.45.2.