kou commented on code in PR #14674:
URL: https://github.com/apache/arrow/pull/14674#discussion_r1043824240


##########
cpp/src/arrow/util/cpu_info.cc:
##########
@@ -363,6 +363,10 @@ int64_t LinuxParseCpuFlags(const std::string& values) {
     {"avx512bw", CpuInfo::AVX512BW},
     {"bmi1", CpuInfo::BMI1},
     {"bmi2", CpuInfo::BMI2},
+    {"avx512ifma", CpuInfo::AVX512IFMA},
+    {"avx512vbmi", CpuInfo::AVX512VBMI},
+    {"avx512_vbmi2", CpuInfo::AVX512VBMI2},
+    {"avx512_vnni", CpuInfo::AVX512VNNI},

Review Comment:
   ```suggestion
       {"avx512vbmi2", CpuInfo::AVX512VBMI2},
       {"avx512vnni", CpuInfo::AVX512VNNI},
   ```



##########
cpp/src/arrow/util/cpu_info.h:
##########
@@ -53,7 +53,11 @@ class ARROW_EXPORT CpuInfo {
   static constexpr int64_t AVX512 = AVX512F | AVX512CD | AVX512VL | AVX512DQ | 
AVX512BW;
   static constexpr int64_t BMI1 = (1LL << 11);
   static constexpr int64_t BMI2 = (1LL << 12);
-
+  static constexpr int64_t AVX512IFMA = (1LL << 13);
+  static constexpr int64_t AVX512VBMI = (1LL << 14);
+  static constexpr int64_t AVX512VBMI2 = (1LL << 15);
+  static constexpr int64_t AVX512VNNI = (1LL << 16);
+  static constexpr int64_t AVX512_ICX = AVX512IFMA | AVX512VBMI | AVX512VNNI;

Review Comment:
   Should this also include `AVX512VBMI2`?



##########
cpp/src/arrow/util/bpacking.cc:
##########
@@ -29,6 +29,9 @@
 #if defined(ARROW_HAVE_RUNTIME_AVX512)
 #include "arrow/util/bpacking_avx512.h"
 #endif
+#if defined(ARROW_HAVE_AVX512_ICX)

Review Comment:
   ```suggestion
   #if defined(ARROW_HAVE_RUNTIME_AVX512_ICX)
   ```
   
   We need to define `ARROW_HAVE_RUNTIME_AVX512_ICX` in 
`cpp/cmake_modules/SetupCxxFlags.cmake`.



##########
cpp/src/arrow/util/bpacking.cc:
##########
@@ -166,14 +169,34 @@ struct Unpack32DynamicFunction {
   }
 };
 
+#if defined(ARROW_HAVE_AVX512_ICX)
+struct Unpack32DynamicFunctionICX {
+  using FunctionType = decltype(&unpack32_default);
+
+  static std::vector<std::pair<DispatchLevel, FunctionType>> implementations() 
{
+    return {{DispatchLevel::NONE, unpack32_default},
+            {DispatchLevel::AVX512_ICX, unpack32_avx512_icx}};
+  }
+};

Review Comment:
   We can't reuse `Unpack32DynamicFunction` for `AVX512_ICX` because 
`unpack32_avx512_icx()` doesn't support `num_bits > 16`, right?
   Can we use `unpack32_specialized()` for `num_bits > 16` like other 
`unpack32_*()`?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to