From e8b91c275d0188386d7b13f6248868cbf9b989a5 Mon Sep 17 00:00:00 2001
From: Neville Dipale <nevilledips@gmail.com>
Date: Fri, 24 Apr 2020 19:44:24 +0200
Subject: [PATCH 1/2] remove SIMD to assess impact on benchmarks

---
 rust/arrow/Cargo.toml                        |   4 +-
 rust/arrow/README.md                         |  10 --
 rust/arrow/src/buffer.rs                     | 110 +++----------
 rust/arrow/src/compute/kernels/arithmetic.rs | 153 ------------------
 rust/arrow/src/compute/kernels/boolean.rs    |   5 -
 rust/arrow/src/compute/kernels/comparison.rs | 111 +------------
 rust/arrow/src/compute/util.rs               | 118 --------------
 rust/arrow/src/datatypes.rs                  | 161 -------------------
 rust/arrow/src/util/bit_util.rs              |  43 -----
 9 files changed, 21 insertions(+), 694 deletions(-)

diff --git a/rust/arrow/Cargo.toml b/rust/arrow/Cargo.toml
index 479cb6cd098..19ccfb8745a 100644
--- a/rust/arrow/Cargo.toml
+++ b/rust/arrow/Cargo.toml
@@ -45,7 +45,6 @@ csv = "1.1"
 num = "0.2"
 regex = "1.3"
 lazy_static = "1.4"
-packed_simd = { version = "0.3", optional = true }
 chrono = "0.4"
 flatbuffers = "0.6"
 hex = "0.4"
@@ -53,9 +52,8 @@ arrow-flight = { path = "../arrow-flight", optional = true }
 prettytable-rs = "0.8.0"
 
 [features]
-simd = ["packed_simd"]
 flight = ["arrow-flight"]
-default = ["simd", "flight"]
+default = ["flight"]
 
 [dev-dependencies]
 criterion = "0.3"
diff --git a/rust/arrow/README.md b/rust/arrow/README.md
index 40a7db4f3c1..a8efaca5baf 100644
--- a/rust/arrow/README.md
+++ b/rust/arrow/README.md
@@ -64,16 +64,6 @@ The above script will run the `flatc` compiler and perform some adjustments to t
 - Remove `org::apache::arrow::flatbuffers` namespace
 - Add includes to each generated file
 
-## SIMD (Single Instruction Multiple Data)
-
-Arrow uses the [packed_simd](https://crates.io/crates/packed_simd) crate to optimize many of the implementations in the
-[compute](https://github.com/apache/arrow/tree/master/rust/arrow/src/compute) module using SIMD intrinsics.  These
-optimizations are enabled by the `simd` feature flag and are turned on by default, but can be disabled, for example:
-
-```bash
-cargo build --no-default-features
-```
-
 # Publishing to crates.io
 
 An Arrow committer can publish this crate after an official project release has
diff --git a/rust/arrow/src/buffer.rs b/rust/arrow/src/buffer.rs
index e98c372cb4f..de36967e937 100644
--- a/rust/arrow/src/buffer.rs
+++ b/rust/arrow/src/buffer.rs
@@ -18,8 +18,6 @@
 //! The main type in the module is `Buffer`, a contiguous immutable memory region of
 //! fixed size aligned at a 64-byte boundary. `MutableBuffer` is like `Buffer`, but it can
 //! be mutated and grown.
-#[cfg(feature = "simd")]
-use packed_simd::u8x64;
 
 use std::cmp;
 use std::convert::AsRef;
@@ -268,27 +266,6 @@ impl<T: AsRef<[u8]>> From<T> for Buffer {
     }
 }
 
-///  Helper function for SIMD `BitAnd` and `BitOr` implementations
-#[cfg(all(any(target_arch = "x86", target_arch = "x86_64"), feature = "simd"))]
-fn bitwise_bin_op_simd_helper<F>(left: &Buffer, right: &Buffer, op: F) -> Buffer
-where
-    F: Fn(u8x64, u8x64) -> u8x64,
-{
-    let mut result = MutableBuffer::new(left.len()).with_bitset(left.len(), false);
-    let lanes = u8x64::lanes();
-    for i in (0..left.len()).step_by(lanes) {
-        let left_data = unsafe { from_raw_parts(left.raw_data().add(i), lanes) };
-        let right_data = unsafe { from_raw_parts(right.raw_data().add(i), lanes) };
-        let result_slice: &mut [u8] = unsafe {
-            from_raw_parts_mut((result.data_mut().as_mut_ptr() as *mut u8).add(i), lanes)
-        };
-        unsafe {
-            bit_util::bitwise_bin_op_simd(&left_data, &right_data, result_slice, &op)
-        };
-    }
-    result.freeze()
-}
-
 impl<'a, 'b> BitAnd<&'b Buffer> for &'a Buffer {
     type Output = Result<Buffer>;
 
@@ -299,27 +276,15 @@ impl<'a, 'b> BitAnd<&'b Buffer> for &'a Buffer {
             ));
         }
 
-        // SIMD implementation if available
-        #[cfg(all(any(target_arch = "x86", target_arch = "x86_64"), feature = "simd"))]
-        {
-            return Ok(bitwise_bin_op_simd_helper(&self, &rhs, |a, b| a & b));
-        }
-
-        // Default implementation
-        #[allow(unreachable_code)]
-        {
-            let mut builder = UInt8BufferBuilder::new(self.len());
-            for i in 0..self.len() {
-                unsafe {
-                    builder
-                        .append(
-                            self.data().get_unchecked(i) & rhs.data().get_unchecked(i),
-                        )
-                        .unwrap();
-                }
+        let mut builder = UInt8BufferBuilder::new(self.len());
+        for i in 0..self.len() {
+            unsafe {
+                builder
+                    .append(self.data().get_unchecked(i) & rhs.data().get_unchecked(i))
+                    .unwrap();
             }
-            Ok(builder.finish())
         }
+        Ok(builder.finish())
     }
 }
 
@@ -333,27 +298,15 @@ impl<'a, 'b> BitOr<&'b Buffer> for &'a Buffer {
             ));
         }
 
-        // SIMD implementation if available
-        #[cfg(all(any(target_arch = "x86", target_arch = "x86_64"), feature = "simd"))]
-        {
-            return Ok(bitwise_bin_op_simd_helper(&self, &rhs, |a, b| a | b));
-        }
-
-        // Default implementation
-        #[allow(unreachable_code)]
-        {
-            let mut builder = UInt8BufferBuilder::new(self.len());
-            for i in 0..self.len() {
-                unsafe {
-                    builder
-                        .append(
-                            self.data().get_unchecked(i) | rhs.data().get_unchecked(i),
-                        )
-                        .unwrap();
-                }
+        let mut builder = UInt8BufferBuilder::new(self.len());
+        for i in 0..self.len() {
+            unsafe {
+                builder
+                    .append(self.data().get_unchecked(i) | rhs.data().get_unchecked(i))
+                    .unwrap();
             }
-            Ok(builder.finish())
         }
+        Ok(builder.finish())
     }
 }
 
@@ -361,38 +314,13 @@ impl Not for &Buffer {
     type Output = Buffer;
 
     fn not(self) -> Buffer {
-        // SIMD implementation if available
-        #[cfg(all(any(target_arch = "x86", target_arch = "x86_64"), feature = "simd"))]
-        {
-            let mut result =
-                MutableBuffer::new(self.len()).with_bitset(self.len(), false);
-            let lanes = u8x64::lanes();
-            for i in (0..self.len()).step_by(lanes) {
-                unsafe {
-                    let data = from_raw_parts(self.raw_data().add(i), lanes);
-                    let data_simd = u8x64::from_slice_unaligned_unchecked(data);
-                    let simd_result = !data_simd;
-                    let result_slice: &mut [u8] = from_raw_parts_mut(
-                        (result.data_mut().as_mut_ptr() as *mut u8).add(i),
-                        lanes,
-                    );
-                    simd_result.write_to_slice_unaligned_unchecked(result_slice);
-                }
-            }
-            return result.freeze();
-        }
-
-        // Default implementation
-        #[allow(unreachable_code)]
-        {
-            let mut builder = UInt8BufferBuilder::new(self.len());
-            for i in 0..self.len() {
-                unsafe {
-                    builder.append(!self.data().get_unchecked(i)).unwrap();
-                }
+        let mut builder = UInt8BufferBuilder::new(self.len());
+        for i in 0..self.len() {
+            unsafe {
+                builder.append(!self.data().get_unchecked(i)).unwrap();
             }
-            builder.finish()
         }
+        builder.finish()
     }
 }
 
diff --git a/rust/arrow/src/compute/kernels/arithmetic.rs b/rust/arrow/src/compute/kernels/arithmetic.rs
index 85d684075aa..c56a5d2a3ff 100644
--- a/rust/arrow/src/compute/kernels/arithmetic.rs
+++ b/rust/arrow/src/compute/kernels/arithmetic.rs
@@ -16,30 +16,15 @@
 // under the License.
 
 //! Defines basic arithmetic kernels for `PrimitiveArrays`.
-//!
-//! These kernels can leverage SIMD if available on your system.  Currently no runtime
-//! detection is provided, you should enable the specific SIMD intrinsics using
-//! `RUSTFLAGS="-C target-feature=+avx2"` for example.  See the documentation
-//! [here](https://doc.rust-lang.org/stable/core/arch/) for more information.
 
-#[cfg(feature = "simd")]
-use std::mem;
 use std::ops::{Add, Div, Mul, Sub};
-#[cfg(feature = "simd")]
-use std::slice::from_raw_parts_mut;
 use std::sync::Arc;
 
 use num::{One, Zero};
 
 use crate::array::*;
-#[cfg(feature = "simd")]
-use crate::bitmap::Bitmap;
 use crate::buffer::Buffer;
-#[cfg(feature = "simd")]
-use crate::buffer::MutableBuffer;
 use crate::compute::util::apply_bin_op_to_option_bitmap;
-#[cfg(all(any(target_arch = "x86", target_arch = "x86_64"), feature = "simd"))]
-use crate::compute::util::simd_load_set_invalid;
 use crate::datatypes;
 use crate::datatypes::ToByteSlice;
 use crate::error::{ArrowError, Result};
@@ -85,128 +70,6 @@ where
     Ok(PrimitiveArray::<T>::from(Arc::new(data)))
 }
 
-/// SIMD vectorized version of `math_op` above.
-#[cfg(all(any(target_arch = "x86", target_arch = "x86_64"), feature = "simd"))]
-fn simd_math_op<T, F>(
-    left: &PrimitiveArray<T>,
-    right: &PrimitiveArray<T>,
-    op: F,
-) -> Result<PrimitiveArray<T>>
-where
-    T: datatypes::ArrowNumericType,
-    T::Simd: Add<Output = T::Simd>
-        + Sub<Output = T::Simd>
-        + Mul<Output = T::Simd>
-        + Div<Output = T::Simd>,
-    F: Fn(T::Simd, T::Simd) -> T::Simd,
-{
-    if left.len() != right.len() {
-        return Err(ArrowError::ComputeError(
-            "Cannot perform math operation on arrays of different length".to_string(),
-        ));
-    }
-
-    let null_bit_buffer = apply_bin_op_to_option_bitmap(
-        left.data().null_bitmap(),
-        right.data().null_bitmap(),
-        |a, b| a & b,
-    )?;
-
-    let lanes = T::lanes();
-    let buffer_size = left.len() * mem::size_of::<T::Native>();
-    let mut result = MutableBuffer::new(buffer_size).with_bitset(buffer_size, false);
-
-    for i in (0..left.len()).step_by(lanes) {
-        let simd_left = T::load(left.value_slice(i, lanes));
-        let simd_right = T::load(right.value_slice(i, lanes));
-        let simd_result = T::bin_op(simd_left, simd_right, &op);
-
-        let result_slice: &mut [T::Native] = unsafe {
-            from_raw_parts_mut(
-                (result.data_mut().as_mut_ptr() as *mut T::Native).add(i),
-                lanes,
-            )
-        };
-        T::write(simd_result, result_slice);
-    }
-
-    let data = ArrayData::new(
-        T::get_data_type(),
-        left.len(),
-        None,
-        null_bit_buffer,
-        left.offset(),
-        vec![result.freeze()],
-        vec![],
-    );
-    Ok(PrimitiveArray::<T>::from(Arc::new(data)))
-}
-
-/// SIMD vectorized version of `divide`, the divide kernel needs it's own implementation as there
-/// is a need to handle situations where a divide by `0` occurs.  This is complicated by `NULL`
-/// slots and padding.
-#[cfg(all(any(target_arch = "x86", target_arch = "x86_64"), feature = "simd"))]
-fn simd_divide<T>(
-    left: &PrimitiveArray<T>,
-    right: &PrimitiveArray<T>,
-) -> Result<PrimitiveArray<T>>
-where
-    T: datatypes::ArrowNumericType,
-    T::Native: One + Zero,
-{
-    if left.len() != right.len() {
-        return Err(ArrowError::ComputeError(
-            "Cannot perform math operation on arrays of different length".to_string(),
-        ));
-    }
-
-    // Create the combined `Bitmap`
-    let null_bit_buffer = apply_bin_op_to_option_bitmap(
-        left.data().null_bitmap(),
-        right.data().null_bitmap(),
-        |a, b| a & b,
-    )?;
-    let bitmap = null_bit_buffer.map(Bitmap::from);
-
-    let lanes = T::lanes();
-    let buffer_size = left.len() * mem::size_of::<T::Native>();
-    let mut result = MutableBuffer::new(buffer_size).with_bitset(buffer_size, false);
-
-    for i in (0..left.len()).step_by(lanes) {
-        let right_no_invalid_zeros =
-            unsafe { simd_load_set_invalid(right, &bitmap, i, lanes, T::Native::one()) };
-        let is_zero = T::eq(T::init(T::Native::zero()), right_no_invalid_zeros);
-        if T::mask_any(is_zero) {
-            return Err(ArrowError::DivideByZero);
-        }
-        let right_no_invalid_zeros =
-            unsafe { simd_load_set_invalid(right, &bitmap, i, lanes, T::Native::one()) };
-        let simd_left = T::load(left.value_slice(i, lanes));
-        let simd_result = T::bin_op(simd_left, right_no_invalid_zeros, |a, b| a / b);
-
-        let result_slice: &mut [T::Native] = unsafe {
-            from_raw_parts_mut(
-                (result.data_mut().as_mut_ptr() as *mut T::Native).add(i),
-                lanes,
-            )
-        };
-        T::write(simd_result, result_slice);
-    }
-
-    let null_bit_buffer = bitmap.map(|b| b.bits);
-
-    let data = ArrayData::new(
-        T::get_data_type(),
-        left.len(),
-        None,
-        null_bit_buffer,
-        left.offset(),
-        vec![result.freeze()],
-        vec![],
-    );
-    Ok(PrimitiveArray::<T>::from(Arc::new(data)))
-}
-
 /// Perform `left + right` operation on two arrays. If either left or right value is null
 /// then the result is also null.
 pub fn add<T>(
@@ -221,10 +84,6 @@ where
         + Div<Output = T::Native>
         + Zero,
 {
-    #[cfg(all(any(target_arch = "x86", target_arch = "x86_64"), feature = "simd"))]
-    return simd_math_op(&left, &right, |a, b| a + b);
-
-    #[allow(unreachable_code)]
     math_op(left, right, |a, b| Ok(a + b))
 }
 
@@ -242,10 +101,6 @@ where
         + Div<Output = T::Native>
         + Zero,
 {
-    #[cfg(all(any(target_arch = "x86", target_arch = "x86_64"), feature = "simd"))]
-    return simd_math_op(&left, &right, |a, b| a - b);
-
-    #[allow(unreachable_code)]
     math_op(left, right, |a, b| Ok(a - b))
 }
 
@@ -263,10 +118,6 @@ where
         + Div<Output = T::Native>
         + Zero,
 {
-    #[cfg(all(any(target_arch = "x86", target_arch = "x86_64"), feature = "simd"))]
-    return simd_math_op(&left, &right, |a, b| a * b);
-
-    #[allow(unreachable_code)]
     math_op(left, right, |a, b| Ok(a * b))
 }
 
@@ -286,10 +137,6 @@ where
         + Zero
         + One,
 {
-    #[cfg(all(any(target_arch = "x86", target_arch = "x86_64"), feature = "simd"))]
-    return simd_divide(&left, &right);
-
-    #[allow(unreachable_code)]
     math_op(left, right, |a, b| {
         if b.is_zero() {
             Err(ArrowError::DivideByZero)
diff --git a/rust/arrow/src/compute/kernels/boolean.rs b/rust/arrow/src/compute/kernels/boolean.rs
index b4981a90363..7dc9afb379a 100644
--- a/rust/arrow/src/compute/kernels/boolean.rs
+++ b/rust/arrow/src/compute/kernels/boolean.rs
@@ -16,11 +16,6 @@
 // under the License.
 
 //! Defines boolean kernels on Arrow `BooleanArray`'s, e.g. `AND`, `OR` and `NOT`.
-//!
-//! These kernels can leverage SIMD if available on your system.  Currently no runtime
-//! detection is provided, you should enable the specific SIMD intrinsics using
-//! `RUSTFLAGS="-C target-feature=+avx2"` for example.  See the documentation
-//! [here](https://doc.rust-lang.org/stable/core/arch/) for more information.
 
 use std::sync::Arc;
 
diff --git a/rust/arrow/src/compute/kernels/comparison.rs b/rust/arrow/src/compute/kernels/comparison.rs
index 390c2769b8d..5249daa756c 100644
--- a/rust/arrow/src/compute/kernels/comparison.rs
+++ b/rust/arrow/src/compute/kernels/comparison.rs
@@ -16,11 +16,6 @@
 // under the License.
 
 //! Defines basic comparison kernels for `PrimitiveArrays`.
-//!
-//! These kernels can leverage SIMD if available on your system.  Currently no runtime
-//! detection is provided, you should enable the specific SIMD intrinsics using
-//! `RUSTFLAGS="-C target-feature=+avx2"` for example.  See the documentation
-//! [here](https://doc.rust-lang.org/stable/core/arch/) for more information.
 
 use regex::Regex;
 use std::collections::HashMap;
@@ -31,8 +26,7 @@ use crate::compute::util::apply_bin_op_to_option_bitmap;
 use crate::datatypes::{ArrowNumericType, BooleanType, DataType};
 use crate::error::{ArrowError, Result};
 
-/// Helper function to perform boolean lambda function on values from two arrays, this
-/// version does not attempt to use SIMD.
+/// Helper function to perform boolean lambda function on values from two arrays
 macro_rules! compare_op {
     ($left: expr, $right:expr, $op:expr) => {{
         if $left.len() != $right.len() {
@@ -198,79 +192,11 @@ pub fn gt_eq_utf8(left: &StringArray, right: &StringArray) -> Result<BooleanArra
     compare_op!(left, right, |a, b| a >= b)
 }
 
-/// Helper function to perform boolean lambda function on values from two arrays using
-/// SIMD.
-#[cfg(all(any(target_arch = "x86", target_arch = "x86_64"), feature = "simd"))]
-fn simd_compare_op<T, F>(
-    left: &PrimitiveArray<T>,
-    right: &PrimitiveArray<T>,
-    op: F,
-) -> Result<BooleanArray>
-where
-    T: ArrowNumericType,
-    F: Fn(T::Simd, T::Simd) -> T::SimdMask,
-{
-    let len = left.len();
-    if len != right.len() {
-        return Err(ArrowError::ComputeError(
-            "Cannot perform comparison operation on arrays of different length"
-                .to_string(),
-        ));
-    }
-
-    let null_bit_buffer = apply_bin_op_to_option_bitmap(
-        left.data().null_bitmap(),
-        right.data().null_bitmap(),
-        |a, b| a & b,
-    )?;
-
-    let lanes = T::lanes();
-    let mut result = BooleanBufferBuilder::new(len);
-
-    let rem = len % lanes;
-
-    for i in (0..len - rem).step_by(lanes) {
-        let simd_left = T::load(left.value_slice(i, lanes));
-        let simd_right = T::load(right.value_slice(i, lanes));
-        let simd_result = op(simd_left, simd_right);
-        for i in 0..lanes {
-            result.append(T::mask_get(&simd_result, i))?;
-        }
-    }
-
-    if rem > 0 {
-        let simd_left = T::load(left.value_slice(len - rem, lanes));
-        let simd_right = T::load(right.value_slice(len - rem, lanes));
-        let simd_result = op(simd_left, simd_right);
-        for i in 0..rem {
-            result.append(T::mask_get(&simd_result, i))?;
-        }
-    }
-
-    let data = ArrayData::new(
-        DataType::Boolean,
-        left.len(),
-        None,
-        null_bit_buffer,
-        left.offset(),
-        vec![result.finish()],
-        vec![],
-    );
-    Ok(PrimitiveArray::<BooleanType>::from(Arc::new(data)))
-}
-
 /// Perform `left == right` operation on two arrays.
 pub fn eq<T>(left: &PrimitiveArray<T>, right: &PrimitiveArray<T>) -> Result<BooleanArray>
 where
     T: ArrowNumericType,
 {
-    #[cfg(all(any(target_arch = "x86", target_arch = "x86_64"), feature = "simd"))]
-    return simd_compare_op(left, right, T::eq);
-
-    #[cfg(any(
-        not(any(target_arch = "x86", target_arch = "x86_64")),
-        not(feature = "simd")
-    ))]
     compare_op!(left, right, |a, b| a == b)
 }
 
@@ -279,13 +205,6 @@ pub fn neq<T>(left: &PrimitiveArray<T>, right: &PrimitiveArray<T>) -> Result<Boo
 where
     T: ArrowNumericType,
 {
-    #[cfg(all(any(target_arch = "x86", target_arch = "x86_64"), feature = "simd"))]
-    return simd_compare_op(left, right, T::ne);
-
-    #[cfg(any(
-        not(any(target_arch = "x86", target_arch = "x86_64")),
-        not(feature = "simd")
-    ))]
     compare_op!(left, right, |a, b| a != b)
 }
 
@@ -295,13 +214,6 @@ pub fn lt<T>(left: &PrimitiveArray<T>, right: &PrimitiveArray<T>) -> Result<Bool
 where
     T: ArrowNumericType,
 {
-    #[cfg(all(any(target_arch = "x86", target_arch = "x86_64"), feature = "simd"))]
-    return simd_compare_op(left, right, T::lt);
-
-    #[cfg(any(
-        not(any(target_arch = "x86", target_arch = "x86_64")),
-        not(feature = "simd")
-    ))]
     compare_op!(left, right, |a, b| a < b)
 }
 
@@ -314,13 +226,6 @@ pub fn lt_eq<T>(
 where
     T: ArrowNumericType,
 {
-    #[cfg(all(any(target_arch = "x86", target_arch = "x86_64"), feature = "simd"))]
-    return simd_compare_op(left, right, T::le);
-
-    #[cfg(any(
-        not(any(target_arch = "x86", target_arch = "x86_64")),
-        not(feature = "simd")
-    ))]
     compare_op!(left, right, |a, b| a <= b)
 }
 
@@ -330,13 +235,6 @@ pub fn gt<T>(left: &PrimitiveArray<T>, right: &PrimitiveArray<T>) -> Result<Bool
 where
     T: ArrowNumericType,
 {
-    #[cfg(all(any(target_arch = "x86", target_arch = "x86_64"), feature = "simd"))]
-    return simd_compare_op(left, right, T::gt);
-
-    #[cfg(any(
-        not(any(target_arch = "x86", target_arch = "x86_64")),
-        not(feature = "simd")
-    ))]
     compare_op!(left, right, |a, b| a > b)
 }
 
@@ -349,13 +247,6 @@ pub fn gt_eq<T>(
 where
     T: ArrowNumericType,
 {
-    #[cfg(all(any(target_arch = "x86", target_arch = "x86_64"), feature = "simd"))]
-    return simd_compare_op(left, right, T::ge);
-
-    #[cfg(any(
-        not(any(target_arch = "x86", target_arch = "x86_64")),
-        not(feature = "simd")
-    ))]
     compare_op!(left, right, |a, b| a >= b)
 }
 
diff --git a/rust/arrow/src/compute/util.rs b/rust/arrow/src/compute/util.rs
index 03930b696bb..f60dd36f3df 100644
--- a/rust/arrow/src/compute/util.rs
+++ b/rust/arrow/src/compute/util.rs
@@ -20,13 +20,7 @@
 use crate::array::*;
 use crate::bitmap::Bitmap;
 use crate::buffer::Buffer;
-#[cfg(feature = "simd")]
-use crate::datatypes::*;
 use crate::error::Result;
-#[cfg(feature = "simd")]
-use num::One;
-#[cfg(feature = "simd")]
-use std::cmp::min;
 
 /// Applies a given binary operation, `op`, to two references to `Option<Bitmap>`'s.
 ///
@@ -93,70 +87,6 @@ pub(super) fn take_value_indices_from_list(
     (UInt32Array::from(values), new_offsets)
 }
 
-/// Creates a new SIMD mask, i.e. `packed_simd::m32x16` or similar. that indicates if the
-/// corresponding array slots represented by the mask are 'valid'.  
-///
-/// Lanes of the SIMD mask can be set to 'valid' (`true`) if the corresponding array slot is not
-/// `NULL`, as indicated by it's `Bitmap`, and is within the length of the array.  Lanes outside the
-/// length represent padding and are set to 'invalid' (`false`).
-#[cfg(all(any(target_arch = "x86", target_arch = "x86_64"), feature = "simd"))]
-unsafe fn is_valid<T>(
-    bitmap: &Option<Bitmap>,
-    i: usize,
-    simd_width: usize,
-    array_len: usize,
-) -> T::SimdMask
-where
-    T: ArrowNumericType,
-{
-    let simd_upper_bound = i + simd_width;
-    let mut validity = T::mask_init(true);
-
-    // Validity based on `Bitmap`
-    if let Some(b) = bitmap {
-        for j in i..min(array_len, simd_upper_bound) {
-            if !b.is_set(j) {
-                validity = T::mask_set(validity, j - i, false);
-            }
-        }
-    }
-
-    // Validity based on the length of the Array
-    for j in array_len..simd_upper_bound {
-        validity = T::mask_set(validity, j - i, false);
-    }
-
-    validity
-}
-
-/// Performs a SIMD load but sets all 'invalid' lanes to a constant value.
-///
-/// 'invalid' lanes are lanes where the corresponding array slots are either `NULL` or between the
-/// length and capacity of the array, i.e. in the padded region.
-///
-/// Note that `array` below has it's own `Bitmap` separate from the `bitmap` argument.  This
-/// function is used to prepare `array`'s for binary operations.  The `bitmap` argument is the
-/// `Bitmap` after the binary operation.
-#[cfg(all(any(target_arch = "x86", target_arch = "x86_64"), feature = "simd"))]
-pub(super) unsafe fn simd_load_set_invalid<T>(
-    array: &PrimitiveArray<T>,
-    bitmap: &Option<Bitmap>,
-    i: usize,
-    simd_width: usize,
-    fill_value: T::Native,
-) -> T::Simd
-where
-    T: ArrowNumericType,
-    T::Native: One,
-{
-    let simd_with_zeros = T::load(array.value_slice(i, simd_width));
-    T::mask_select(
-        is_valid::<T>(bitmap, i, simd_width, array.len()),
-        simd_with_zeros,
-        T::init(fill_value),
-    )
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -224,52 +154,4 @@ mod tests {
         .data();
         assert_eq!(data, indexed.data());
     }
-
-    #[test]
-    #[cfg(all(any(target_arch = "x86", target_arch = "x86_64"), feature = "simd"))]
-    fn test_is_valid() {
-        let a = Int32Array::from(vec![
-            Some(15),
-            None,
-            None,
-            Some(1),
-            None,
-            None,
-            Some(5),
-            None,
-            None,
-            Some(4),
-        ]);
-        let simd_lanes = 16;
-        let data = a.data();
-        let bitmap = data.null_bitmap();
-        let result = unsafe { is_valid::<Int32Type>(&bitmap, 0, simd_lanes, a.len()) };
-        for i in 0..simd_lanes {
-            if i % 3 != 0 || i > 9 {
-                assert_eq!(false, result.extract(i));
-            } else {
-                assert_eq!(true, result.extract(i));
-            }
-        }
-    }
-
-    #[test]
-    #[cfg(all(any(target_arch = "x86", target_arch = "x86_64"), feature = "simd"))]
-    fn test_simd_load_set_invalid() {
-        let a = Int64Array::from(vec![None, Some(15), Some(5), Some(0)]);
-        let new_bitmap = &Some(Bitmap::from(Buffer::from([0b00001010])));
-        let simd_lanes = 8;
-        let result = unsafe {
-            simd_load_set_invalid::<Int64Type>(&a, &new_bitmap, 0, simd_lanes, 1)
-        };
-        for i in 0..simd_lanes {
-            if i == 1 {
-                assert_eq!(15_i64, result.extract(i));
-            } else if i == 3 {
-                assert_eq!(0_i64, result.extract(i));
-            } else {
-                assert_eq!(1_i64, result.extract(i));
-            }
-        }
-    }
 }
diff --git a/rust/arrow/src/datatypes.rs b/rust/arrow/src/datatypes.rs
index ce6f2d26ecd..73b3949ab42 100644
--- a/rust/arrow/src/datatypes.rs
+++ b/rust/arrow/src/datatypes.rs
@@ -25,14 +25,10 @@
 use std::collections::HashMap;
 use std::fmt;
 use std::mem::size_of;
-#[cfg(feature = "simd")]
-use std::ops::{Add, Div, Mul, Sub};
 use std::slice::from_raw_parts;
 use std::str::FromStr;
 use std::sync::Arc;
 
-#[cfg(feature = "simd")]
-use packed_simd::*;
 use serde_derive::{Deserialize, Serialize};
 use serde_json::{
     json, Number, Value, Value::Number as VNumber, Value::String as VString,
@@ -488,167 +484,10 @@ impl ArrowDictionaryKeyType for Int32Type {}
 
 impl ArrowDictionaryKeyType for Int64Type {}
 
-/// A subtype of primitive type that represents numeric values.
-///
-/// SIMD operations are defined in this trait if available on the target system.
-#[cfg(all(any(target_arch = "x86", target_arch = "x86_64"), feature = "simd"))]
-pub trait ArrowNumericType: ArrowPrimitiveType
-where
-    Self::Simd: Add<Output = Self::Simd>
-        + Sub<Output = Self::Simd>
-        + Mul<Output = Self::Simd>
-        + Div<Output = Self::Simd>,
-{
-    /// Defines the SIMD type that should be used for this numeric type
-    type Simd;
-
-    /// Defines the SIMD Mask type that should be used for this numeric type
-    type SimdMask;
-
-    /// The number of SIMD lanes available
-    fn lanes() -> usize;
-
-    /// Initializes a SIMD register to a constant value
-    fn init(value: Self::Native) -> Self::Simd;
-
-    /// Loads a slice into a SIMD register
-    fn load(slice: &[Self::Native]) -> Self::Simd;
-
-    /// Creates a new SIMD mask for this SIMD type filling it with `value`
-    fn mask_init(value: bool) -> Self::SimdMask;
-
-    /// Gets the value of a single lane in a SIMD mask
-    fn mask_get(mask: &Self::SimdMask, idx: usize) -> bool;
-
-    /// Sets the value of a single lane of a SIMD mask
-    fn mask_set(mask: Self::SimdMask, idx: usize, value: bool) -> Self::SimdMask;
-
-    /// Selects elements of `a` and `b` using `mask`
-    fn mask_select(mask: Self::SimdMask, a: Self::Simd, b: Self::Simd) -> Self::Simd;
-
-    /// Returns `true` if any of the lanes in the mask are `true`
-    fn mask_any(mask: Self::SimdMask) -> bool;
-
-    /// Performs a SIMD binary operation
-    fn bin_op<F: Fn(Self::Simd, Self::Simd) -> Self::Simd>(
-        left: Self::Simd,
-        right: Self::Simd,
-        op: F,
-    ) -> Self::Simd;
-
-    /// SIMD version of equal
-    fn eq(left: Self::Simd, right: Self::Simd) -> Self::SimdMask;
-
-    /// SIMD version of not equal
-    fn ne(left: Self::Simd, right: Self::Simd) -> Self::SimdMask;
-
-    /// SIMD version of less than
-    fn lt(left: Self::Simd, right: Self::Simd) -> Self::SimdMask;
-
-    /// SIMD version of less than or equal to
-    fn le(left: Self::Simd, right: Self::Simd) -> Self::SimdMask;
-
-    /// SIMD version of greater than
-    fn gt(left: Self::Simd, right: Self::Simd) -> Self::SimdMask;
-
-    /// SIMD version of greater than or equal to
-    fn ge(left: Self::Simd, right: Self::Simd) -> Self::SimdMask;
-
-    /// Writes a SIMD result back to a slice
-    fn write(simd_result: Self::Simd, slice: &mut [Self::Native]);
-}
-
-#[cfg(any(
-    not(any(target_arch = "x86", target_arch = "x86_64")),
-    not(feature = "simd")
-))]
 pub trait ArrowNumericType: ArrowPrimitiveType {}
 
 macro_rules! make_numeric_type {
     ($impl_ty:ty, $native_ty:ty, $simd_ty:ident, $simd_mask_ty:ident) => {
-        #[cfg(all(any(target_arch = "x86", target_arch = "x86_64"), feature = "simd"))]
-        impl ArrowNumericType for $impl_ty {
-            type Simd = $simd_ty;
-
-            type SimdMask = $simd_mask_ty;
-
-            fn lanes() -> usize {
-                Self::Simd::lanes()
-            }
-
-            fn init(value: Self::Native) -> Self::Simd {
-                Self::Simd::splat(value)
-            }
-
-            fn load(slice: &[Self::Native]) -> Self::Simd {
-                unsafe { Self::Simd::from_slice_unaligned_unchecked(slice) }
-            }
-
-            fn mask_init(value: bool) -> Self::SimdMask {
-                Self::SimdMask::splat(value)
-            }
-
-            fn mask_get(mask: &Self::SimdMask, idx: usize) -> bool {
-                unsafe { mask.extract_unchecked(idx) }
-            }
-
-            fn mask_set(mask: Self::SimdMask, idx: usize, value: bool) -> Self::SimdMask {
-                unsafe { mask.replace_unchecked(idx, value) }
-            }
-
-            /// Selects elements of `a` and `b` using `mask`
-            fn mask_select(
-                mask: Self::SimdMask,
-                a: Self::Simd,
-                b: Self::Simd,
-            ) -> Self::Simd {
-                mask.select(a, b)
-            }
-
-            fn mask_any(mask: Self::SimdMask) -> bool {
-                mask.any()
-            }
-
-            fn bin_op<F: Fn(Self::Simd, Self::Simd) -> Self::Simd>(
-                left: Self::Simd,
-                right: Self::Simd,
-                op: F,
-            ) -> Self::Simd {
-                op(left, right)
-            }
-
-            fn eq(left: Self::Simd, right: Self::Simd) -> Self::SimdMask {
-                left.eq(right)
-            }
-
-            fn ne(left: Self::Simd, right: Self::Simd) -> Self::SimdMask {
-                left.ne(right)
-            }
-
-            fn lt(left: Self::Simd, right: Self::Simd) -> Self::SimdMask {
-                left.lt(right)
-            }
-
-            fn le(left: Self::Simd, right: Self::Simd) -> Self::SimdMask {
-                left.le(right)
-            }
-
-            fn gt(left: Self::Simd, right: Self::Simd) -> Self::SimdMask {
-                left.gt(right)
-            }
-
-            fn ge(left: Self::Simd, right: Self::Simd) -> Self::SimdMask {
-                left.ge(right)
-            }
-
-            fn write(simd_result: Self::Simd, slice: &mut [Self::Native]) {
-                unsafe { simd_result.write_to_slice_unaligned_unchecked(slice) };
-            }
-        }
-        #[cfg(any(
-            not(any(target_arch = "x86", target_arch = "x86_64")),
-            not(feature = "simd")
-        ))]
         impl ArrowNumericType for $impl_ty {}
     };
 }
diff --git a/rust/arrow/src/util/bit_util.rs b/rust/arrow/src/util/bit_util.rs
index e5767091387..5f6878a7ff2 100644
--- a/rust/arrow/src/util/bit_util.rs
+++ b/rust/arrow/src/util/bit_util.rs
@@ -17,9 +17,6 @@
 
 //! Utils for working with bits
 
-#[cfg(feature = "simd")]
-use packed_simd::u8x64;
-
 static BIT_MASK: [u8; 8] = [1, 2, 4, 8, 16, 32, 64, 128];
 
 static POPCOUNT_TABLE: [u8; 256] = [
@@ -155,22 +152,6 @@ pub fn ceil(value: usize, divisor: usize) -> usize {
     result
 }
 
-/// Performs SIMD bitwise binary operations.
-///
-/// Note that each slice should be 64 bytes and it is the callers responsibility to ensure
-/// that this is the case.  If passed slices larger than 64 bytes the operation will only
-/// be performed on the first 64 bytes.  Slices less than 64 bytes will panic.
-#[cfg(all(any(target_arch = "x86", target_arch = "x86_64"), feature = "simd"))]
-pub unsafe fn bitwise_bin_op_simd<F>(left: &[u8], right: &[u8], result: &mut [u8], op: F)
-where
-    F: Fn(u8x64, u8x64) -> u8x64,
-{
-    let left_simd = u8x64::from_slice_unaligned_unchecked(left);
-    let right_simd = u8x64::from_slice_unaligned_unchecked(right);
-    let simd_result = op(left_simd, right_simd);
-    simd_result.write_to_slice_unaligned_unchecked(result);
-}
-
 #[cfg(test)]
 mod tests {
     use rand::{thread_rng, Rng};
@@ -361,28 +342,4 @@ mod tests {
         assert_eq!(ceil(10, 10000000000), 1);
         assert_eq!(ceil(10000000000, 1000000000), 10);
     }
-
-    #[test]
-    #[cfg(all(any(target_arch = "x86", target_arch = "x86_64"), feature = "simd"))]
-    fn test_bitwise_and_simd() {
-        let buf1 = [0b00110011u8; 64];
-        let buf2 = [0b11110000u8; 64];
-        let mut buf3 = [0b00000000; 64];
-        unsafe { bitwise_bin_op_simd(&buf1, &buf2, &mut buf3, |a, b| a & b) };
-        for i in buf3.iter() {
-            assert_eq!(&0b00110000u8, i);
-        }
-    }
-
-    #[test]
-    #[cfg(all(any(target_arch = "x86", target_arch = "x86_64"), feature = "simd"))]
-    fn test_bitwise_or_simd() {
-        let buf1 = [0b00110011u8; 64];
-        let buf2 = [0b11110000u8; 64];
-        let mut buf3 = [0b00000000; 64];
-        unsafe { bitwise_bin_op_simd(&buf1, &buf2, &mut buf3, |a, b| a | b) };
-        for i in buf3.iter() {
-            assert_eq!(&0b11110011u8, i);
-        }
-    }
 }

From 7fcd0481bb7cf8441dca7809fc437b75978a6566 Mon Sep 17 00:00:00 2001
From: Neville Dipale <nevilledips@gmail.com>
Date: Sat, 2 May 2020 10:38:46 +0200
Subject: [PATCH 2/2] use slow divide path if RHS has nulls

---
 rust/arrow/src/compute/kernels/arithmetic.rs | 27 +++++++++++++++-----
 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/rust/arrow/src/compute/kernels/arithmetic.rs b/rust/arrow/src/compute/kernels/arithmetic.rs
index c56a5d2a3ff..82ea906b8b0 100644
--- a/rust/arrow/src/compute/kernels/arithmetic.rs
+++ b/rust/arrow/src/compute/kernels/arithmetic.rs
@@ -137,13 +137,28 @@ where
         + Zero
         + One,
 {
-    math_op(left, right, |a, b| {
-        if b.is_zero() {
-            Err(ArrowError::DivideByZero)
-        } else {
-            Ok(a / b)
+    if right.null_count() == 0 {
+        math_op(left, right, |a, b| {
+            if b.is_zero() {
+                Err(ArrowError::DivideByZero)
+            } else {
+                Ok(a / b)
+            }
+        })
+    } else {
+        // check for nulls, then check if RHS is zero
+        let mut b = PrimitiveBuilder::<T>::new(left.len());
+        for i in 0..left.len() {
+            if left.is_null(i) || right.is_null(i) {
+                b.append_null()?;
+            } else if right.value(i).is_zero() {
+                return Err(ArrowError::DivideByZero);
+            } else {
+                b.append_value(left.value(i) / right.value(i))?;
+            }
         }
-    })
+        Ok(b.finish())
+    }
 }
 
 #[cfg(test)]
