ci: cover i386 and ClangCL for the AVX2-dispatch guard; drop dead _M_X64

.github/workflows/ci.yml

@@ -69,9 +69,80 @@ jobs:
         run: cmake -E make_directory _build && cmake -E chdir _build cmake .. -DCMAKE_BUILD_TYPE=${{ matrix.build_type }} -DHDR_LOG_REQUIRED=${{ matrix.hdr_log_required }} ${{ matrix.cmake_args }}
       - name: Build
         run: cmake --build _build --config ${{ matrix.build_type }}
+      - name: Assert AVX2 dispatch enabled on 64-bit x86
+        if: matrix.os == 'linux'
+        run: |
+          cc -E -dM -Iinclude src/hdr_histogram.c | grep -q "define HDR_HAS_AVX2_DISPATCH" \
+            || { echo "ERROR: AVX2 dispatch unexpectedly compiled out on 64-bit x86"; exit 1; }
       - name: Test
         run: cmake -E chdir _build ctest --build-config ${{ matrix.build_type }}
 
+  # Regression coverage for the AVX2-dispatch guard in src/hdr_histogram.c.
+  # The default matrix above excludes 32-bit Linux and never uses the ClangCL
+  # toolset, so neither of the two configurations the guard specifically
+  # handles was exercised. These two jobs close that gap.
+
+  # 32-bit x86 GCC (i386): HDR_HAS_AVX2_DISPATCH must be compiled out here,
+  # because the AVX2 kernel uses _mm_extract_epi64, which is unavailable in
+  # 32-bit codegen. HDR_LOG_REQUIRED=DISABLED avoids needing a 32-bit zlib;
+  # the core suites still exercise hdr_value_at_percentile (the dispatched path).
+  build-linux-i386:
+    name: build (linux, Debug, i386, gcc -m32)
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v1
+        with:
+          submodules: recursive
+      - name: Install 32-bit toolchain
+        run: |
+          sudo apt-get update
+          sudo apt-get install -y gcc-multilib g++-multilib
+      - name: Configure
+        run: cmake -E make_directory _build && cmake -E chdir _build cmake .. -DCMAKE_BUILD_TYPE=Debug -DHDR_LOG_REQUIRED=DISABLED -DCMAKE_C_FLAGS=-m32 -DCMAKE_CXX_FLAGS=-m32
+      - name: Build
+        run: cmake --build _build
+      - name: Assert the build is really 32-bit
+        run: |
+          file _build/test/hdr_histogram_test
+          file _build/test/hdr_histogram_test | grep -q "ELF 32-bit" || { echo "ERROR: expected a 32-bit binary; -m32 was not honored"; exit 1; }
+      - name: Assert AVX2 dispatch compiled out on i386
+        run: |
+          # Write macros to a file first so a broken 'cc -m32' (e.g. missing
+          # multilib) fails the step under 'set -e' instead of feeding grep
+          # empty input and silently passing this negative assert.
+          cc -m32 -E -dM -Iinclude src/hdr_histogram.c > macros.txt
+          if grep -q "define HDR_HAS_AVX2_DISPATCH" macros.txt; then
+            echo "ERROR: AVX2 dispatch unexpectedly enabled on i386"; exit 1
+          fi
+      - name: Test
+        run: cmake -E chdir _build ctest --output-on-failure
+
+  # clang-cl on Windows (the ClangCL VS toolset): clang-cl defines __clang__
+  # AND _MSC_VER, so before the guard was fixed it enabled the dispatch and
+  # failed to link (__cpu_model). HDR_HAS_AVX2_DISPATCH must be compiled out
+  # here; this job fails if the exclusion regresses.
+  build-windows-clangcl:
+    name: build (windows, Debug, x64, ClangCL toolset)
+    runs-on: windows-latest
+    env:
+      VCPKG_DEFAULT_TRIPLET: x64-windows
+      VCPKG_TARGET_ARCHITECTURE: x64
+      VCPKG_LIBRARY_LINKAGE: static
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v1
+        with:
+          submodules: recursive
+      - name: Install dependencies (windows)
+        run: vcpkg install zlib
+      - name: Configure
+        run: cmake -E make_directory _build && cmake -E chdir _build cmake .. -T ClangCL -A x64 -DCMAKE_BUILD_TYPE=Debug -DCMAKE_TOOLCHAIN_FILE=C:/vcpkg/scripts/buildsystems/vcpkg.cmake
+      - name: Build
+        run: cmake --build _build --config Debug
+      - name: Test
+        run: cmake -E chdir _build ctest --build-config Debug --output-on-failure
+
   # Run the unit suite under AddressSanitizer + UndefinedBehaviorSanitizer so
   # memory-safety and integer-overflow regressions (the class the fuzzers find)
   # are caught deterministically per-PR, not only in the weekly fuzzing run.

src/hdr_histogram.c

@@ -34,9 +34,13 @@
 #  define HDR_UNLIKELY(x) (x)
 #endif
 
-/* Runtime-dispatched AVX2 path: keep the rest of this TU at the project's
-   baseline ISA so the shipped binary does not silently require AVX2. */
-#if (defined(__x86_64__) || defined(_M_X64)) \
+/* Runtime-dispatched AVX2 path; rest of TU stays at baseline ISA so the binary
+   doesn't silently require AVX2. 64-bit x86 + GCC/Clang only:
+     - 32-bit x86: _mm_extract_epi64 unavailable in 32-bit codegen.
+     - _MSC_VER: __builtin_cpu_supports's __cpu_model isn't linked under MSVC;
+       clang-cl also defines __x86_64__/__clang__ so this exclusion is load-bearing.
+     - __INTEL_COMPILER: ICC classic. */
+#if defined(__x86_64__) \
     && (defined(__GNUC__) || defined(__clang__)) && !defined(__INTEL_COMPILER) && !defined(_MSC_VER)
 #  define HDR_HAS_AVX2_DISPATCH 1
 #  include <immintrin.h>
@@ -187,7 +191,9 @@ static int64_t power(int64_t base, int64_t exp)
 static int32_t count_leading_zeros_64(int64_t value)
 {
 #if defined(_MSC_VER) && !(defined(__clang__) && (defined(_M_ARM) || defined(_M_ARM64)))
-    uint32_t leading_zero = 0;
+    /* _BitScanReverse* out-param is 'unsigned long*' (32-bit on Windows);
+       uint32_t* is a hard error under clang-cl */
+    unsigned long leading_zero = 0;
 #if defined(_WIN64)
     _BitScanReverse64(&leading_zero, value);
 #else

src/hdr_interval_recorder.c

@@ -68,10 +68,10 @@ struct hdr_histogram* hdr_interval_recorder_sample_and_recycle(
     hdr_phaser_reader_lock(&r->phaser);
 
     /* volatile read */
-    old_active = hdr_atomic_load_pointer(&r->active);
+    old_active = hdr_atomic_load_pointer((void**) &r->active);
 
     /* volatile write */
-    hdr_atomic_store_pointer(&r->active, histogram_to_recycle);
+    hdr_atomic_store_pointer((void**) &r->active, histogram_to_recycle);
 
     hdr_phaser_flip_phase(&r->phaser, 0);
 
@@ -93,7 +93,7 @@ static void hdr_interval_recorder_update(
 {
     int64_t val = hdr_phaser_writer_enter(&r->phaser);
 
-    void* active = hdr_atomic_load_pointer(&r->active);
+    void* active = hdr_atomic_load_pointer((void**) &r->active);
 
     update_action(active, arg);