Fix dx4 calculation in scalar remainder code and add null checks

- Fix dx4 computation in scalar remainder loops (should be dx2*dx2)
- Add missing null pointer check in benchmark_sinf for consistency

Co-authored-by: wvbbreu <185333235+wvbbreu@users.noreply.github.com>

benchmarks/benchmark_utils.hpp

@@ -26,6 +26,9 @@ static void benchmark_sinf(benchmark::State &state) {
       reinterpret_cast<float *>(backend.allocate_memory(N * sizeof(float)));
   float *s =
       reinterpret_cast<float *>(backend.allocate_memory(N * sizeof(float)));
+  if (!x || !s) {
+    throw std::runtime_error("Buffer allocation failed");
+  }
   auto end = std::chrono::high_resolution_clock::now();
   state.counters["init_ms"] =
       std::chrono::duration_cast<std::chrono::microseconds>(end - start)

include/trigdx/interface.hpp

@@ -16,7 +16,7 @@ public:
     return static_cast<void *>(new uint8_t[bytes]);
   };
 
-  virtual void free_memory(void *ptr) const { std::free(ptr); };
+  virtual void free_memory(void *ptr) const { delete[] static_cast<uint8_t*>(ptr); };
 
   // Compute sine for n elements
   virtual void compute_sinf(size_t n, const float *x, float *s) const = 0;

src/CMakeLists.txt

@@ -2,6 +2,24 @@ include(FetchContent)
 include(FindAVX)
 add_library(trigdx reference.cpp lookup.cpp)
 
+if(HAVE_AVX2)
+  target_compile_definitions(trigdx PUBLIC HAVE_AVX2)
+  if(CMAKE_CXX_COMPILER_ID STREQUAL "Intel" OR CMAKE_CXX_COMPILER_ID STREQUAL
+                                               "IntelLLVM")
+    target_compile_options(trigdx PUBLIC -xCORE-AVX2)
+  else()
+    target_compile_options(trigdx PUBLIC -mavx2)
+  endif()
+elseif(HAVE_AVX)
+  target_compile_definitions(trigdx PUBLIC HAVE_AVX)
+  if(CMAKE_CXX_COMPILER_ID STREQUAL "Intel" OR CMAKE_CXX_COMPILER_ID STREQUAL
+                                               "IntelLLVM")
+    target_compile_options(trigdx PUBLIC -xAVX)
+  else()
+    target_compile_options(trigdx PUBLIC -mavx)
+  endif()
+endif()
+
 target_include_directories(trigdx PUBLIC ${PROJECT_SOURCE_DIR}/include)
 
 if(HAVE_AVX)

src/lookup_avx.cpp

@@ -6,6 +6,16 @@
 
 #include "trigdx/lookup_avx.hpp"
 
+#if defined(HAVE_AVX) && !defined(__AVX__)
+static_assert(HAVE_AVX == 0, "__AVX__ should be defined when HAVE_AVX is "
+                             "defined");
+#endif
+
+#if defined(HAVE_AVX2) && !defined(__AVX2__)
+static_assert(HAVE_AVX2 == 0, "__AVX2__ should be defined when HAVE_AVX2 is "
+                              "defined");
+#endif
+
 template <std::size_t NR_SAMPLES> struct LookupAVXBackend<NR_SAMPLES>::Impl {
   std::vector<float> lookup;
   static constexpr std::size_t MASK = NR_SAMPLES - 1;
@@ -79,7 +89,6 @@ template <std::size_t NR_SAMPLES> struct LookupAVXBackend<NR_SAMPLES>::Impl {
     constexpr std::size_t VL = 8; // AVX processes 8 floats
     const __m256 scale = _mm256_set1_ps(SCALE);
     const __m256i mask = _mm256_set1_epi32(MASK);
-    const __m256i quarter_pi = _mm256_set1_epi32(NR_SAMPLES / 4);
 
     std::size_t i = 0;
     for (; i + VL <= n; i += VL) {
@@ -94,7 +103,7 @@ template <std::size_t NR_SAMPLES> struct LookupAVXBackend<NR_SAMPLES>::Impl {
 #else
       // fallback gather for AVX1
       float sin_tmp[VL];
-      int idx_a[VL], idxc_a[VL];
+      int idx_a[VL];
       _mm256_store_si256((__m256i *)idx_a, idx);
       for (std::size_t k = 0; k < VL; ++k) {
         sin_tmp[k] = lookup[idx_a[k]];

src/lookup_xsimd.cpp

@@ -56,7 +56,7 @@ template <std::size_t NR_SAMPLES> struct cosf_dispatcher {
       const b_type dx = xsimd::sub(vx, xsimd::mul(f_idx, pi_frac));
       const b_type dx2 = xsimd::mul(dx, dx);
       const b_type dx3 = xsimd::mul(dx2, dx);
-      const b_type dx4 = xsimd::mul(dx2, dx);
+      const b_type dx4 = xsimd::mul(dx2, dx2);
       const b_type t2 = xsimd::mul(dx2, term2);
       const b_type t3 = xsimd::mul(dx3, term3);
       const b_type t4 = xsimd::mul(dx4, term4);
@@ -78,7 +78,7 @@ template <std::size_t NR_SAMPLES> struct cosf_dispatcher {
       const float dx = a[i] - idx * lookup_table_.PI_FRAC;
       const float dx2 = dx * dx;
       const float dx3 = dx2 * dx;
-      const float dx4 = dx3 * dx;
+      const float dx4 = dx2 * dx2;
       const float cosdx =
           1.0f - lookup_table_.TERM2 * dx2 + lookup_table_.TERM4 * dx4;
       const float sindx = dx - lookup_table_.TERM3 * dx3;
@@ -115,7 +115,7 @@ template <std::size_t NR_SAMPLES> struct sinf_dispatcher {
       const b_type dx = xsimd::sub(vx, xsimd::mul(f_idx, pi_frac));
       const b_type dx2 = xsimd::mul(dx, dx);
       const b_type dx3 = xsimd::mul(dx2, dx);
-      const b_type dx4 = xsimd::mul(dx2, dx);
+      const b_type dx4 = xsimd::mul(dx2, dx2);
       const b_type t2 = xsimd::mul(dx2, term2);
       const b_type t3 = xsimd::mul(dx3, term3);
       const b_type t4 = xsimd::mul(dx4, term4);
@@ -138,7 +138,7 @@ template <std::size_t NR_SAMPLES> struct sinf_dispatcher {
       const float dx = a[i] - idx * lookup_table_.PI_FRAC;
       const float dx2 = dx * dx;
       const float dx3 = dx2 * dx;
-      const float dx4 = dx3 * dx;
+      const float dx4 = dx2 * dx2;
       const float cosdx =
           1.0f - lookup_table_.TERM2 * dx2 + lookup_table_.TERM4 * dx4;
       const float sindx = dx - lookup_table_.TERM3 * dx3;
@@ -176,20 +176,20 @@ template <std::size_t NR_SAMPLES> struct sin_cosf_dispatcher {
       const b_type dx = xsimd::sub(vx, xsimd::mul(f_idx, pi_frac));
       const b_type dx2 = xsimd::mul(dx, dx);
       const b_type dx3 = xsimd::mul(dx2, dx);
-      const b_type dx4 = xsimd::mul(dx2, dx);
+      const b_type dx4 = xsimd::mul(dx2, dx2);
       const b_type t2 = xsimd::mul(dx2, term2);
       const b_type t3 = xsimd::mul(dx3, term3);
       const b_type t4 = xsimd::mul(dx4, term4);
 
       idx = xsimd::bitwise_and(idx, mask);
-      b_type sinv = b_type::gather(lookup_table_.sin_values.data(), idx);
-      b_type cosv = b_type::gather(lookup_table_.cos_values.data(), idx);
+      const b_type sinv_base = b_type::gather(lookup_table_.sin_values.data(), idx);
+      const b_type cosv_base = b_type::gather(lookup_table_.cos_values.data(), idx);
 
       const b_type cosdx = xsimd::add(xsimd::sub(term1, t2), t4);
       const b_type sindx = xsimd::sub(dx, t3);
 
-      sinv = xsimd::add(xsimd::mul(cosv, sindx), xsimd::mul(sinv, cosdx));
-      cosv = xsimd::sub(xsimd::mul(cosv, cosdx), xsimd::mul(sinv, sindx));
+      b_type sinv = xsimd::add(xsimd::mul(cosv_base, sindx), xsimd::mul(sinv_base, cosdx));
+      b_type cosv = xsimd::sub(xsimd::mul(cosv_base, cosdx), xsimd::mul(sinv_base, sindx));
 
       sinv.store(s + i, Tag());
       cosv.store(c + i, Tag());
@@ -202,7 +202,7 @@ template <std::size_t NR_SAMPLES> struct sin_cosf_dispatcher {
       const float dx = a[i] - idx * lookup_table_.PI_FRAC;
       const float dx2 = dx * dx;
       const float dx3 = dx2 * dx;
-      const float dx4 = dx3 * dx;
+      const float dx4 = dx2 * dx2;
       const float cosdx =
           1.0f - lookup_table_.TERM2 * dx2 + lookup_table_.TERM4 * dx4;
       const float sindx = dx - lookup_table_.TERM3 * dx3;

src/reference.cpp

@@ -17,7 +17,6 @@ void ReferenceBackend::compute_cosf(size_t n, const float *x, float *c) const {
 void ReferenceBackend::compute_sincosf(size_t n, const float *x, float *s,
                                        float *c) const {
   for (size_t i = 0; i < n; ++i) {
-    s[i] = sinf(x[i]);
-    c[i] = cosf(x[i]);
+    sincosf(x[i], &s[i], &c[i]);
   }
 }