Merge pull request #70 from r-devulap/fix-moree-warnings

Fix more compiler warnings
intel · Sep 6, 2023 · b9f9340 · b9f9340
2 parents ed6c409 + 441eb5a
commit b9f9340
Show file tree

Hide file tree

Showing 4 changed files with 48 additions and 21 deletions.
diff --git a/.github/workflows/c-cpp.yml b/.github/workflows/c-cpp.yml
@@ -1,4 +1,4 @@
-name: Tests and benchmarks
+name: Build and run tests
 
 on:
   push:
@@ -7,7 +7,7 @@ on:
     branches: [ "main" ]
 
 jobs:
-  BasicBuildAndTest:
+  ICX:
 
     runs-on: ubuntu-latest
 
@@ -17,7 +17,7 @@ jobs:
     - name: Install dependencies
       run: |
         sudo apt update
-        sudo apt -y install g++-12 g++-10 libgtest-dev meson curl git cmake
+        sudo apt -y install g++-10 libgtest-dev meson curl git cmake
 
     - name: Install google benchmarks
       run: |
@@ -33,31 +33,58 @@ jobs:
         mkdir /tmp/sde && tar -xvf /tmp/sde.tar.xz -C /tmp/sde/
         sudo mv /tmp/sde/* /opt/sde && sudo ln -s /opt/sde/sde64 /usr/bin/sde
 
-    - name: Build with gcc-10
+    - name: Build
       env:
         CXX: g++-10
       run: |
         make clean
-        meson setup --warnlevel 0 --buildtype plain builddir
+        meson setup --warnlevel 2 --werror --buildtype plain builddir
         cd builddir
         ninja
 
-    - name: Run test suite on SKX
-      run: sde -skx -- ./builddir/testexe
+    - name: Run test suite on ICX
+      run: sde -icx -- ./builddir/testexe
 
-    - name: Build with gcc-12
+  SPR:
+
+    runs-on: ubuntu-latest
+
+    steps:
+    - uses: actions/checkout@v3
+
+    - name: Install dependencies
+      run: |
+        sudo apt update
+        sudo apt -y install g++-12 libgtest-dev meson curl git cmake
+
+    - name: Install google benchmarks
+      run: |
+        git clone https://github.com/google/benchmark.git
+        cd benchmark
+        cmake -E make_directory "build"
+        cmake -E chdir "build" cmake -DBENCHMARK_ENABLE_GTEST_TESTS=OFF -DBENCHMARK_ENABLE_TESTING=OFF -DCMAKE_BUILD_TYPE=Release ../
+        sudo cmake --build "build" --config Release --target install
+
+    - name: Install Intel SDE
+      run: |
+        curl -o /tmp/sde.tar.xz https://downloadmirror.intel.com/732268/sde-external-9.7.0-2022-05-09-lin.tar.xz
+        mkdir /tmp/sde && tar -xvf /tmp/sde.tar.xz -C /tmp/sde/
+        sudo mv /tmp/sde/* /opt/sde && sudo ln -s /opt/sde/sde64 /usr/bin/sde
+
+    - name: Build
       env:
         CXX: g++-12
       run: |
         make clean
-        meson setup --warnlevel 0 --buildtype plain builddir
+        meson setup --warnlevel 2 --werror --buildtype plain builddir
         cd builddir
         ninja
 
-    - name: Run test suite on SPR
-      run: sde -spr -- ./builddir/testexe
+    - name: Run _Float16 test suite on SPR
+      run: sde -spr -- ./builddir/testexe --gtest_filter="*float16*"
 
   compare-benchmarks-with-main:
+    if: ${{ false }}  # disable for now
 
     runs-on: ubuntu-latest
 

diff --git a/Makefile b/Makefile
@@ -75,7 +75,7 @@ benchexe: $(BENCHOBJS) $(UTILOBJS)
 
 .PHONY: meson
 meson:
-	meson setup --warnlevel 2 --buildtype plain builddir
+	meson setup --warnlevel 2 --werror --buildtype plain builddir
 	cd builddir && ninja
 
 .PHONY: clean

diff --git a/src/avx512-common-qsort.h b/src/avx512-common-qsort.h
@@ -73,8 +73,8 @@
 #ifdef _MSC_VER
 #define X86_SIMD_SORT_INLINE static inline
 #define X86_SIMD_SORT_FINLINE static __forceinline
-#define LIKELY(x)
-#define UNLIKELY(x)
+#define LIKELY(x) (x)
+#define UNLIKELY(x) (x)
 #elif defined(__CYGWIN__)
 /*
  * Force inline in cygwin to work around a compiler bug. See
@@ -90,8 +90,8 @@
 #else
 #define X86_SIMD_SORT_INLINE static
 #define X86_SIMD_SORT_FINLINE static
-#define LIKELY(x)
-#define UNLIKELY(x)
+#define LIKELY(x) (x)
+#define UNLIKELY(x) (x)
 #endif
 
 #if __GNUC__ >= 8

diff --git a/tests/test-qsortfp16.cpp b/tests/test-qsortfp16.cpp
@@ -21,7 +21,7 @@ TEST(avx512_qsort_float16, test_arrsizes)
 
         for (size_t ii = 0; ii < arrsizes.size(); ++ii) {
             /* Random array */
-            for (size_t jj = 0; jj < arrsizes[ii]; ++jj) {
+            for (auto jj = 0; jj < arrsizes[ii]; ++jj) {
                 _Float16 temp = (float)rand() / (float)(RAND_MAX);
                 arr.push_back(temp);
                 sortedarr.push_back(temp);
@@ -86,7 +86,7 @@ TEST(avx512_qselect_float16, test_arrsizes)
 
         for (size_t ii = 0; ii < arrsizes.size(); ++ii) {
             /* Random array */
-            for (size_t jj = 0; jj < arrsizes[ii]; ++jj) {
+            for (auto jj = 0; jj < arrsizes[ii]; ++jj) {
                 _Float16 temp = (float)rand() / (float)(RAND_MAX);
                 arr.push_back(temp);
                 sortedarr.push_back(temp);
@@ -129,23 +129,23 @@ TEST(avx512_partial_qsort_float16, test_ranges)
         std::vector<_Float16> psortedarr;
 
         /* Random array */
-        for (size_t ii = 0; ii < arrsize; ++ii) {
+        for (auto ii = 0; ii < arrsize; ++ii) {
             _Float16 temp = (float)rand() / (float)(RAND_MAX);
             arr.push_back(temp);
             sortedarr.push_back(temp);
         }
         /* Sort with std::sort for comparison */
         std::sort(sortedarr.begin(), sortedarr.end());
 
-        for (size_t ii = 0; ii < nranges; ++ii) {
+        for (auto ii = 0; ii < nranges; ++ii) {
             psortedarr = arr;
 
             int k = get_uniform_rand_array<int64_t>(1, arrsize, 1).front();
 
             /* Sort the range and verify all the required elements match the presorted set */
             avx512_partial_qsort<_Float16>(
                     psortedarr.data(), k, psortedarr.size());
-            for (size_t jj = 0; jj < k; jj++) {
+            for (auto jj = 0; jj < k; jj++) {
                 ASSERT_EQ(sortedarr[jj], psortedarr[jj]);
             }