Relax ULP requirement for Log/Log2
authorJeff Leger <jleger@codeaurora.org>
Fri, 12 Jan 2018 20:26:37 +0000 (15:26 -0500)
committerJeff Leger <jleger@codeaurora.org>
Wed, 31 Jan 2018 19:45:56 +0000 (14:45 -0500)
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
modules/glshared/glsBuiltinPrecisionTests.cpp

index cc271a1..bbbdf3e 100644 (file)
@@ -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; }
index afe990b..34171f6 100644 (file)
@@ -244,6 +244,13 @@ bool contains (const typename Traits<T>::IVal& ival, const T& value)
        return Traits<T>::doContains(ival, value);
 }
 
+//! Returns true iff every element of `ival` contains corresponding element of `value` within the warning interval
+template <typename T>
+bool containsWarning(const typename Traits<T>::IVal& ival, const T& value)
+{
+       return Traits<T>::doContainsWarning(ival, value);
+}
+
 //! Print out an interval with the precision of `fmt`.
 template <typename T>
 void printIVal (const FloatFormat& fmt, const typename Traits<T>::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<Void>
 {
        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<In, Out>&     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<Out0>::IVal     reference0;
                typename Traits<Out1>::IVal     reference1;
 
@@ -4628,15 +4676,39 @@ void PrecisionCase::testStatement (const Variables<In, Out>&    variables,
                switch (outCount)
                {
                        case 2:
-                               reference1 = convert<Out1>(highpFmt, env.lookup(*variables.out1));
-                               if (!m_status.check(contains(reference1, outputs.out1[valueNdx]),
-                                                                       "Shader output 1 is outside acceptable range"))
+                               reference1      = convert<Out1>(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<Out0>(highpFmt, env.lookup(*variables.out0));
-                               if (!m_status.check(contains(reference0, outputs.out0[valueNdx]),
-                                                                       "Shader output 0 is outside acceptable range"))
+                               reference0      = convert<Out0>(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<In, Out>&      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<In, Out>&      variables,
        }
        else
        {
-               log() << TestLog::Message << numErrors << "/" << numValues << " inputs failed."
+               log() << TestLog::Message << numErrors << "/" << numValues << " inputs failed or had quality warnings."
                          << TestLog::EndMessage;
        }
 }