Rollup merge of rust-lang#62828 - nikic:fadd-mul-reductions, r=eddyb

Remove vector fadd/fmul reduction workarounds The bugs that this was working around have been fixed in LLVM 9. r? @gnzlbg
Centril · Jul 26, 2019 · 5ffdca7 · 5ffdca7
2 parents 632ce74 + 6fae7db
commit 5ffdca7
Show file tree

Hide file tree

Showing 7 changed files with 37 additions and 102 deletions.
diff --git a/src/librustc_codegen_llvm/builder.rs b/src/librustc_codegen_llvm/builder.rs
@@ -1153,21 +1153,21 @@ impl Builder<'a, 'll, 'tcx> {
         }
     }
 
+    pub fn vector_reduce_fadd(&mut self, acc: &'ll Value, src: &'ll Value) -> &'ll Value {
+        unsafe { llvm::LLVMRustBuildVectorReduceFAdd(self.llbuilder, acc, src) }
+    }
+    pub fn vector_reduce_fmul(&mut self, acc: &'ll Value, src: &'ll Value) -> &'ll Value {
+        unsafe { llvm::LLVMRustBuildVectorReduceFMul(self.llbuilder, acc, src) }
+    }
     pub fn vector_reduce_fadd_fast(&mut self, acc: &'ll Value, src: &'ll Value) -> &'ll Value {
         unsafe {
-            // FIXME: add a non-fast math version once
-            // https://bugs.llvm.org/show_bug.cgi?id=36732
-            // is fixed.
             let instr = llvm::LLVMRustBuildVectorReduceFAdd(self.llbuilder, acc, src);
             llvm::LLVMRustSetHasUnsafeAlgebra(instr);
             instr
         }
     }
     pub fn vector_reduce_fmul_fast(&mut self, acc: &'ll Value, src: &'ll Value) -> &'ll Value {
         unsafe {
-            // FIXME: add a non-fast math version once
-            // https://bugs.llvm.org/show_bug.cgi?id=36732
-            // is fixed.
             let instr = llvm::LLVMRustBuildVectorReduceFMul(self.llbuilder, acc, src);
             llvm::LLVMRustSetHasUnsafeAlgebra(instr);
             instr

diff --git a/src/librustc_codegen_llvm/common.rs b/src/librustc_codegen_llvm/common.rs
@@ -166,25 +166,6 @@ impl CodegenCx<'ll, 'tcx> {
             r
         }
     }
-
-    pub fn const_get_real(&self, v: &'ll Value) -> Option<(f64, bool)> {
-        unsafe {
-            if self.is_const_real(v) {
-                let mut loses_info: llvm::Bool = 0;
-                let r = llvm::LLVMConstRealGetDouble(v, &mut loses_info);
-                let loses_info = if loses_info == 1 { true } else { false };
-                Some((r, loses_info))
-            } else {
-                None
-            }
-        }
-    }
-
-    fn is_const_real(&self, v: &'ll Value) -> bool {
-        unsafe {
-            llvm::LLVMIsAConstantFP(v).is_some()
-        }
-    }
 }
 
 impl ConstMethods<'tcx> for CodegenCx<'ll, 'tcx> {

diff --git a/src/librustc_codegen_llvm/intrinsic.rs b/src/librustc_codegen_llvm/intrinsic.rs
@@ -1640,29 +1640,11 @@ fn generic_simd_intrinsic(
                         }
                     },
                     ty::Float(f) => {
-                        // ordered arithmetic reductions take an accumulator
                         let acc = if $ordered {
-                            let acc = args[1].immediate();
-                            // FIXME: https://bugs.llvm.org/show_bug.cgi?id=36734
-                            // * if the accumulator of the fadd isn't 0, incorrect
-                            //   code is generated
-                            // * if the accumulator of the fmul isn't 1, incorrect
-                            //   code is generated
-                            match bx.const_get_real(acc) {
-                                None => return_error!("accumulator of {} is not a constant", $name),
-                                Some((v, loses_info)) => {
-                                    if $name.contains("mul") && v != 1.0_f64 {
-                                        return_error!("accumulator of {} is not 1.0", $name);
-                                    } else if $name.contains("add") && v != 0.0_f64 {
-                                        return_error!("accumulator of {} is not 0.0", $name);
-                                    } else if loses_info {
-                                        return_error!("accumulator of {} loses information", $name);
-                                    }
-                                }
-                            }
-                            acc
+                            // ordered arithmetic reductions take an accumulator
+                            args[1].immediate()
                         } else {
-                            // unordered arithmetic reductions do not:
+                            // unordered arithmetic reductions use the identity accumulator
                             let identity_acc = if $name.contains("mul") { 1.0 } else { 0.0 };
                             match f.bit_width() {
                                 32 => bx.const_real(bx.type_f32(), identity_acc),
@@ -1688,8 +1670,8 @@ unsupported {} from `{}` with element `{}` of size `{}` to `{}`"#,
         }
     }
 
-    arith_red!("simd_reduce_add_ordered": vector_reduce_add, vector_reduce_fadd_fast, true);
-    arith_red!("simd_reduce_mul_ordered": vector_reduce_mul, vector_reduce_fmul_fast, true);
+    arith_red!("simd_reduce_add_ordered": vector_reduce_add, vector_reduce_fadd, true);
+    arith_red!("simd_reduce_mul_ordered": vector_reduce_mul, vector_reduce_fmul, true);
     arith_red!("simd_reduce_add_unordered": vector_reduce_add, vector_reduce_fadd_fast, false);
     arith_red!("simd_reduce_mul_unordered": vector_reduce_mul, vector_reduce_fmul_fast, false);
 

diff --git a/src/librustc_codegen_llvm/llvm/ffi.rs b/src/librustc_codegen_llvm/llvm/ffi.rs
@@ -719,7 +719,6 @@ extern "C" {
     pub fn LLVMConstIntGetZExtValue(ConstantVal: &Value) -> c_ulonglong;
     pub fn LLVMRustConstInt128Get(ConstantVal: &Value, SExt: bool,
                                   high: &mut u64, low: &mut u64) -> bool;
-    pub fn LLVMConstRealGetDouble (ConstantVal: &Value, losesInfo: &mut Bool) -> f64;
 
 
     // Operations on composite constants
@@ -1663,7 +1662,6 @@ extern "C" {
     pub fn LLVMRustWriteValueToString(value_ref: &Value, s: &RustString);
 
     pub fn LLVMIsAConstantInt(value_ref: &Value) -> Option<&Value>;
-    pub fn LLVMIsAConstantFP(value_ref: &Value) -> Option<&Value>;
 
     pub fn LLVMRustPassKind(Pass: &Pass) -> PassKind;
     pub fn LLVMRustFindAndCreatePass(Pass: *const c_char) -> Option<&'static mut Pass>;

diff --git a/src/test/run-pass/simd/simd-intrinsic-generic-reduction.rs b/src/test/run-pass/simd/simd-intrinsic-generic-reduction.rs
@@ -2,7 +2,7 @@
 #![allow(non_camel_case_types)]
 
 // ignore-emscripten
-// ignore-aarch64 FIXME: https://github.com/rust-lang/rust/issues/54510
+// min-system-llvm-version: 9.0
 
 // Test that the simd_reduce_{op} intrinsics produce the correct results.
 
@@ -124,14 +124,14 @@ fn main() {
         assert_eq!(r, 6_f32);
         let r: f32 = simd_reduce_mul_unordered(x);
         assert_eq!(r, -24_f32);
-        // FIXME: only works correctly for accumulator, 0:
-        // https://bugs.llvm.org/show_bug.cgi?id=36734
         let r: f32 = simd_reduce_add_ordered(x, 0.);
         assert_eq!(r, 6_f32);
-        // FIXME: only works correctly for accumulator, 1:
-        // https://bugs.llvm.org/show_bug.cgi?id=36734
         let r: f32 = simd_reduce_mul_ordered(x, 1.);
         assert_eq!(r, -24_f32);
+        let r: f32 = simd_reduce_add_ordered(x, 1.);
+        assert_eq!(r, 7_f32);
+        let r: f32 = simd_reduce_mul_ordered(x, 2.);
+        assert_eq!(r, -48_f32);
 
         let r: f32 = simd_reduce_min(x);
         assert_eq!(r, -2_f32);

diff --git a/src/test/ui/simd-intrinsic/simd-intrinsic-generic-reduction.rs b/src/test/ui/simd-intrinsic/simd-intrinsic-generic-reduction.rs
@@ -30,13 +30,10 @@ fn main() {
     let z = f32x4(0.0, 0.0, 0.0, 0.0);
 
     unsafe {
-        simd_reduce_add_ordered(z, 0_f32);
-        simd_reduce_mul_ordered(z, 1_f32);
-
-        simd_reduce_add_ordered(z, 2_f32);
-        //~^ ERROR accumulator of simd_reduce_add_ordered is not 0.0
-        simd_reduce_mul_ordered(z, 3_f32);
-        //~^ ERROR accumulator of simd_reduce_mul_ordered is not 1.0
+        simd_reduce_add_ordered(z, 0);
+        //~^ ERROR expected return type `f32` (element of input `f32x4`), found `i32`
+        simd_reduce_mul_ordered(z, 1);
+        //~^ ERROR expected return type `f32` (element of input `f32x4`), found `i32`
 
         let _: f32 = simd_reduce_and(x);
         //~^ ERROR expected return type `u32` (element of input `u32x4`), found `f32`
@@ -56,16 +53,5 @@ fn main() {
         //~^ ERROR unsupported simd_reduce_all from `f32x4` with element `f32` to `bool`
         let _: bool = simd_reduce_any(z);
         //~^ ERROR unsupported simd_reduce_any from `f32x4` with element `f32` to `bool`
-
-        foo(0_f32);
     }
 }
-
-#[inline(never)]
-unsafe fn foo(x: f32) {
-    let z = f32x4(0.0, 0.0, 0.0, 0.0);
-    simd_reduce_add_ordered(z, x);
-    //~^ ERROR accumulator of simd_reduce_add_ordered is not a constant
-    simd_reduce_mul_ordered(z, x);
-    //~^ ERROR accumulator of simd_reduce_mul_ordered is not a constant
-}
diff --git a/src/test/ui/simd-intrinsic/simd-intrinsic-generic-reduction.stderr b/src/test/ui/simd-intrinsic/simd-intrinsic-generic-reduction.stderr
@@ -1,74 +1,62 @@
-error[E0511]: invalid monomorphization of `simd_reduce_add_ordered` intrinsic: accumulator of simd_reduce_add_ordered is not 0.0
-  --> $DIR/simd-intrinsic-generic-reduction.rs:36:9
+error[E0511]: invalid monomorphization of `simd_reduce_add_ordered` intrinsic: expected return type `f32` (element of input `f32x4`), found `i32`
+  --> $DIR/simd-intrinsic-generic-reduction.rs:33:9
    |
-LL |         simd_reduce_add_ordered(z, 2_f32);
-   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+LL |         simd_reduce_add_ordered(z, 0);
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-error[E0511]: invalid monomorphization of `simd_reduce_mul_ordered` intrinsic: accumulator of simd_reduce_mul_ordered is not 1.0
-  --> $DIR/simd-intrinsic-generic-reduction.rs:38:9
+error[E0511]: invalid monomorphization of `simd_reduce_mul_ordered` intrinsic: expected return type `f32` (element of input `f32x4`), found `i32`
+  --> $DIR/simd-intrinsic-generic-reduction.rs:35:9
    |
-LL |         simd_reduce_mul_ordered(z, 3_f32);
-   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+LL |         simd_reduce_mul_ordered(z, 1);
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error[E0511]: invalid monomorphization of `simd_reduce_and` intrinsic: expected return type `u32` (element of input `u32x4`), found `f32`
-  --> $DIR/simd-intrinsic-generic-reduction.rs:41:22
+  --> $DIR/simd-intrinsic-generic-reduction.rs:38:22
    |
 LL |         let _: f32 = simd_reduce_and(x);
    |                      ^^^^^^^^^^^^^^^^^^
 
 error[E0511]: invalid monomorphization of `simd_reduce_or` intrinsic: expected return type `u32` (element of input `u32x4`), found `f32`
-  --> $DIR/simd-intrinsic-generic-reduction.rs:43:22
+  --> $DIR/simd-intrinsic-generic-reduction.rs:40:22
    |
 LL |         let _: f32 = simd_reduce_or(x);
    |                      ^^^^^^^^^^^^^^^^^
 
 error[E0511]: invalid monomorphization of `simd_reduce_xor` intrinsic: expected return type `u32` (element of input `u32x4`), found `f32`
-  --> $DIR/simd-intrinsic-generic-reduction.rs:45:22
+  --> $DIR/simd-intrinsic-generic-reduction.rs:42:22
    |
 LL |         let _: f32 = simd_reduce_xor(x);
    |                      ^^^^^^^^^^^^^^^^^^
 
 error[E0511]: invalid monomorphization of `simd_reduce_and` intrinsic: unsupported simd_reduce_and from `f32x4` with element `f32` to `f32`
-  --> $DIR/simd-intrinsic-generic-reduction.rs:48:22
+  --> $DIR/simd-intrinsic-generic-reduction.rs:45:22
    |
 LL |         let _: f32 = simd_reduce_and(z);
    |                      ^^^^^^^^^^^^^^^^^^
 
 error[E0511]: invalid monomorphization of `simd_reduce_or` intrinsic: unsupported simd_reduce_or from `f32x4` with element `f32` to `f32`
-  --> $DIR/simd-intrinsic-generic-reduction.rs:50:22
+  --> $DIR/simd-intrinsic-generic-reduction.rs:47:22
    |
 LL |         let _: f32 = simd_reduce_or(z);
    |                      ^^^^^^^^^^^^^^^^^
 
 error[E0511]: invalid monomorphization of `simd_reduce_xor` intrinsic: unsupported simd_reduce_xor from `f32x4` with element `f32` to `f32`
-  --> $DIR/simd-intrinsic-generic-reduction.rs:52:22
+  --> $DIR/simd-intrinsic-generic-reduction.rs:49:22
    |
 LL |         let _: f32 = simd_reduce_xor(z);
    |                      ^^^^^^^^^^^^^^^^^^
 
 error[E0511]: invalid monomorphization of `simd_reduce_all` intrinsic: unsupported simd_reduce_all from `f32x4` with element `f32` to `bool`
-  --> $DIR/simd-intrinsic-generic-reduction.rs:55:23
+  --> $DIR/simd-intrinsic-generic-reduction.rs:52:23
    |
 LL |         let _: bool = simd_reduce_all(z);
    |                       ^^^^^^^^^^^^^^^^^^
 
 error[E0511]: invalid monomorphization of `simd_reduce_any` intrinsic: unsupported simd_reduce_any from `f32x4` with element `f32` to `bool`
-  --> $DIR/simd-intrinsic-generic-reduction.rs:57:23
+  --> $DIR/simd-intrinsic-generic-reduction.rs:54:23
    |
 LL |         let _: bool = simd_reduce_any(z);
    |                       ^^^^^^^^^^^^^^^^^^
 
-error[E0511]: invalid monomorphization of `simd_reduce_add_ordered` intrinsic: accumulator of simd_reduce_add_ordered is not a constant
-  --> $DIR/simd-intrinsic-generic-reduction.rs:67:5
-   |
-LL |     simd_reduce_add_ordered(z, x);
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-
-error[E0511]: invalid monomorphization of `simd_reduce_mul_ordered` intrinsic: accumulator of simd_reduce_mul_ordered is not a constant
-  --> $DIR/simd-intrinsic-generic-reduction.rs:69:5
-   |
-LL |     simd_reduce_mul_ordered(z, x);
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-
-error: aborting due to 12 previous errors
+error: aborting due to 10 previous errors