From 9b247c06be3706ba69f26d2849154c8b16239014 Mon Sep 17 00:00:00 2001 From: Ben Ashbaugh Date: Tue, 18 Feb 2025 09:09:17 -0800 Subject: [PATCH] fix several compile issues with Visual Studio toolchains (#2219) Fixes several compile issues I am seeing for my version of Visual Studio related to an ambiguous call to `fpclassify`, which is called by `isnan` and other similar functions, specifically for the `cl_half` type: ``` 19>C:\Program Files (x86)\Windows Kits\10\Include\10.0.19041.0\ucrt\corecrt_math.h(401,1): error C2668: 'fpclassify': ambiguous call to overloaded function 19>C:\Program Files (x86)\Windows Kits\10\Include\10.0.19041.0\ucrt\corecrt_math.h(298,31): message : could be 'int fpclassify(long double) throw()' 19>C:\Program Files (x86)\Windows Kits\10\Include\10.0.19041.0\ucrt\corecrt_math.h(293,31): message : or 'int fpclassify(double) throw()' 19>C:\Program Files (x86)\Windows Kits\10\Include\10.0.19041.0\ucrt\corecrt_math.h(288,31): message : or 'int fpclassify(float) throw()' 19>C:\Program Files (x86)\Windows Kits\10\Include\10.0.19041.0\ucrt\corecrt_math.h(401,1): message : while trying to match the argument list '(_Ty)' ``` Some of these issues seem like differences in compiler behavior, but at least one appears to have identified a legitimate bug. Specifically, this change: * Removes the special-case checks for finite half numbers for commonfns, since this is already handled by `UlpFn`. (test with: `test_commonfns degrees radians`) * Assigns to temporary variables to eliminate the ambiguous function call for relationals. (test with: `test_relationals relational*`) * Properly converts from half to float when checking for NaNs for select. This is the one that seems like a legitimate bug. (test with: `test_select select_half_ushort select_half_short`) * Uses `std::enable_if` to disambiguate a function call for spirv_new. (test with: `test_spirv_new decorate_saturated*`) If it's helpful, my specific Visual Studio version is: ``` Microsoft Visual Studio Professional 2019 Version 16.11.20 VisualStudio.16.Release/16.11.20+32929.386 ``` I also have the Windows Software Development Kit 10.0.19041.685 installed. --- test_conformance/commonfns/test_base.h | 21 ---------- test_conformance/commonfns/test_unary_fn.cpp | 8 ---- .../relationals/test_comparisons_fp.cpp | 5 ++- test_conformance/select/util_select.cpp | 22 ++++++---- test_conformance/spirv_new/test_decorate.cpp | 41 +++++++++++-------- 5 files changed, 43 insertions(+), 54 deletions(-) diff --git a/test_conformance/commonfns/test_base.h b/test_conformance/commonfns/test_base.h index bcce3ba4..ef394635 100644 --- a/test_conformance/commonfns/test_base.h +++ b/test_conformance/commonfns/test_base.h @@ -182,27 +182,6 @@ template inline half conv_to_half(const T &val) return 0; } -template bool isfinite_fp(const T &v) -{ - if (std::is_same::value) - { - // Extract FP16 exponent and mantissa - uint16_t h_exp = (((half)v) >> (CL_HALF_MANT_DIG - 1)) & 0x1F; - uint16_t h_mant = ((half)v) & 0x3FF; - - // !Inf test - return !(h_exp == 0x1F && h_mant == 0); - } - else - { -#if !defined(_WIN32) - return std::isfinite(v); -#else - return isfinite(v); -#endif - } -} - template float UlpFn(const T &val, const double &r) { if (std::is_same::value) diff --git a/test_conformance/commonfns/test_unary_fn.cpp b/test_conformance/commonfns/test_unary_fn.cpp index 23b665a4..6515adf7 100644 --- a/test_conformance/commonfns/test_unary_fn.cpp +++ b/test_conformance/commonfns/test_unary_fn.cpp @@ -65,10 +65,6 @@ int verify_degrees(const T *const inptr, const T *const outptr, int n) { r = (180.0 / M_PI) * conv_to_dbl(inptr[i]); - if (std::is_same::value) - if (!isfinite_fp(conv_to_half(r)) && !isfinite_fp(outptr[i])) - continue; - error = UlpFn(outptr[i], r); if (fabsf(error) > max_error) @@ -115,10 +111,6 @@ int verify_radians(const T *const inptr, const T *const outptr, int n) { r = (M_PI / 180.0) * conv_to_dbl(inptr[i]); - if (std::is_same::value) - if (!isfinite_fp(conv_to_half(r)) && !isfinite_fp(outptr[i])) - continue; - error = UlpFn(outptr[i], r); if (fabsf(error) > max_error) diff --git a/test_conformance/relationals/test_comparisons_fp.cpp b/test_conformance/relationals/test_comparisons_fp.cpp index 6c4890d7..14b1696f 100644 --- a/test_conformance/relationals/test_comparisons_fp.cpp +++ b/test_conformance/relationals/test_comparisons_fp.cpp @@ -368,8 +368,9 @@ int RelationalsFPTest::test_equiv_kernel(unsigned int vecSize, { if (gInfNanSupport == 0) { - if (isnan(inDataA[i * vecSize + j]) - || isnan(inDataB[i * vecSize + j])) + float a = inDataA[i * vecSize + j]; + float b = inDataB[i * vecSize + j]; + if (isnan(a) || isnan(b)) fail = 0; else fail = 1; diff --git a/test_conformance/select/util_select.cpp b/test_conformance/select/util_select.cpp index 078ff64a..a685b7f6 100644 --- a/test_conformance/select/util_select.cpp +++ b/test_conformance/select/util_select.cpp @@ -18,6 +18,7 @@ #include #include #include "test_select.h" +#include "CL/cl_half.h" //----------------------------------------- @@ -830,10 +831,12 @@ size_t check_half(const void *const test, const void *const correct, if (memcmp(t, c, count * sizeof(c[0])) != 0) { - for (i = 0; i < count; i++) /* Allow nans to be binary different */ - if ((t[i] != c[i]) - && !(isnan(((cl_half *)correct)[i]) - && isnan(((cl_half *)test)[i]))) + // Allow nans to be binary different + for (i = 0; i < count; i++) + { + float fcorrect = cl_half_to_float(c[i]); + float ftest = cl_half_to_float(t[i]); + if ((t[i] != c[i]) && !(isnan(fcorrect) && isnan(ftest))) { log_error("\n(check_half) Error for vector size %zu found at " "0x%8.8zx (of 0x%8.8zx): " @@ -841,6 +844,7 @@ size_t check_half(const void *const test, const void *const correct, vector_size, i, count, c[i], t[i]); return i + 1; } + } } return 0; @@ -855,9 +859,12 @@ size_t check_float(const void *const test, const void *const correct, if (memcmp(t, c, count * sizeof(c[0])) != 0) { - for (i = 0; i < count; i++) /* Allow nans to be binary different */ - if ((t[i] != c[i]) - && !(isnan(((float *)correct)[i]) && isnan(((float *)test)[i]))) + // Allow nans to be binary different + for (i = 0; i < count; i++) + { + float fcorrect = ((float *)correct)[i]; + float ftest = ((float *)test)[i]; + if ((t[i] != c[i]) && !(isnan(fcorrect) && isnan(ftest))) { log_error("\n(check_float) Error for vector size %zu found at " "0x%8.8zx (of 0x%8.8zx): " @@ -865,6 +872,7 @@ size_t check_float(const void *const test, const void *const correct, vector_size, i, count, c[i], t[i]); return i + 1; } + } } return 0; diff --git a/test_conformance/spirv_new/test_decorate.cpp b/test_conformance/spirv_new/test_decorate.cpp index b1168fe0..fc9fc522 100644 --- a/test_conformance/spirv_new/test_decorate.cpp +++ b/test_conformance/spirv_new/test_decorate.cpp @@ -21,6 +21,7 @@ #include #include #include +#include #include @@ -216,29 +217,37 @@ static inline Ti generate_saturated_rhs_input(RandomSeed &seed) } template -static inline To compute_saturated_output(Ti lhs, Ti rhs, - cl_half_rounding_mode half_rounding) +static inline + typename std::enable_if::value, To>::type + compute_saturated_output(Ti lhs, Ti rhs, + cl_half_rounding_mode half_rounding) { constexpr auto loVal = std::numeric_limits::min(); constexpr auto hiVal = std::numeric_limits::max(); - if (std::is_same::value) + cl_float f = cl_half_to_float(lhs) * cl_half_to_float(rhs); + + // Quantize to fp16: + f = cl_half_to_float(cl_half_from_float(f, half_rounding)); + + To val = static_cast(std::min(std::max(f, loVal), hiVal)); + if (isnan(cl_half_to_float(rhs))) { - cl_float f = cl_half_to_float(lhs) * cl_half_to_float(rhs); - - // Quantize to fp16: - f = cl_half_to_float(cl_half_from_float(f, half_rounding)); - - To val = (To)std::min(std::max(f, loVal), hiVal); - if (isnan(cl_half_to_float(rhs))) - { - val = 0; - } - return val; + val = 0; } + return val; +} - Tl ival = (Tl)(lhs * rhs); - To val = (To)std::min(std::max(ival, loVal), hiVal); +template +static inline + typename std::enable_if::value, To>::type + compute_saturated_output(Ti lhs, Ti rhs, cl_half_rounding_mode) +{ + constexpr auto loVal = std::numeric_limits::min(); + constexpr auto hiVal = std::numeric_limits::max(); + + Tl ival = static_cast(lhs * rhs); + To val = static_cast(std::min(std::max(ival, loVal), hiVal)); if (isnan(rhs)) {