Skip to content

Commit

Permalink
Rollup merge of rust-lang#49231 - gnzlbg:fix_vec_fminmax, r=rkruppe
Browse files Browse the repository at this point in the history
fix vector fmin/fmax non-fast/fast intrinsics NaN handling

This bugs shows up in release mode tests of `stdsimd`: rust-lang/stdarch#391 . The intrinsics are thoroughly tested there for roundoff errors, NaN, and overflow behavior.

The problem was that the non-fast intrinsics where specifying `NoNaNs == true`, which meant that they don't support NaNs. This is incorrect, the non-fast intrinsics should handle NaNs properly.

Also, the "fast" intrinsics where specifying `NoNaNs == false` which meant that they support NaNs and then fast-math, which probably disables this support. This was not intended either.

I've added a comment specifying what the boolean flags do.
  • Loading branch information
kennytm committed Mar 22, 2018
2 parents 3e95c71 + e0165af commit 87f5a4b
Showing 1 changed file with 4 additions and 4 deletions.
8 changes: 4 additions & 4 deletions src/librustc_trans/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1036,7 +1036,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
pub fn vector_reduce_fmin(&self, src: ValueRef) -> ValueRef {
self.count_insn("vector.reduce.fmin");
unsafe {
let instr = llvm::LLVMRustBuildVectorReduceFMin(self.llbuilder, src, true);
let instr = llvm::LLVMRustBuildVectorReduceFMin(self.llbuilder, src, /*NoNaNs:*/ false);
if instr.is_null() {
bug!("LLVMRustBuildVectorReduceFMin is not available in LLVM version < 5.0");
}
Expand All @@ -1046,7 +1046,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
pub fn vector_reduce_fmax(&self, src: ValueRef) -> ValueRef {
self.count_insn("vector.reduce.fmax");
unsafe {
let instr = llvm::LLVMRustBuildVectorReduceFMax(self.llbuilder, src, true);
let instr = llvm::LLVMRustBuildVectorReduceFMax(self.llbuilder, src, /*NoNaNs:*/ false);
if instr.is_null() {
bug!("LLVMRustBuildVectorReduceFMax is not available in LLVM version < 5.0");
}
Expand All @@ -1056,7 +1056,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
pub fn vector_reduce_fmin_fast(&self, src: ValueRef) -> ValueRef {
self.count_insn("vector.reduce.fmin_fast");
unsafe {
let instr = llvm::LLVMRustBuildVectorReduceFMin(self.llbuilder, src, false);
let instr = llvm::LLVMRustBuildVectorReduceFMin(self.llbuilder, src, /*NoNaNs:*/ true);
if instr.is_null() {
bug!("LLVMRustBuildVectorReduceFMin is not available in LLVM version < 5.0");
}
Expand All @@ -1067,7 +1067,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
pub fn vector_reduce_fmax_fast(&self, src: ValueRef) -> ValueRef {
self.count_insn("vector.reduce.fmax_fast");
unsafe {
let instr = llvm::LLVMRustBuildVectorReduceFMax(self.llbuilder, src, false);
let instr = llvm::LLVMRustBuildVectorReduceFMax(self.llbuilder, src, /*NoNaNs:*/ true);
if instr.is_null() {
bug!("LLVMRustBuildVectorReduceFMax is not available in LLVM version < 5.0");
}
Expand Down

0 comments on commit 87f5a4b

Please sign in to comment.