Merge pull request #15 from r-devulap/maint

Raghuveer Devulapalli · web-flow · commit 58501d026a39 · 2023-03-13T14:14:38.000-07:00
MAINT: cleanup code and split tests
diff --git a/Makefile b/Makefile
@@ -10,17 +10,21 @@ TESTOBJS	:= $(filter-out $(TESTDIR)/main.o ,$(TESTOBJS))
 CXXFLAGS	+= -I$(SRCDIR) -I$(UTILS)
 GTESTCFLAGS	= `pkg-config --cflags gtest`
 GTESTLDFLAGS	= `pkg-config --libs gtest`
+MARCHFLAG	= -march=icelake-client -O3
 
 all : test bench
 
+$(UTILS)/cpuinfo.o : $(UTILS)/cpuinfo.cpp
+		$(CXX) $(CXXFLAGS) -c $(UTILS)/cpuinfo.cpp -o $(UTILS)/cpuinfo.o
+
 $(TESTDIR)/%.o : $(TESTDIR)/%.cpp $(SRCS)
-		$(CXX) -march=icelake-client -O3 $(CXXFLAGS) $(GTESTCFLAGS) -c $< -o $@
+		$(CXX) $(CXXFLAGS) $(MARCHFLAG) $(GTESTCFLAGS) -c $< -o $@
 
-test: $(TESTDIR)/main.cpp $(TESTOBJS) $(SRCS)
-		$(CXX) tests/main.cpp $(TESTOBJS) $(CXXFLAGS) $(GTESTLDFLAGS) -o testexe
+test: $(TESTDIR)/main.cpp $(TESTOBJS) $(UTILS)/cpuinfo.o $(SRCS)
+		$(CXX) tests/main.cpp $(TESTOBJS) $(UTILS)/cpuinfo.o $(MARCHFLAG) $(CXXFLAGS) $(GTESTLDFLAGS) -o testexe
 
-bench: $(BENCHDIR)/main.cpp $(SRCS)
-		$(CXX) $(BENCHDIR)/main.cpp $(CXXFLAGS) -march=icelake-client -O3 -o benchexe
+bench: $(BENCHDIR)/main.cpp $(SRCS) $(UTILS)/cpuinfo.o
+		$(CXX) $(BENCHDIR)/main.cpp $(CXXFLAGS) $(UTILS)/cpuinfo.o $(MARCHFLAG) -o benchexe
 
 clean:
-		rm -f $(TESTDIR)/*.o testexe benchexe
+		rm -f $(TESTDIR)/*.o testexe benchexe
diff --git a/src/avx512-64bit-keyvaluesort.hpp b/src/avx512-64bit-keyvaluesort.hpp
@@ -803,11 +803,6 @@ void heap_sort(type_t *keys, uint64_t *indexes, int64_t size)
     }
 }
 
-template <typename T>
-struct sortkv_t {
-    T key;
-    uint64_t value;
-};
 template <typename vtype, typename type_t>
 void qsort_64bit_(type_t *keys,
                   uint64_t *indexes,
diff --git a/tests/main.cpp b/tests/main.cpp
@@ -3,6 +3,10 @@
  * * SPDX-License-Identifier: BSD-3-Clause
  * *******************************************/
 
+#include "avx512-16bit-qsort.hpp"
+#include "avx512-32bit-qsort.hpp"
+#include "avx512-64bit-keyvaluesort.hpp"
+#include "avx512-64bit-qsort.hpp"
 #include <gtest/gtest.h>
 
 int main(int argc, char **argv)
diff --git a/tests/test_keyvalue.cpp b/tests/test_keyvalue.cpp
@@ -3,66 +3,17 @@
  * * SPDX-License-Identifier: BSD-3-Clause
  * *******************************************/
 
-#include "avx512-16bit-qsort.hpp"
-#include "avx512-32bit-qsort.hpp"
-#include "avx512-64bit-keyvaluesort.hpp"
-#include "avx512-64bit-qsort.hpp"
-#include "cpuinfo.h"
+#include "avx512-common-keyvaluesort.h"
 #include "rand_array.h"
 #include <gtest/gtest.h>
 #include <vector>
 
-template <typename T>
-class avx512_sort : public ::testing::Test {
-};
-TYPED_TEST_SUITE_P(avx512_sort);
-
-TYPED_TEST_P(avx512_sort, test_arrsizes)
-{
-    if (cpu_has_avx512bw()) {
-        if ((sizeof(TypeParam) == 2) && (!cpu_has_avx512_vbmi2())) {
-            GTEST_SKIP() << "Skipping this test, it requires avx512_vbmi2";
-        }
-        std::vector<int64_t> arrsizes;
-        for (int64_t ii = 0; ii < 1024; ++ii) {
-            arrsizes.push_back((TypeParam)ii);
-        }
-        std::vector<TypeParam> arr;
-        std::vector<TypeParam> sortedarr;
-        for (size_t ii = 0; ii < arrsizes.size(); ++ii) {
-            /* Random array */
-            arr = get_uniform_rand_array<TypeParam>(arrsizes[ii]);
-            sortedarr = arr;
-            /* Sort with std::sort for comparison */
-            std::sort(sortedarr.begin(), sortedarr.end());
-            avx512_qsort<TypeParam>(arr.data(), arr.size());
-            ASSERT_EQ(sortedarr, arr);
-            arr.clear();
-            sortedarr.clear();
-        }
-    }
-    else {
-        GTEST_SKIP() << "Skipping this test, it requires avx512bw";
-    }
-}
-
-REGISTER_TYPED_TEST_SUITE_P(avx512_sort, test_arrsizes);
-
-using Types = testing::Types<uint16_t,
-                             int16_t,
-                             float,
-                             double,
-                             uint32_t,
-                             int32_t,
-                             uint64_t,
-                             int64_t>;
-INSTANTIATE_TYPED_TEST_SUITE_P(TestPrefix, avx512_sort, Types);
-
 template <typename K, typename V = uint64_t>
 struct sorted_t {
     K key;
     K value;
 };
+
 template <typename K, typename V = uint64_t>
 bool compare(sorted_t<K, V> a, sorted_t<K, V> b)
 {
@@ -87,7 +38,8 @@ TYPED_TEST_P(TestKeyValueSort, KeyValueSort)
 
     for (size_t ii = 0; ii < keysizes.size(); ++ii) {
         /* Random array */
-        keys = get_uniform_rand_array_key<TypeParam>(keysizes[ii]);
+        keys =
+            get_uniform_rand_array_with_uniquevalues<TypeParam>(keysizes[ii]);
         values = get_uniform_rand_array<uint64_t>(keysizes[ii]);
         for (size_t i = 0; i < keys.size(); i++) {
             sorted_t<TypeParam, uint64_t> tmp_s;
@@ -113,4 +65,4 @@ TYPED_TEST_P(TestKeyValueSort, KeyValueSort)
 REGISTER_TYPED_TEST_SUITE_P(TestKeyValueSort, KeyValueSort);
 
 using TypesKv = testing::Types<double, uint64_t, int64_t>;
-INSTANTIATE_TYPED_TEST_SUITE_P(TestPrefixKv, TestKeyValueSort, TypesKv);
+INSTANTIATE_TYPED_TEST_SUITE_P(TestPrefixKv, TestKeyValueSort, TypesKv);
diff --git a/tests/test_qsort.cpp b/tests/test_qsort.cpp
@@ -0,0 +1,56 @@
+/*******************************************
+ * * Copyright (C) 2022 Intel Corporation
+ * * SPDX-License-Identifier: BSD-3-Clause
+ * *******************************************/
+
+#include "avx512-common-qsort.h"
+#include "cpuinfo.h"
+#include "rand_array.h"
+#include <gtest/gtest.h>
+#include <vector>
+
+template <typename T>
+class avx512_sort : public ::testing::Test {
+};
+TYPED_TEST_SUITE_P(avx512_sort);
+
+TYPED_TEST_P(avx512_sort, test_arrsizes)
+{
+    if (cpu_has_avx512bw()) {
+        if ((sizeof(TypeParam) == 2) && (!cpu_has_avx512_vbmi2())) {
+            GTEST_SKIP() << "Skipping this test, it requires avx512_vbmi2";
+        }
+        std::vector<int64_t> arrsizes;
+        for (int64_t ii = 0; ii < 1024; ++ii) {
+            arrsizes.push_back((TypeParam)ii);
+        }
+        std::vector<TypeParam> arr;
+        std::vector<TypeParam> sortedarr;
+        for (size_t ii = 0; ii < arrsizes.size(); ++ii) {
+            /* Random array */
+            arr = get_uniform_rand_array<TypeParam>(arrsizes[ii]);
+            sortedarr = arr;
+            /* Sort with std::sort for comparison */
+            std::sort(sortedarr.begin(), sortedarr.end());
+            avx512_qsort<TypeParam>(arr.data(), arr.size());
+            ASSERT_EQ(sortedarr, arr);
+            arr.clear();
+            sortedarr.clear();
+        }
+    }
+    else {
+        GTEST_SKIP() << "Skipping this test, it requires avx512bw";
+    }
+}
+
+REGISTER_TYPED_TEST_SUITE_P(avx512_sort, test_arrsizes);
+
+using Types = testing::Types<uint16_t,
+                             int16_t,
+                             float,
+                             double,
+                             uint32_t,
+                             int32_t,
+                             uint64_t,
+                             int64_t>;
+INSTANTIATE_TYPED_TEST_SUITE_P(TestPrefix, avx512_sort, Types);
diff --git a/utils/cpuinfo.cpp b/utils/cpuinfo.cpp
@@ -0,0 +1,43 @@
+/*******************************************
+ * * Copyright (C) 2022 Intel Corporation
+ * * SPDX-License-Identifier: BSD-3-Clause
+ * *******************************************/
+
+#include "cpuinfo.h"
+
+static void
+cpuid(uint32_t feature, uint32_t *a, uint32_t *b, uint32_t *c, uint32_t *d)
+{
+    __asm__ volatile(
+            "cpuid"
+            "\n\t"
+            : "=a"(*a), "=b"(*b), "=c"(*c), "=d"(*d)
+            : "a"(feature), "c"(0));
+}
+
+int cpu_has_avx512_vbmi2()
+{
+    uint32_t eax(0), ebx(0), ecx(0), edx(0);
+    cpuid(0x07, &eax, &ebx, &ecx, &edx);
+    return (ecx >> 6) & 0x1;
+}
+
+int cpu_has_avx512bw()
+{
+    uint32_t eax(0), ebx(0), ecx(0), edx(0);
+    cpuid(0x07, &eax, &ebx, &ecx, &edx);
+    return (ebx >> 30) & 0x1;
+}
+
+// TODO:
+//int check_os_supports_avx512()
+//{
+//    uint32_t eax(0), ebx(0), ecx(0), edx(0);
+//    cpuid(0x01, &eax, &ebx, &ecx, &edx);
+//    // XSAVE:
+//    if ((ecx >> 27) & 0x1) {
+//	uint32_t xget_eax, xget_edx, index(0);
+//	__asm__ ("xgetbv" : "=a"(xget_eax), "=d"(xget_edx) : "c" (index))
+//    }
+//
+//}
diff --git a/utils/cpuinfo.h b/utils/cpuinfo.h
@@ -3,41 +3,9 @@
  * * SPDX-License-Identifier: BSD-3-Clause
  * *******************************************/
 
+#include <stdint.h>
 #include <cpuid.h>
 
-static void
-cpuid(uint32_t feature, uint32_t *a, uint32_t *b, uint32_t *c, uint32_t *d)
-{
-    __asm__ volatile(
-            "cpuid"
-            "\n\t"
-            : "=a"(*a), "=b"(*b), "=c"(*c), "=d"(*d)
-            : "a"(feature), "c"(0));
-}
+int cpu_has_avx512_vbmi2();
 
-int cpu_has_avx512_vbmi2()
-{
-    uint32_t eax(0), ebx(0), ecx(0), edx(0);
-    cpuid(0x07, &eax, &ebx, &ecx, &edx);
-    return (ecx >> 6) & 0x1;
-}
-
-int cpu_has_avx512bw()
-{
-    uint32_t eax(0), ebx(0), ecx(0), edx(0);
-    cpuid(0x07, &eax, &ebx, &ecx, &edx);
-    return (ebx >> 30) & 0x1;
-}
-
-// TODO:
-//int check_os_supports_avx512()
-//{
-//    uint32_t eax(0), ebx(0), ecx(0), edx(0);
-//    cpuid(0x01, &eax, &ebx, &ecx, &edx);
-//    // XSAVE:
-//    if ((ecx >> 27) & 0x1) {
-//	uint32_t xget_eax, xget_edx, index(0);
-//	__asm__ ("xgetbv" : "=a"(xget_eax), "=d"(xget_edx) : "c" (index))
-//    }
-//
-//}
+int cpu_has_avx512bw();
diff --git a/utils/rand_array.h b/utils/rand_array.h
@@ -41,52 +41,16 @@ static std::vector<T> get_uniform_rand_array(
     }
     return arr;
 }
-template <typename T>
-static std::vector<T> get_uniform_rand_array_key(
-        int64_t arrsize,
-        T max = std::numeric_limits<T>::max(),
-        T min = std::numeric_limits<T>::min(),
-        typename std::enable_if<std::is_integral<T>::value>::type * = 0)
-{
-    std::vector<T> arr;
-    std::random_device r;
-    std::default_random_engine e1(r());
-    std::uniform_int_distribution<T> uniform_dist(min, max);
-    for (int64_t ii = 0; ii < arrsize; ++ii) {
-
-        while (true) {
-            T tmp = uniform_dist(e1);
-            auto iter = std::find(arr.begin(), arr.end(), tmp);
-            if (iter == arr.end()) {
-                arr.emplace_back(tmp);
-                break;
-            }
-        }
-    }
-    return arr;
-}
 
 template <typename T>
-static std::vector<T> get_uniform_rand_array_key(
+static std::vector<T> get_uniform_rand_array_with_uniquevalues(
         int64_t arrsize,
         T max = std::numeric_limits<T>::max(),
-        T min = std::numeric_limits<T>::min(),
-        typename std::enable_if<std::is_floating_point<T>::value>::type * = 0)
+        T min = std::numeric_limits<T>::min())
 {
-    std::random_device rd;
-    std::mt19937 gen(rd());
-    std::uniform_real_distribution<T> dis(min, max);
-    std::vector<T> arr;
-    for (int64_t ii = 0; ii < arrsize; ++ii) {
-
-        while (true) {
-            T tmp = dis(gen);
-            auto iter = std::find(arr.begin(), arr.end(), tmp);
-            if (iter == arr.end()) {
-                arr.emplace_back(tmp);
-                break;
-            }
-        }
-    }
+    std::vector<T> arr = get_uniform_rand_array<T>(arrsize, max, min);
+    typename std::vector<T>::iterator ip =
+        std::unique(arr.begin(), arr.begin() + arrsize);
+    arr.resize(std::distance(arr.begin(), ip));
     return arr;
-}
+}

Original file line number	Diff line number	Diff line change
`@@ -803,11 +803,6 @@ void heap_sort(type_t keys, uint64_t indexes, int64_t size)`
`803`	`803`	`}`
`804`	`804`	`}`
`805`	`805`
`806`		`-template <typename T>`
`807`		`-struct sortkv_t {`
`808`		`- T key;`
`809`		`- uint64_t value;`
`810`		`-};`
`811`	`806`	`template <typename vtype, typename type_t>`
`812`	`807`	`void qsort_64bit_(type_t *keys,`
`813`	`808`	`uint64_t *indexes,`