apache · emkornfield · Sep 4, 2020 · Sep 13, 2020 · Sep 13, 2020 · Sep 13, 2020
diff --git a/cpp/cmake_modules/SetupCxxFlags.cmake b/cpp/cmake_modules/SetupCxxFlags.cmake
@@ -68,10 +68,10 @@ if(ARROW_CPU_FLAG STREQUAL "x86")
     add_definitions(-DARROW_HAVE_RUNTIME_SSE4_2)
   endif()
   if(CXX_SUPPORTS_AVX2 AND ARROW_RUNTIME_SIMD_LEVEL MATCHES "^(AVX2|AVX512|MAX)$")
-    add_definitions(-DARROW_HAVE_RUNTIME_AVX2)
+    add_definitions(-DARROW_HAVE_RUNTIME_AVX2 -DARROW_HAVE_RUNTIME_BMI2)
   endif()
   if(CXX_SUPPORTS_AVX512 AND ARROW_RUNTIME_SIMD_LEVEL MATCHES "^(AVX512|MAX)$")
-    add_definitions(-DARROW_HAVE_RUNTIME_AVX512)
+    add_definitions(-DARROW_HAVE_RUNTIME_AVX512 -DARROW_HAVE_RUNTIME_BMI2)
   endif()
 elseif(ARROW_CPU_FLAG STREQUAL "ppc")
   # power compiler flags, gcc/clang only

diff --git a/cpp/src/arrow/buffer.cc b/cpp/src/arrow/buffer.cc
@@ -206,7 +206,7 @@ class PoolBuffer : public ResizableBuffer {
   }
 
   Status Resize(const int64_t new_size, bool shrink_to_fit = true) override {
-    if (new_size < 0) {
+    if (ARROW_PREDICT_FALSE(new_size < 0)) {
       return Status::Invalid("Negative buffer resize: ", new_size);
     }
     if (mutable_data_ && shrink_to_fit && new_size <= size_) {

diff --git a/cpp/src/arrow/util/cpu_info.h b/cpp/src/arrow/util/cpu_info.h
@@ -106,6 +106,11 @@ class ARROW_EXPORT CpuInfo {
   /// Returns the vendor of the cpu.
   Vendor vendor() const { return vendor_; }
 
+  bool HasEfficientBmi2() const {
+    // BMI2 (pext, pdep) is only efficient on Intel X86 processors.
+    return vendor() == Vendor::Intel && IsSupported(BMI2);
+  }
+
  private:
   CpuInfo();
 

diff --git a/cpp/src/arrow/util/simd.h b/cpp/src/arrow/util/simd.h
@@ -29,6 +29,10 @@
 #else
 // gcc/clang (possibly others)
 
+#if defined(ARROW_HAVE_BMI2)
+#include <x86intrin.h>
+#endif
+
 #if defined(ARROW_HAVE_AVX2) || defined(ARROW_HAVE_AVX512)
 #include <immintrin.h>
 #elif defined(ARROW_HAVE_SSE4_2)

diff --git a/cpp/src/parquet/CMakeLists.txt b/cpp/src/parquet/CMakeLists.txt
@@ -188,6 +188,7 @@ set(PARQUET_SRCS
     file_writer.cc
     internal_file_decryptor.cc
     internal_file_encryptor.cc
+    level_comparison.cc
     level_conversion.cc
     metadata.cc
     murmur3.cc
@@ -202,6 +203,27 @@ set(PARQUET_SRCS
     stream_writer.cc
     types.cc)
 
+if(CXX_SUPPORTS_AVX2)
+  # AVX2 is used as a proxy for BMI2.
+  list(APPEND PARQUET_SRCS level_comparison_avx2.cc level_conversion_bmi2.cc)
+  set_source_files_properties(level_comparison_avx2.cc
+                              PROPERTIES
+                              SKIP_PRECOMPILE_HEADERS
+                              ON
+                              COMPILE_FLAGS
+                              "${ARROW_AVX2_FLAG}")
+  # WARNING: DO NOT BLINDLY COPY THIS CODE FOR OTHER BMI2 USE CASES.
+  # This code is always guarded by runtime dispatch which verifies
+  # BMI2 is present.  For a very small number of CPUs AVX2 does not
+  # imply BMI2.
+  set_source_files_properties(level_conversion_bmi2.cc
+                              PROPERTIES
+                              SKIP_PRECOMPILE_HEADERS
+                              ON
+                              COMPILE_FLAGS
+                              "${ARROW_AVX2_FLAG} -DARROW_HAVE_BMI2 -mbmi2")
+endif()
+
 if(PARQUET_REQUIRE_ENCRYPTION)
   set(PARQUET_SRCS ${PARQUET_SRCS} encryption_internal.cc)
 else()

diff --git a/cpp/src/parquet/arrow/arrow_reader_writer_test.cc b/cpp/src/parquet/arrow/arrow_reader_writer_test.cc
@@ -416,7 +416,6 @@ void DoSimpleRoundtrip(const std::shared_ptr<Table>& table, bool use_threads,
                               ::arrow::default_memory_pool(), &reader));
 
   reader->set_use_threads(use_threads);
-
   if (column_subset.size() > 0) {
     ASSERT_OK_NO_THROW(reader->ReadTable(column_subset, out));
   } else {
@@ -2361,8 +2360,7 @@ TEST(ArrowReadWrite, SingleColumnNullableStruct) {
       3);
 }
 
-// Disabled until implementation can be finished.
-TEST(TestArrowReadWrite, DISABLED_CanonicalNestedRoundTrip) {
+TEST(TestArrowReadWrite, CanonicalNestedRoundTrip) {
   auto doc_id = field("DocId", ::arrow::int64(), /*nullable=*/false);
   auto links = field(
       "Links",
@@ -2391,7 +2389,7 @@ TEST(TestArrowReadWrite, DISABLED_CanonicalNestedRoundTrip) {
   // string literals implemented properly
   auto name_array = ::arrow::ArrayFromJSON(
       name->type(),
-      "([[{\"Language\": [{\"Code\": \"en_us\", \"Country\":\"us\"},"
+      "[[{\"Language\": [{\"Code\": \"en_us\", \"Country\":\"us\"},"
       "{\"Code\": \"en_us\", \"Country\": null}],"
       "\"Url\": \"http://A\"},"
       "{\"Url\": \"http://B\"},"
@@ -2810,12 +2808,6 @@ TEST_F(TestNestedSchemaRead, ReadTablePartial) {
   ASSERT_NO_FATAL_FAILURE(ValidateTableArrayTypes(*table));
 }
 
-TEST_F(TestNestedSchemaRead, StructAndListTogetherUnsupported) {
-  ASSERT_NO_FATAL_FAILURE(CreateSimpleNestedParquet(Repetition::REPEATED));
-  std::shared_ptr<Table> table;
-  ASSERT_RAISES(NotImplemented, reader_->ReadTable(&table));
-}
-
 TEST_P(TestNestedSchemaRead, DeepNestedSchemaRead) {
 #ifdef PARQUET_VALGRIND
   const int num_trees = 3;
@@ -2994,7 +2986,6 @@ TEST_P(TestArrowReaderAdHocSparkAndHvr, ReadDecimals) {
     ASSERT_OK(builder.Append(value));
   }
   ASSERT_OK(builder.Finish(&expected_array));
-
   AssertArraysEqual(*expected_array, *chunk);
 }