Fix shader operator tests' precision expectation
authorShahbaz Youssefi <syoussefi@google.com>
Mon, 22 Nov 2021 20:30:26 +0000 (15:30 -0500)
committerMatthew Netsch <quic_mnetsch@quicinc.com>
Thu, 2 Jun 2022 19:05:12 +0000 (19:05 +0000)
es3fShaderOperatorTests.cpp takes the reported precision for lowp and
mediump (through glGetShaderPrecisionFormat) and tests that that many
bits are actually supported by the type.

glGetShaderPrecisionFormat is a legacy function that's not very
meaningful on modern hardware (see Vulkan issue #2931), and there is no
equivalent for its functionality in Vulkan.

This relevant tests are changed such that the test now verifies that _at
least_ that many bits are used in lowp and mediump operations, rather
than _exactly_.  This aligns with the semantics of RelaxedPrecision in
SPIR-V.

Affected tests:
dEQP-GLES3.functional.shaders.operator.*

Components: OpenGL ES
VK-GL-CTS issue: 3344

Change-Id: I88a38d6ee067de8313f25327578c6666c257dc8b
(cherry picked from commit 23d62b971cec55ae49cddc9a9e37614bc9f80668)

modules/gles3/functional/es3fShaderOperatorTests.cpp

index c71eea4..b791894 100644 (file)
@@ -232,6 +232,16 @@ static string stringJoin (const vector<string>& elems, const string& delim)
        return result;
 }
 
+static void stringReplace (string& str, const string& from, const string& to)
+{
+    size_t     start_pos       = 0;
+    while ((start_pos = str.find(from, start_pos)) != std::string::npos)
+       {
+        str.replace(start_pos, from.length(), to);
+        start_pos += to.length();
+    }
+}
+
 static string twoValuedVec4 (const string& first, const string& second, const BVec4& firstMask)
 {
        vector<string> elems(4);
@@ -318,7 +328,7 @@ static inline bool isBoolType (ValueType type)
        return (type & (VALUE_BOOL | VALUE_BOOL_VEC | VALUE_BOOL_GENTYPE)) != 0;
 }
 
-static inline float getGLSLUintMaxAsFloat (const glw::Functions& gl, ShaderType shaderType, Precision uintPrecision)
+static inline int getGLSLUintBits (const glw::Functions& gl, ShaderType shaderType, Precision uintPrecision)
 {
        deUint32 intPrecisionGL;
        deUint32 shaderTypeGL;
@@ -350,8 +360,18 @@ static inline float getGLSLUintMaxAsFloat (const glw::Functions& gl, ShaderType
 
        TCU_CHECK(de::inBounds(range[0], 8, 32));
 
-       const int numBitsInType = range[0] + 1;
-       return (float)((1ull << numBitsInType) - 1);
+       const int       numBitsInType                   = range[0] + 1;
+       return numBitsInType;
+}
+
+static inline float getGLSLUintMaxAsFloat (const glw::Functions& gl, ShaderType shaderType, Precision uintPrecision)
+{
+       const int       numBitsInType                   = getGLSLUintBits(gl, shaderType, uintPrecision);
+       const float     maxAsFloat                              = static_cast<float>((1ull << numBitsInType) - 1);
+       const float     maxRepresentableAsFloat = floorf(nextafterf(maxAsFloat, 0));
+
+       // Not accurate for integers wider than 24 bits.
+       return numBitsInType > 24 ? maxRepresentableAsFloat : maxAsFloat;
 }
 
 // Float scalar that can be either constant or a symbol that can be evaluated later.
@@ -399,6 +419,36 @@ public:
                }
        }
 
+       deUint32 getValueMask (const glw::Functions& gl, ShaderType shaderType) const
+       {
+               if (m_isConstant)
+                       return 0;
+
+               int bits = 0;
+               switch (m_value.symbol)
+               {
+                       case SYMBOL_LOWP_UINT_MAX_RECIPROCAL:
+                       case SYMBOL_LOWP_UINT_MAX:
+                               bits = getGLSLUintBits(gl, shaderType, PRECISION_LOWP);
+                               break;
+
+                       case SYMBOL_MEDIUMP_UINT_MAX_RECIPROCAL:
+                       case SYMBOL_MEDIUMP_UINT_MAX:
+                               bits = getGLSLUintBits(gl, shaderType, PRECISION_MEDIUMP);
+                               break;
+
+                       case SYMBOL_ONE_MINUS_UINT32MAX_DIV_LOWP_UINT_MAX:
+                       case SYMBOL_ONE_MINUS_UINT32MAX_DIV_MEDIUMP_UINT_MAX:
+                               return 0;
+
+                       default:
+                               DE_ASSERT(false);
+                               return 0;
+               }
+
+               return bits == 32 ? 0 : (1u << bits) - 1;
+       }
+
 private:
        bool m_isConstant;
 
@@ -745,22 +795,45 @@ void ShaderOperatorCase::setupShaderData (void)
        frag << "void main()\n";
        frag << "{\n";
 
+       bool    isResFloatVec   = de::inRange<int>(m_spec.output, TYPE_FLOAT, TYPE_FLOAT_VEC4);
+       bool    isResBoolVec    = de::inRange<int>(m_spec.output, TYPE_BOOL, TYPE_BOOL_VEC4);
+       bool    hasReference    = !isResFloatVec && !isResBoolVec && (m_spec.precision == PRECISION_LOWP || m_spec.precision == PRECISION_MEDIUMP);
+       string  refShaderOp             = m_shaderOp;
+
        // Expression inputs.
        string prefix = m_isVertexCase ? "a_" : "v_";
        for (int i = 0; i < m_spec.numInputs; i++)
        {
-               DataType                inType          = m_spec.inputs[i].type;
-               int                             inSize          = getDataTypeScalarSize(inType);
-               bool                    isInt           = de::inRange<int>(inType, TYPE_INT, TYPE_INT_VEC4);
-               bool                    isUint          = de::inRange<int>(inType, TYPE_UINT, TYPE_UINT_VEC4);
-               bool                    isBool          = de::inRange<int>(inType, TYPE_BOOL, TYPE_BOOL_VEC4);
-               const char*             typeName        = getDataTypeName(inType);
-               const char*             swizzle         = s_inSwizzles[i][inSize-1];
-
+               DataType                inType                  = m_spec.inputs[i].type;
+               int                             inSize                  = getDataTypeScalarSize(inType);
+               bool                    isBool                  = de::inRange<int>(inType, TYPE_BOOL, TYPE_BOOL_VEC4);
+               bool                    isInt                   = de::inRange<int>(inType, TYPE_INT, TYPE_INT_VEC4);
+               bool                    isUint                  = de::inRange<int>(inType, TYPE_UINT, TYPE_UINT_VEC4);
+               const char*             typeName                = getDataTypeName(inType);
+               const char*             swizzle                 = s_inSwizzles[i][inSize-1];
+               bool                    hasReferenceIn  = hasReference && !isBool;
+
+               // For int/uint types, generate:
+               //
+               //     highp type highp_inN = ...;
+               //     precision type inN = highp_inN;
+               //
+               // inN_high will be used later for reference checking.
+               //
+               // For other types, generate:
+               //
+               //     precision type inN = ...;
+               //
                op << "\t";
-               if (precision && !isBool) op << precision << " ";
+               if (precision && !isBool)
+               {
+                       if (hasReferenceIn)     op << "highp ";
+                       else                            op << precision << " ";
+               }
 
-               op << typeName << " in" << i << " = ";
+               op << typeName << " ";
+               if (hasReferenceIn)     op << "highp_";
+               op << "in" << i << " = ";
 
                if (isBool)
                {
@@ -781,6 +854,14 @@ void ShaderOperatorCase::setupShaderData (void)
                        op << ")";
 
                op << ";\n";
+
+               if (hasReferenceIn)
+               {
+                       op << "\t" << precision << " " << typeName << " in" << i << " = highp_in" << i << ";\n";
+
+                       string inputName = "in" + string(1, static_cast<char>('0' + i));
+                       stringReplace(refShaderOp, inputName, "highp_" + inputName);
+               }
        }
 
        // Result variable.
@@ -790,27 +871,112 @@ void ShaderOperatorCase::setupShaderData (void)
 
                op << "\t";
                if (precision && !isBoolOut) op << precision << " ";
-               op << outTypeName << " res = " << outTypeName << "(0.0);\n\n";
+               op << outTypeName << " res = " << outTypeName << "(0.0);\n";
+
+               if (hasReference)
+               {
+                       op << "\thighp " << outTypeName << " ref = " << outTypeName << "(0.0);\n";
+               }
+
+               op << "\n";
        }
 
        // Expression.
-       op << "\t" << m_shaderOp << "\n\n";
-
-       // Convert to color.
-       bool    isResFloatVec   = de::inRange<int>(m_spec.output, TYPE_FLOAT, TYPE_FLOAT_VEC4);
+       op << "\t" << m_shaderOp << "\n";
+       if (hasReference)
+       {
+               stringReplace(refShaderOp, "res", "ref");
+               op << "\t" << refShaderOp << "\n";
+       }
+       op << "\n";
+
+       // Implementations may use more bits than advertised.  Assume an implementation advertising 16
+       // bits for mediump, but actually using 24 bits for a particular operation.  We have:
+       //
+       //     highp ref = expr;
+       //     mediump res = expr;
+       //
+       // We expect res&0xFFFF to be correct, because that ensures that _at least_ 16 bits were
+       // provided.  However, we also need to make sure that if there is anything in the upper 16 bits
+       // of res, that those bits match with ref.  In short, we expect to see the following bits
+       // (assume the advertised number of bits is N, and the actual calculation is done in M bits):
+       //
+       //     ref = a31 ... aM aM-1 ... aN aN-1 ... a0
+       //     res =   0 ...  0 bM-1 ... bN bN-1 ... b0
+       //
+       // The test verifies that bN-1 ... b0 is correct based on the shader output.  We additionally
+       // want to make sure that:
+       //
+       //  - bM-1 ... bN is identical to aM-1 ... aN
+       //  - bits above bM-1 are zero.
+       //
+       // This is done as follows:
+       //
+       //     diff = res ^ ref  --> should produce a31 ... aM 0 ... 0
+       //     diff == 0: accept res
+       //     diff != 0:
+       //         lsb = log2((~diff + 1u) & diff)  --> log2(0 .. 0 1 0 ...0)
+       //                                              == findLSB(diff)
+       //
+       //         outOfRangeMask = 0xFFFFFFFF << lsb  --> 1 ... 1 0 ... 0
+       //
+       //         (res & outOfRangeMask) == 0: accept res
+       //
+       // Note that (diff & ~outOfRangeMask) == 0 necessarily holds, because outOfRangeMask has 1s
+       // starting from the first bit that differs between res and ref, which means that res and ref
+       // are identical in those bits.
        int             outScalarSize   = getDataTypeScalarSize(m_spec.output);
+       string floatType = "";
+       if (!isResFloatVec)
+       {
+               if (outScalarSize == 1)
+                       floatType = "float";
+               else
+                       floatType = "vec" + string(1, static_cast<char>('0' + outScalarSize));
+       }
 
-       op << "\thighp vec4 color = vec4(0.0, 0.0, 0.0, 1.0);\n";
-       op << "\tcolor." << s_outSwizzles[outScalarSize-1] << " = ";
+       if (hasReference)
+       {
+               bool    isInt   = de::inRange<int>(m_spec.output, TYPE_INT, TYPE_INT_VEC4);
+               const char* outTypeName = getDataTypeName(m_spec.output);
+               const char* outBasicTypeName = getDataTypeName(isInt ? TYPE_INT : TYPE_UINT);
+               deUint32        resultMask      = m_spec.resultScale.getValueMask(m_renderCtx.getFunctions(), shaderType);
+
+               op << "\thighp " << outTypeName << " diff = res ^ ref;\n";
+               op << "\tdiff = (~diff + " << outTypeName << "(1)) & diff;\n";
+               op << "\thighp " << outTypeName << " lsb = " << outTypeName << "(32);\n";
+               op << "\thighp " << outTypeName << " outOfRangeMask = " << outTypeName << "(0);\n";
+               if (outScalarSize == 1)
+               {
+                       op << "\tif (diff != " << outTypeName << "(0))\n\t{\n";
+                       op << "\t\tlsb = " << outTypeName << "(log2(" << floatType << "(diff)));\n";
+                       op << "\t\toutOfRangeMask = " << outTypeName << "(0xFFFFFFFF) << lsb;\n";
+                       op << "\t}\n";
+               }
+               else
+               {
+                       op << "\tbvec" << outScalarSize << " isDiffZero = equal(diff, " << outTypeName << "(0));\n";
+                       op << "\thighp " << outTypeName << " lsbUnsantized = " << outTypeName << "(log2(vec" << outScalarSize << "((~diff + " << outTypeName << "(1)) & diff)));\n";
+                       for (int channel = 0; channel < outScalarSize; ++channel)
+                       {
+                               op << "\tif (!isDiffZero[" << channel << "])\n\t{\n";
+                               op << "\t\tlsb[" << channel << "] = lsbUnsantized[" << channel << "];\n";
+                               op << "\t\toutOfRangeMask[" << channel << "] = " << outBasicTypeName << "(0xFFFFFFFF) << lsb[" << channel << "];\n";
+                               op << "\t}\n";
+                       }
+               }
+               op << "\thighp " << outTypeName << " outOfRangeRes = res & outOfRangeMask;\n";
+               op << "\tif (outOfRangeRes != " << outTypeName << "(0)) res = " << outTypeName << "(0);\n";
 
-       if (!isResFloatVec && outScalarSize == 1)
-               op << "float(res)";
-       else if (!isResFloatVec)
-               op << "vec" << outScalarSize << "(res)";
-       else
-               op << "res";
+               if (resultMask != 0)
+                       op << "\tres &= " << outTypeName << "(" << resultMask << ");\n";
+
+               op << "\n";
+       }
 
-       op << ";\n";
+       // Convert to color.
+       op << "\thighp vec4 color = vec4(0.0, 0.0, 0.0, 1.0);\n";
+       op << "\tcolor." << s_outSwizzles[outScalarSize-1] << " = " << floatType << "(res);\n";
 
        // Scale & bias.
        float   resultScale             = m_spec.resultScale.getValue(m_renderCtx.getFunctions(), shaderType);
@@ -1928,7 +2094,7 @@ void ShaderOperatorTests::init (void)
                                                        string name = precisionPrefix;
 
                                                        // Generate shader op.
-                                                       string shaderOp = string("res = ");
+                                                       string shaderOp = "res = ";
 
                                                        // Setup shader data info.
                                                        shaderSpec.numInputs            = 0;
@@ -2173,7 +2339,7 @@ void ShaderOperatorTests::init (void)
                                        shaderSpec.inputs[inputNdx] = ShaderValue(type, rangeMin, rangeMax);
                                }
 
-                               string expression = string("") + "res = (" + s_sequenceCases[caseNdx].expressionStr + ");";
+                               string expression = string("res = (") + s_sequenceCases[caseNdx].expressionStr + ");";
 
                                TestCaseGroup* group = s_sequenceCases[caseNdx].containsSideEffects ? sequenceSideEffGroup : sequenceNoSideEffGroup;
                                group->addChild(new ShaderOperatorCase(m_context, name.c_str(), "", isVertexCase, s_sequenceCases[caseNdx].evalFunc, expression.c_str(), shaderSpec));