Code review fixes

sterrettm2 · sterrettm2 · commit 1834edc8a6c5 · 2024-01-12T10:41:47.000-08:00
diff --git a/src/avx2-32bit-half.hpp b/src/avx2-32bit-half.hpp
@@ -7,7 +7,7 @@
 #ifndef AVX2_HALF_32BIT
 #define AVX2_HALF_32BIT
 
-#include "xss-common-qsort.h"
+#include "xss-common-includes.h"
 #include "avx2-emu-funcs.hpp"
 
 /*
@@ -46,7 +46,7 @@ template <>
 struct avx2_half_vector<int32_t> {
     using type_t = int32_t;
     using reg_t = __m128i;
-    using ymmi_t = __m128i;
+    using regi_t = __m128i;
     using opmask_t = __m128i;
     static const uint8_t numlanes = 4;
     static constexpr simd_type vec_type = simd_type::AVX2;
@@ -70,7 +70,7 @@ struct avx2_half_vector<int32_t> {
         auto mask = ((0x1ull << num_to_read) - 0x1ull);
         return convert_int_to_avx2_mask_half(mask);
     }
-    static ymmi_t seti(int v1, int v2, int v3, int v4)
+    static regi_t seti(int v1, int v2, int v3, int v4)
     {
         return _mm_set_epi32(v1, v2, v3, v4);
     }
@@ -86,8 +86,7 @@ struct avx2_half_vector<int32_t> {
     {
         opmask_t equal = eq(x, y);
         opmask_t greater = _mm_cmpgt_epi32(x, y);
-        return _mm_castps_si128(
-                _mm_or_ps(_mm_castsi128_ps(equal), _mm_castsi128_ps(greater)));
+        return _mm_or_si128(equal, greater);
     }
     static opmask_t eq(reg_t x, reg_t y)
     {
@@ -150,10 +149,6 @@ struct avx2_half_vector<int32_t> {
     {
         return _mm_castps_si128(_mm_permutevar_ps(_mm_castsi128_ps(ymm), idx));
     }
-    static reg_t permutevar(reg_t ymm, __m128i idx)
-    {
-        return _mm_castps_si128(_mm_permutevar_ps(_mm_castsi128_ps(ymm), idx));
-    }
     static reg_t reverse(reg_t ymm)
     {
         const __m128i rev_index = _mm_set_epi32(0, 1, 2, 3);
@@ -205,7 +200,7 @@ template <>
 struct avx2_half_vector<uint32_t> {
     using type_t = uint32_t;
     using reg_t = __m128i;
-    using ymmi_t = __m128i;
+    using regi_t = __m128i;
     using opmask_t = __m128i;
     static const uint8_t numlanes = 4;
     static constexpr simd_type vec_type = simd_type::AVX2;
@@ -229,7 +224,7 @@ struct avx2_half_vector<uint32_t> {
         auto mask = ((0x1ull << num_to_read) - 0x1ull);
         return convert_int_to_avx2_mask_half(mask);
     }
-    static ymmi_t seti(int v1, int v2, int v3, int v4)
+    static regi_t seti(int v1, int v2, int v3, int v4)
     {
         return _mm_set_epi32(v1, v2, v3, v4);
     }
@@ -299,10 +294,6 @@ struct avx2_half_vector<uint32_t> {
     {
         return _mm_castps_si128(_mm_permutevar_ps(_mm_castsi128_ps(ymm), idx));
     }
-    static reg_t permutevar(reg_t ymm, __m128i idx)
-    {
-        return _mm_castps_si128(_mm_permutevar_ps(_mm_castsi128_ps(ymm), idx));
-    }
     static reg_t reverse(reg_t ymm)
     {
         const __m128i rev_index = _mm_set_epi32(0, 1, 2, 3);
@@ -354,7 +345,7 @@ template <>
 struct avx2_half_vector<float> {
     using type_t = float;
     using reg_t = __m128;
-    using ymmi_t = __m128i;
+    using regi_t = __m128i;
     using opmask_t = __m128i;
     static const uint8_t numlanes = 4;
     static constexpr simd_type vec_type = simd_type::AVX2;
@@ -374,7 +365,7 @@ struct avx2_half_vector<float> {
         return _mm_set1_ps(type_max());
     }
 
-    static ymmi_t seti(int v1, int v2, int v3, int v4)
+    static regi_t seti(int v1, int v2, int v3, int v4)
     {
         return _mm_set_epi32(v1, v2, v3, v4);
     }
@@ -464,10 +455,6 @@ struct avx2_half_vector<float> {
     {
         return _mm_permutevar_ps(ymm, idx);
     }
-    static reg_t permutevar(reg_t ymm, __m128i idx)
-    {
-        return _mm_permutevar_ps(ymm, idx);
-    }
     static reg_t reverse(reg_t ymm)
     {
         const __m128i rev_index = _mm_set_epi32(0, 1, 2, 3);
@@ -520,23 +507,15 @@ struct avx2_32bit_half_swizzle_ops {
     template <typename vtype, int scale>
     X86_SIMD_SORT_INLINE typename vtype::reg_t swap_n(typename vtype::reg_t reg)
     {
-        __m128i v = vtype::cast_to(reg);
-
         if constexpr (scale == 2) {
-            __m128 vf = _mm_castsi128_ps(v);
-            vf = _mm_permute_ps(vf, 0b10110001);
-            v = _mm_castps_si128(vf);
+            return vtype::template shuffle<0b10110001>(reg);
         }
         else if constexpr (scale == 4) {
-            __m128 vf = _mm_castsi128_ps(v);
-            vf = _mm_permute_ps(vf, 0b01001110);
-            v = _mm_castps_si128(vf);
+            return vtype::template shuffle<0b01001110>(reg);
         }
         else {
             static_assert(scale == -1, "should not be reached");
         }
-
-        return vtype::cast_from(v);
     }
 
     template <typename vtype, int scale>
diff --git a/src/avx2-emu-funcs.hpp b/src/avx2-emu-funcs.hpp
@@ -277,7 +277,7 @@ void avx2_emu_mask_compressstoreu32(void *base_addr,
     const __m256i &left = _mm256_loadu_si256(
             (const __m256i *)avx2_compressstore_lut32_left[shortMask].data());
 
-    typename vtype::reg_t temp = vtype::permutevar(reg, perm);
+    typename vtype::reg_t temp = vtype::permutexvar(perm, reg);
 
     vtype::mask_storeu(leftStore, left, temp);
 }
@@ -300,7 +300,7 @@ void avx2_emu_mask_compressstoreu32_half(
             (const __m128i *)avx2_compressstore_lut32_half_left[shortMask]
                     .data());
 
-    typename vtype::reg_t temp = vtype::permutevar(reg, perm);
+    typename vtype::reg_t temp = vtype::permutexvar(perm, reg);
 
     vtype::mask_storeu(leftStore, left, temp);
 }
@@ -341,7 +341,7 @@ int avx2_double_compressstore32(void *left_addr,
     const __m256i &perm = _mm256_loadu_si256(
             (const __m256i *)avx2_compressstore_lut32_perm[shortMask].data());
 
-    typename vtype::reg_t temp = vtype::permutevar(reg, perm);
+    typename vtype::reg_t temp = vtype::permutexvar(perm, reg);
 
     vtype::storeu(leftStore, temp);
     vtype::storeu(rightStore, temp);
@@ -365,7 +365,7 @@ int avx2_double_compressstore32_half(void *left_addr,
             (const __m128i *)avx2_compressstore_lut32_half_perm[shortMask]
                     .data());
 
-    typename vtype::reg_t temp = vtype::permutevar(reg, perm);
+    typename vtype::reg_t temp = vtype::permutexvar(perm, reg);
 
     vtype::storeu(leftStore, temp);
     vtype::storeu(rightStore, temp);
diff --git a/src/xss-common-argsort.h b/src/xss-common-argsort.h
@@ -64,20 +64,6 @@ std_argsort(T *arr, arrsize_t *arg, arrsize_t left, arrsize_t right)
               });
 }
 
-/* Workaround for NumPy failed build on macOS x86_64: implicit instantiation of
- * undefined template 'zmm_vector<unsigned long>'*/
-#ifdef __APPLE__
-using argtypeAVX512 =
-        typename std::conditional<sizeof(arrsize_t) == sizeof(int32_t),
-                                  ymm_vector<uint32_t>,
-                                  zmm_vector<uint64_t>>::type;
-#else
-using argtypeAVX512 =
-        typename std::conditional<sizeof(arrsize_t) == sizeof(int32_t),
-                                  ymm_vector<arrsize_t>,
-                                  zmm_vector<arrsize_t>>::type;
-#endif
-
 /*
  * Parition one ZMM register based on the pivot and returns the index of the
  * last element that is less than equal to the pivot.
@@ -129,7 +115,7 @@ X86_SIMD_SORT_INLINE int32_t partition_vec_avx2(type_t *arg,
     /* which elements are larger than the pivot */
     typename vtype::opmask_t ge_mask_vtype = vtype::ge(curr_vec, pivot_vec);
     typename argtype::opmask_t ge_mask
-            = extend_mask<vtype, argtype>(ge_mask_vtype);
+            = resize_mask<vtype, argtype>(ge_mask_vtype);
 
     auto l_store = arg + left;
     auto r_store = arg + right - vtype::numlanes;
@@ -727,19 +713,4 @@ avx2_argselect(T *arr, arrsize_t k, arrsize_t arrsize, bool hasnan = false)
     return indices;
 }
 
-/* To maintain compatibility with NumPy build */
-template <typename T>
-X86_SIMD_SORT_INLINE void
-avx512_argselect(T *arr, int64_t *arg, arrsize_t k, arrsize_t arrsize)
-{
-    avx512_argselect(arr, reinterpret_cast<arrsize_t *>(arg), k, arrsize);
-}
-
-template <typename T>
-X86_SIMD_SORT_INLINE void
-avx512_argsort(T *arr, int64_t *arg, arrsize_t arrsize)
-{
-    avx512_argsort(arr, reinterpret_cast<arrsize_t *>(arg), arrsize);
-}
-
 #endif // XSS_COMMON_ARGSORT
diff --git a/src/xss-network-keyvaluesort.hpp b/src/xss-network-keyvaluesort.hpp
@@ -10,7 +10,7 @@
 #define NETWORK_32BIT_7 7, 6, 5, 4, 3, 2, 1, 0, 15, 14, 13, 12, 11, 10, 9, 8
 
 template <typename keyType, typename valueType>
-typename valueType::opmask_t extend_mask(typename keyType::opmask_t mask)
+typename valueType::opmask_t resize_mask(typename keyType::opmask_t mask)
 {
     using inT = typename keyType::opmask_t;
     using outT = typename valueType::opmask_t;
@@ -48,7 +48,7 @@ COEX(reg_t1 &key1, reg_t1 &key2, reg_t2 &index1, reg_t2 &index2)
     reg_t1 key_t1 = vtype1::min(key1, key2);
     reg_t1 key_t2 = vtype1::max(key1, key2);
 
-    auto eqMask = extend_mask<vtype1, vtype2>(vtype1::eq(key_t1, key1));
+    auto eqMask = resize_mask<vtype1, vtype2>(vtype1::eq(key_t1, key1));
 
     reg_t2 index_t1 = vtype2::mask_mov(index2, eqMask, index1);
     reg_t2 index_t2 = vtype2::mask_mov(index1, eqMask, index2);
@@ -73,7 +73,7 @@ X86_SIMD_SORT_INLINE reg_t1 cmp_merge(reg_t1 in1,
     reg_t1 tmp_keys = cmp_merge<vtype1>(in1, in2, mask);
     indexes1 = vtype2::mask_mov(
             indexes2,
-            extend_mask<vtype1, vtype2>(vtype1::eq(tmp_keys, in1)),
+            resize_mask<vtype1, vtype2>(vtype1::eq(tmp_keys, in1)),
             indexes1);
     return tmp_keys; // 0 -> min, 1 -> max
 }
@@ -503,7 +503,7 @@ X86_SIMD_SORT_INLINE void argsort_n_vec(typename keyType::type_t *keys,
     for (int i = numVecs / 2; i < numVecs; i++) {
         indexVecs[i] = indexType::mask_loadu(
                 indexType::zmm_max(),
-                extend_mask<keyType, indexType>(ioMasks[i - numVecs / 2]),
+                resize_mask<keyType, indexType>(ioMasks[i - numVecs / 2]),
                 indices + i * indexType::numlanes);
 
         keyVecs[i] = keyType::template mask_i64gather<sizeof(
@@ -532,7 +532,7 @@ X86_SIMD_SORT_INLINE void argsort_n_vec(typename keyType::type_t *keys,
     for (int i = numVecs / 2, j = 0; i < numVecs; i++, j++) {
         indexType::mask_storeu(
                 indices + i * indexType::numlanes,
-                extend_mask<keyType, indexType>(ioMasks[i - numVecs / 2]),
+                resize_mask<keyType, indexType>(ioMasks[i - numVecs / 2]),
                 indexVecs[i]);
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -7,7 +7,7 @@`
`7`	`7`	`#ifndef AVX2_HALF_32BIT`
`8`	`8`	`#define AVX2_HALF_32BIT`
`9`	`9`
`10`		`-#include "xss-common-qsort.h"`
	`10`	`+#include "xss-common-includes.h"`
`11`	`11`	`#include "avx2-emu-funcs.hpp"`
`12`	`12`
`13`	`13`	`/*`
`@@ -46,7 +46,7 @@ template <>`
`46`	`46`	`struct avx2_half_vector<int32_t> {`
`47`	`47`	`using type_t = int32_t;`
`48`	`48`	`using reg_t = __m128i;`
`49`		`- using ymmi_t = __m128i;`
	`49`	`+ using regi_t = __m128i;`
`50`	`50`	`using opmask_t = __m128i;`
`51`	`51`	`static const uint8_t numlanes = 4;`
`52`	`52`	`static constexpr simd_type vec_type = simd_type::AVX2;`
`@@ -70,7 +70,7 @@ struct avx2_half_vector<int32_t> {`
`70`	`70`	`auto mask = ((0x1ull << num_to_read) - 0x1ull);`
`71`	`71`	`return convert_int_to_avx2_mask_half(mask);`
`72`	`72`	`}`
`73`		`- static ymmi_t seti(int v1, int v2, int v3, int v4)`
	`73`	`+ static regi_t seti(int v1, int v2, int v3, int v4)`
`74`	`74`	`{`
`75`	`75`	`return _mm_set_epi32(v1, v2, v3, v4);`
`76`	`76`	`}`
`@@ -86,8 +86,7 @@ struct avx2_half_vector<int32_t> {`
`86`	`86`	`{`
`87`	`87`	`opmask_t equal = eq(x, y);`
`88`	`88`	`opmask_t greater = _mm_cmpgt_epi32(x, y);`
`89`		`- return _mm_castps_si128(`
`90`		`- _mm_or_ps(_mm_castsi128_ps(equal), _mm_castsi128_ps(greater)));`
	`89`	`+ return _mm_or_si128(equal, greater);`
`91`	`90`	`}`
`92`	`91`	`static opmask_t eq(reg_t x, reg_t y)`
`93`	`92`	`{`
`@@ -150,10 +149,6 @@ struct avx2_half_vector<int32_t> {`
`150`	`149`	`{`
`151`	`150`	`return _mm_castps_si128(_mm_permutevar_ps(_mm_castsi128_ps(ymm), idx));`
`152`	`151`	`}`
`153`		`- static reg_t permutevar(reg_t ymm, __m128i idx)`
`154`		`- {`
`155`		`- return _mm_castps_si128(_mm_permutevar_ps(_mm_castsi128_ps(ymm), idx));`
`156`		`- }`
`157`	`152`	`static reg_t reverse(reg_t ymm)`
`158`	`153`	`{`
`159`	`154`	`const __m128i rev_index = _mm_set_epi32(0, 1, 2, 3);`
`@@ -205,7 +200,7 @@ template <>`
`205`	`200`	`struct avx2_half_vector<uint32_t> {`
`206`	`201`	`using type_t = uint32_t;`
`207`	`202`	`using reg_t = __m128i;`
`208`		`- using ymmi_t = __m128i;`
	`203`	`+ using regi_t = __m128i;`
`209`	`204`	`using opmask_t = __m128i;`
`210`	`205`	`static const uint8_t numlanes = 4;`
`211`	`206`	`static constexpr simd_type vec_type = simd_type::AVX2;`
`@@ -229,7 +224,7 @@ struct avx2_half_vector<uint32_t> {`
`229`	`224`	`auto mask = ((0x1ull << num_to_read) - 0x1ull);`
`230`	`225`	`return convert_int_to_avx2_mask_half(mask);`
`231`	`226`	`}`
`232`		`- static ymmi_t seti(int v1, int v2, int v3, int v4)`
	`227`	`+ static regi_t seti(int v1, int v2, int v3, int v4)`
`233`	`228`	`{`
`234`	`229`	`return _mm_set_epi32(v1, v2, v3, v4);`
`235`	`230`	`}`
`@@ -299,10 +294,6 @@ struct avx2_half_vector<uint32_t> {`
`299`	`294`	`{`
`300`	`295`	`return _mm_castps_si128(_mm_permutevar_ps(_mm_castsi128_ps(ymm), idx));`
`301`	`296`	`}`
`302`		`- static reg_t permutevar(reg_t ymm, __m128i idx)`
`303`		`- {`
`304`		`- return _mm_castps_si128(_mm_permutevar_ps(_mm_castsi128_ps(ymm), idx));`
`305`		`- }`
`306`	`297`	`static reg_t reverse(reg_t ymm)`
`307`	`298`	`{`
`308`	`299`	`const __m128i rev_index = _mm_set_epi32(0, 1, 2, 3);`
`@@ -354,7 +345,7 @@ template <>`
`354`	`345`	`struct avx2_half_vector<float> {`
`355`	`346`	`using type_t = float;`
`356`	`347`	`using reg_t = __m128;`
`357`		`- using ymmi_t = __m128i;`
	`348`	`+ using regi_t = __m128i;`
`358`	`349`	`using opmask_t = __m128i;`
`359`	`350`	`static const uint8_t numlanes = 4;`
`360`	`351`	`static constexpr simd_type vec_type = simd_type::AVX2;`
`@@ -374,7 +365,7 @@ struct avx2_half_vector<float> {`
`374`	`365`	`return _mm_set1_ps(type_max());`
`375`	`366`	`}`
`376`	`367`
`377`		`- static ymmi_t seti(int v1, int v2, int v3, int v4)`
	`368`	`+ static regi_t seti(int v1, int v2, int v3, int v4)`
`378`	`369`	`{`
`379`	`370`	`return _mm_set_epi32(v1, v2, v3, v4);`
`380`	`371`	`}`
`@@ -464,10 +455,6 @@ struct avx2_half_vector<float> {`
`464`	`455`	`{`
`465`	`456`	`return _mm_permutevar_ps(ymm, idx);`
`466`	`457`	`}`
`467`		`- static reg_t permutevar(reg_t ymm, __m128i idx)`
`468`		`- {`
`469`		`- return _mm_permutevar_ps(ymm, idx);`
`470`		`- }`
`471`	`458`	`static reg_t reverse(reg_t ymm)`
`472`	`459`	`{`
`473`	`460`	`const __m128i rev_index = _mm_set_epi32(0, 1, 2, 3);`
`@@ -520,23 +507,15 @@ struct avx2_32bit_half_swizzle_ops {`
`520`	`507`	`template <typename vtype, int scale>`
`521`	`508`	`X86_SIMD_SORT_INLINE typename vtype::reg_t swap_n(typename vtype::reg_t reg)`
`522`	`509`	`{`
`523`		`- __m128i v = vtype::cast_to(reg);`
`524`		`-`
`525`	`510`	`if constexpr (scale == 2) {`
`526`		`- __m128 vf = _mm_castsi128_ps(v);`
`527`		`- vf = _mm_permute_ps(vf, 0b10110001);`
`528`		`- v = _mm_castps_si128(vf);`
	`511`	`+ return vtype::template shuffle<0b10110001>(reg);`
`529`	`512`	`}`
`530`	`513`	`else if constexpr (scale == 4) {`
`531`		`- __m128 vf = _mm_castsi128_ps(v);`
`532`		`- vf = _mm_permute_ps(vf, 0b01001110);`
`533`		`- v = _mm_castps_si128(vf);`
	`514`	`+ return vtype::template shuffle<0b01001110>(reg);`
`534`	`515`	`}`
`535`	`516`	`else {`
`536`	`517`	`static_assert(scale == -1, "should not be reached");`
`537`	`518`	`}`
`538`		`-`
`539`		`- return vtype::cast_from(v);`
`540`	`519`	`}`
`541`	`520`
`542`	`521`	`template <typename vtype, int scale>`