From c881d23a8fd45aa7c830420712704d573d74b4c4 Mon Sep 17 00:00:00 2001 From: Mohankumar Nekkarakalaya Date: Wed, 20 Oct 2021 17:14:08 -0700 Subject: [PATCH] Allow NaN result when result exceeds limit Based on spec if the result is too large to be represented in the floating point the result is undefined. Issue was found with ldexp. Since fix is applicable in general updated the isFinite() to take maxValue and checks the interval value with the allowed max value in the context (and similarly for negative numbers). If the result is beyond the 32-bit floating-point representation expected result is unbounded Components: Vulkan, OpenGL, Framework VK-GL-CTS Issue: 3138 Affected tests: dEQP-VK.glsl.builtin.function.* dEQP-VK.glsl.builtin.precision.* dEQP-GLES31.functional.shaders.builtin_functions.precision.* dEQP-GLES31.functional.shaders.builtin_functions.common.fma.* Change-Id: Iba27d6b0d7d0bc433a1d0c055cef6f6a61b239ba --- .../shaderexecutor/vktShaderBuiltinPrecisionTests.cpp | 16 ++++++++-------- framework/common/tcuInterval.hpp | 7 +++++-- .../gles31/functional/es31fShaderCommonFunctionTests.cpp | 4 ++-- modules/glshared/glsBuiltinPrecisionTests.cpp | 14 +++++++------- 4 files changed, 22 insertions(+), 19 deletions(-) diff --git a/external/vulkancts/modules/vulkan/shaderexecutor/vktShaderBuiltinPrecisionTests.cpp b/external/vulkancts/modules/vulkan/shaderexecutor/vktShaderBuiltinPrecisionTests.cpp index e37bbcf..a896fe6 100644 --- a/external/vulkancts/modules/vulkan/shaderexecutor/vktShaderBuiltinPrecisionTests.cpp +++ b/external/vulkancts/modules/vulkan/shaderexecutor/vktShaderBuiltinPrecisionTests.cpp @@ -2557,7 +2557,7 @@ public: const typename Signature::IArgs& iargs) const { // Fast-path for common case - if (iargs.a.isOrdinary() && iargs.b.isOrdinary()) + if (iargs.a.isOrdinary(ctx.format.getMaxValue()) && iargs.b.isOrdinary(ctx.format.getMaxValue())) { Interval ret; TCU_SET_INTERVAL_BOUNDS(ret, sum, @@ -2585,7 +2585,7 @@ public: Interval b = iargs.b; // Fast-path for common case - if (a.isOrdinary() && b.isOrdinary()) + if (a.isOrdinary(ctx.format.getMaxValue()) && b.isOrdinary(ctx.format.getMaxValue())) { Interval ret; if (a.hi() < 0) @@ -2634,7 +2634,7 @@ public: Interval doApply (const EvalContext& ctx, const typename Signature::IArgs& iargs) const { // Fast-path for common case - if (iargs.a.isOrdinary() && iargs.b.isOrdinary()) + if (iargs.a.isOrdinary(ctx.format.getMaxValue()) && iargs.b.isOrdinary(ctx.format.getMaxValue())) { Interval ret; @@ -3251,7 +3251,7 @@ protected: ret |= ctx.format.roundOut(Interval(-DE_PI_DOUBLE, DE_PI_DOUBLE), true); } - if (!yi.isFinite() || !xi.isFinite()) + if (!yi.isFinite(ctx.format.getMaxValue()) || !xi.isFinite(ctx.format.getMaxValue())) { // Infinities may not be supported, allow anything, including NaN ret |= TCU_NAN; @@ -4255,7 +4255,7 @@ public: } protected: - TIRet doApply (const EvalContext&, const TIArgs& iargs) const + TIRet doApply (const EvalContext& ctx, const TIArgs& iargs) const { Interval fracIV; Interval& wholeIV = const_cast(iargs.b); @@ -4265,7 +4265,7 @@ protected: TCU_INTERVAL_APPLY_MONOTONE1(wholeIV, x, iargs.a, whole, deModf(x, &intPart); whole = intPart); - if (!iargs.a.isFinite()) + if (!iargs.a.isFinite(ctx.format.getMaxValue())) { // Behavior on modf(Inf) not well-defined, allow anything as a fractional part // See Khronos bug 13907 @@ -4517,7 +4517,7 @@ protected: bool any = iargs.a.hasNaN() || iargs.b.hi() > (maxExp + 1); Interval ret(any, ldexp(iargs.a.lo(), (int)iargs.b.lo()), ldexp(iargs.a.hi(), (int)iargs.b.hi())); if (iargs.b.lo() < minExp) ret |= 0.0; - if (!ret.isFinite()) ret |= TCU_NAN; + if (!ret.isFinite(ctx.format.getMaxValue())) ret |= TCU_NAN; return ctx.format.convert(ret); } }; @@ -4534,7 +4534,7 @@ Interval LdExp >::doApply(const EvalContext& ctx, // Add 1ULP precision tolerance to account for differing rounding modes between the GPU and deLdExp. ret += Interval(-ctx.format.ulp(ret.lo()), ctx.format.ulp(ret.hi())); if (iargs.b.lo() < minExp) ret |= 0.0; - if (!ret.isFinite()) ret |= TCU_NAN; + if (!ret.isFinite(ctx.format.getMaxValue())) ret |= TCU_NAN; return ctx.format.convert(ret); } diff --git a/framework/common/tcuInterval.hpp b/framework/common/tcuInterval.hpp index 952ba81..f5cf640 100644 --- a/framework/common/tcuInterval.hpp +++ b/framework/common/tcuInterval.hpp @@ -97,8 +97,11 @@ public: bool hasNaN (void) const { return m_hasNaN; } Interval nan (void) const { return m_hasNaN ? TCU_NAN : Interval(); } bool empty (void) const { return m_lo > m_hi; } - bool isFinite (void) const { return m_lo > -TCU_INFINITY && m_hi < TCU_INFINITY; } - bool isOrdinary (void) const { return !hasNaN() && !empty() && isFinite(); } + + // The interval is represented in double, it can extend outside the range of smaller floating-point formats + // and get rounded to infinity. + bool isFinite (double maxValue) const { return m_lo > -maxValue && m_hi < maxValue; } + bool isOrdinary (double maxValue) const { return !hasNaN() && !empty() && isFinite(maxValue); } void warning (double lo_, double hi_) { diff --git a/modules/gles31/functional/es31fShaderCommonFunctionTests.cpp b/modules/gles31/functional/es31fShaderCommonFunctionTests.cpp index 21ee0f6..bae3ea3 100644 --- a/modules/gles31/functional/es31fShaderCommonFunctionTests.cpp +++ b/modules/gles31/functional/es31fShaderCommonFunctionTests.cpp @@ -2033,13 +2033,13 @@ public: TCU_SET_INTERVAL(prod2, tmp, tmp = ia.hi() * ib.lo()); TCU_SET_INTERVAL(prod3, tmp, tmp = ia.hi() * ib.hi()); - prod = format.convert(format.roundOut(prod0 | prod1 | prod2 | prod3, ia.isFinite() && ib.isFinite())); + prod = format.convert(format.roundOut(prod0 | prod1 | prod2 | prod3, ia.isFinite(format.getMaxValue()) && ib.isFinite(format.getMaxValue()))); TCU_SET_INTERVAL_BOUNDS(res, tmp, tmp = prod.lo() + ic.lo(), tmp = prod.hi() + ic.hi()); - return format.convert(format.roundOut(res, prod.isFinite() && ic.isFinite())); + return format.convert(format.roundOut(res, prod.isFinite(format.getMaxValue()) && ic.isFinite(format.getMaxValue()))); } bool compare (const void* const* inputs, const void* const* outputs) diff --git a/modules/glshared/glsBuiltinPrecisionTests.cpp b/modules/glshared/glsBuiltinPrecisionTests.cpp index f59297c..fbeee6a 100644 --- a/modules/glshared/glsBuiltinPrecisionTests.cpp +++ b/modules/glshared/glsBuiltinPrecisionTests.cpp @@ -1926,7 +1926,7 @@ public: const IArgs& iargs) const { // Fast-path for common case - if (iargs.a.isOrdinary() && iargs.b.isOrdinary()) + if (iargs.a.isOrdinary(ctx.format.getMaxValue()) && iargs.b.isOrdinary(ctx.format.getMaxValue())) { Interval ret; TCU_SET_INTERVAL_BOUNDS(ret, sum, @@ -1953,7 +1953,7 @@ public: Interval b = iargs.b; // Fast-path for common case - if (a.isOrdinary() && b.isOrdinary()) + if (a.isOrdinary(ctx.format.getMaxValue()) && b.isOrdinary(ctx.format.getMaxValue())) { Interval ret; if (a.hi() < 0) @@ -2001,7 +2001,7 @@ public: Interval doApply (const EvalContext& ctx, const IArgs& iargs) const { // Fast-path for common case - if (iargs.a.isOrdinary() && iargs.b.isOrdinary()) + if (iargs.a.isOrdinary(ctx.format.getMaxValue()) && iargs.b.isOrdinary(ctx.format.getMaxValue())) { Interval ret; @@ -2486,7 +2486,7 @@ public: ATan2 (void) : CFloatFunc2 ("atan", deAtan2) {} protected: - Interval innerExtrema (const EvalContext&, + Interval innerExtrema (const EvalContext& ctx, const Interval& yi, const Interval& xi) const { @@ -2500,7 +2500,7 @@ protected: ret |= Interval(-DE_PI_DOUBLE, DE_PI_DOUBLE); } - if ((!yi.isFinite() || !xi.isFinite())) + if (!yi.isFinite(ctx.format.getMaxValue()) || !xi.isFinite(ctx.format.getMaxValue())) { // Infinities may not be supported, allow anything, including NaN ret |= TCU_NAN; @@ -3319,7 +3319,7 @@ public: } protected: - IRet doApply (const EvalContext&, const IArgs& iargs) const + IRet doApply (const EvalContext& ctx, const IArgs& iargs) const { Interval fracIV; Interval& wholeIV = const_cast(iargs.b); @@ -3329,7 +3329,7 @@ protected: TCU_INTERVAL_APPLY_MONOTONE1(wholeIV, x, iargs.a, whole, deModf(x, &intPart); whole = intPart); - if (!iargs.a.isFinite()) + if (!iargs.a.isFinite(ctx.format.getMaxValue())) { // Behavior on modf(Inf) not well-defined, allow anything as a fractional part // See Khronos bug 13907 -- 2.7.4