From cc10ab844aa64540bd5e1fa704fddfbc41f4d459 Mon Sep 17 00:00:00 2001 From: Jeff Leger Date: Fri, 12 Jan 2018 15:26:37 -0500 Subject: [PATCH] Relax ULP requirement for Log/Log2 Relax ULP requirement from 3 to 4 for log/log2. Implementations that require 4 ULPs will generate a quality warning. Affects: dEQP-GLES3.functional.shaders.builtin_functions.precision.log.highp_vertex.* Components: Framework, AOSP Khronos OpenGL/API issue: 57 Change-Id: Ibc910322058ee0d4f0a33e21ba256fffe200b3be --- framework/common/tcuInterval.hpp | 62 ++++++++++----- modules/glshared/glsBuiltinPrecisionTests.cpp | 104 ++++++++++++++++++++++---- 2 files changed, 133 insertions(+), 33 deletions(-) diff --git a/framework/common/tcuInterval.hpp b/framework/common/tcuInterval.hpp index cc271a1..bbbdf3e 100644 --- a/framework/common/tcuInterval.hpp +++ b/framework/common/tcuInterval.hpp @@ -58,42 +58,60 @@ class Interval { public: // Empty interval. - Interval (void) - : m_hasNaN (false) - , m_lo (TCU_INFINITY) - , m_hi (-TCU_INFINITY) {} + Interval (void) + : m_hasNaN (false) + , m_lo (TCU_INFINITY) + , m_hi (-TCU_INFINITY) + , m_warningLo (-TCU_INFINITY) + , m_warningHi (TCU_INFINITY) {} // Intentionally not explicit. Conversion from double to Interval is common // and reasonable. - Interval (double val) - : m_hasNaN (!!deIsNaN(val)) - , m_lo (m_hasNaN ? TCU_INFINITY : val) - , m_hi (m_hasNaN ? -TCU_INFINITY : val) {} + Interval (double val) + : m_hasNaN (!!deIsNaN(val)) + , m_lo (m_hasNaN ? TCU_INFINITY : val) + , m_hi (m_hasNaN ? -TCU_INFINITY : val) + , m_warningLo (-TCU_INFINITY) + , m_warningHi (TCU_INFINITY) {} - Interval (bool hasNaN_, double lo_, double hi_) - : m_hasNaN(hasNaN_), m_lo(lo_), m_hi(hi_) {} + + Interval(bool hasNaN_, double lo_, double hi_) + : m_hasNaN(hasNaN_), m_lo(lo_), m_hi(hi_), m_warningLo(-TCU_INFINITY), m_warningHi(TCU_INFINITY) {} + + Interval(bool hasNaN_, double lo_, double hi_, double wlo_, double whi_) + : m_hasNaN(hasNaN_), m_lo(lo_), m_hi(hi_), m_warningLo(wlo_), m_warningHi(whi_) {} Interval (const Interval& a, const Interval& b) - : m_hasNaN (a.m_hasNaN || b.m_hasNaN) - , m_lo (de::min(a.lo(), b.lo())) - , m_hi (de::max(a.hi(), b.hi())) {} + : m_hasNaN (a.m_hasNaN || b.m_hasNaN) + , m_lo (de::min(a.lo(), b.lo())) + , m_hi (de::max(a.hi(), b.hi())) + , m_warningLo (de::min(a.warningLo(), b.warningLo())) + , m_warningHi (de::max(a.warningHi(), b.warningHi())) {} double length (void) const { return m_hi - m_lo; } double lo (void) const { return m_lo; } double hi (void) const { return m_hi; } + double warningLo (void) const { return m_warningLo; } + double warningHi (void) const { return m_warningHi; } 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(); } + void warning (double lo, double hi) + { + m_warningLo = lo; + m_warningHi = hi; + } Interval operator| (const Interval& other) const { return Interval(m_hasNaN || other.m_hasNaN, de::min(m_lo, other.m_lo), - de::max(m_hi, other.m_hi)); - + de::max(m_hi, other.m_hi), + de::min(m_warningLo, other.m_warningLo), + de::max(m_warningHi, other.m_warningHi)); } Interval& operator|= (const Interval& other) @@ -105,7 +123,9 @@ public: { return Interval(m_hasNaN && other.m_hasNaN, de::max(m_lo, other.m_lo), - de::min(m_hi, other.m_hi)); + de::min(m_hi, other.m_hi), + de::max(m_warningLo, other.m_warningLo), + de::min(m_warningHi, other.m_warningHi)); } Interval& operator&= (const Interval& other) @@ -119,6 +139,12 @@ public: (!other.hasNaN() || hasNaN())); } + bool containsWarning(const Interval& other) const + { + return (other.lo() >= warningLo() && other.hi() <= warningHi() && + (!other.hasNaN() || hasNaN())); + } + bool intersects (const Interval& other) const { return ((other.hi() >= lo() && other.lo() <= hi()) || @@ -127,7 +153,7 @@ public: Interval operator- (void) const { - return Interval(hasNaN(), -hi(), -lo()); + return Interval(hasNaN(), -hi(), -lo(), -warningHi(), -warningLo()); } static Interval unbounded (bool nan = false) @@ -151,6 +177,8 @@ private: bool m_hasNaN; double m_lo; double m_hi; + double m_warningLo; + double m_warningHi; } DE_WARN_UNUSED_TYPE; inline Interval operator+ (const Interval& x) { return x; } diff --git a/modules/glshared/glsBuiltinPrecisionTests.cpp b/modules/glshared/glsBuiltinPrecisionTests.cpp index afe990b..34171f6 100644 --- a/modules/glshared/glsBuiltinPrecisionTests.cpp +++ b/modules/glshared/glsBuiltinPrecisionTests.cpp @@ -244,6 +244,13 @@ bool contains (const typename Traits::IVal& ival, const T& value) return Traits::doContains(ival, value); } +//! Returns true iff every element of `ival` contains corresponding element of `value` within the warning interval +template +bool containsWarning(const typename Traits::IVal& ival, const T& value) +{ + return Traits::doContainsWarning(ival, value); +} + //! Print out an interval with the precision of `fmt`. template void printIVal (const FloatFormat& fmt, const typename Traits::IVal& ival, ostream& os) @@ -313,6 +320,11 @@ struct ScalarTraits return a.contains(double(value)); } + static bool doContainsWarning(const Interval& a, T value) + { + return a.containsWarning(double(value)); + } + static Interval doConvert (const FloatFormat& fmt, const IVal& ival) { return fmt.convert(ival); @@ -422,6 +434,15 @@ struct ContainerTraits return true; } + static bool doContainsWarning(const IVal& ival, const T& value) + { + for (int ndx = 0; ndx < T::SIZE; ++ndx) + if (!containsWarning(ival[ndx], value[ndx])) + return false; + + return true; + } + static void doPrintIVal (const FloatFormat& fmt, const IVal ival, ostream& os) { os << "("; @@ -492,11 +513,12 @@ struct Traits { typedef Void IVal; - static Void doMakeIVal (const Void& value) { return value; } - static Void doUnion (const Void&, const Void&) { return Void(); } - static bool doContains (const Void&, Void) { return true; } - static Void doRound (const FloatFormat&, const Void& value) { return value; } - static Void doConvert (const FloatFormat&, const Void& value) { return value; } + static Void doMakeIVal (const Void& value) { return value; } + static Void doUnion (const Void&, const Void&) { return Void(); } + static bool doContains (const Void&, Void) { return true; } + static bool doContainsWarning (const Void&, Void) { return true; } + static Void doRound (const FloatFormat&, const Void& value) { return value; } + static Void doConvert (const FloatFormat&, const Void& value) { return value; } static void doPrintValue (const FloatFormat&, const Void&, ostream& os) { @@ -1702,8 +1724,10 @@ protected: { const double exact = this->applyExact(arg0); const double prec = this->precision(ctx, exact, arg0); - - return exact + Interval(-prec, prec); + const double wprec = this->warningPrecision(ctx, exact, arg0); + Interval ioutput = exact + Interval(-prec, prec); + ioutput.warning(exact - wprec, exact + wprec); + return ioutput; } virtual double applyExact (double) const @@ -1717,6 +1741,12 @@ protected: } virtual double precision (const EvalContext& ctx, double, double) const = 0; + + virtual double warningPrecision (const EvalContext& ctx, double exact, double arg0) const + { + return precision(ctx, exact, arg0); + } + }; class CFloatFunc1 : public FloatFunc1 @@ -2142,6 +2172,21 @@ protected: return 0; } + + // OpenGL API Issue #57 "Clarifying the required ULP precision for GLSL built-in log()". Agreed that + // implementations will be allowed 4 ULPs for HIGHP Log/Log2, but CTS should generate a quality warning. + double warningPrecision(const EvalContext& ctx, double ret, double x) const + { + if (ctx.floatPrecision == glu::PRECISION_HIGHP && x > 0) + { + return (0.5 <= x && x <= 2.0) ? deLdExp(1.0, -21) : ctx.format.ulp(ret, 4.0); + } + else + { + return precision(ctx, ret, x); + } + } + }; class Log2 : public LogFunc { public: Log2 (void) : LogFunc("log2", deLog2) {} }; @@ -4608,7 +4653,10 @@ void PrecisionCase::testStatement (const Variables& variables, // shader output to the reference. for (size_t valueNdx = 0; valueNdx < numValues; valueNdx++) { - bool result = true; + bool result = true; + bool inExpectedRange; + bool inWarningRange; + const char* failStr = "Fail"; typename Traits::IVal reference0; typename Traits::IVal reference1; @@ -4628,15 +4676,39 @@ void PrecisionCase::testStatement (const Variables& variables, switch (outCount) { case 2: - reference1 = convert(highpFmt, env.lookup(*variables.out1)); - if (!m_status.check(contains(reference1, outputs.out1[valueNdx]), - "Shader output 1 is outside acceptable range")) + reference1 = convert(highpFmt, env.lookup(*variables.out1)); + inExpectedRange = contains(reference1, outputs.out1[valueNdx]); + inWarningRange = containsWarning(reference1, outputs.out1[valueNdx]); + if (!inExpectedRange && inWarningRange) + { + m_status.addResult(QP_TEST_RESULT_QUALITY_WARNING, "Shader output 1 has low-quality shader precision"); + failStr = "QualityWarning"; + result = false; + } + else if (!inExpectedRange) + { + m_status.addResult(QP_TEST_RESULT_FAIL, "Shader output 1 is outside acceptable range"); + failStr = "Fail"; result = false; + } + case 1: - reference0 = convert(highpFmt, env.lookup(*variables.out0)); - if (!m_status.check(contains(reference0, outputs.out0[valueNdx]), - "Shader output 0 is outside acceptable range")) + reference0 = convert(highpFmt, env.lookup(*variables.out0)); + inExpectedRange = contains(reference0, outputs.out0[valueNdx]); + inWarningRange = containsWarning(reference0, outputs.out0[valueNdx]); + if (!inExpectedRange && inWarningRange) + { + m_status.addResult(QP_TEST_RESULT_QUALITY_WARNING, "Shader output 0 has low-quality shader precision"); + failStr = "QualityWarning"; result = false; + } + else if (!inExpectedRange) + { + m_status.addResult(QP_TEST_RESULT_FAIL, "Shader output 0 is outside acceptable range"); + failStr = "Fail"; + result = false; + } + default: break; } @@ -4647,7 +4719,7 @@ void PrecisionCase::testStatement (const Variables& variables, { MessageBuilder builder = log().message(); - builder << (result ? "Passed" : "Failed") << " sample:\n"; + builder << (result ? "Passed" : failStr) << " sample:\n"; if (inCount > 0) { @@ -4706,7 +4778,7 @@ void PrecisionCase::testStatement (const Variables& variables, } else { - log() << TestLog::Message << numErrors << "/" << numValues << " inputs failed." + log() << TestLog::Message << numErrors << "/" << numValues << " inputs failed or had quality warnings." << TestLog::EndMessage; } } -- 2.7.4