Make ARB_shader_precision tests ignore 0.0 vs -0.0.

Submitted by Kenneth Graunke on May 16, 2016, 10:31 p.m.

Details

Message ID 1463437899-16204-1-git-send-email-kenneth@whitecape.org
State New
Headers show
Series "Make ARB_shader_precision tests ignore 0.0 vs -0.0." ( rev: 1 ) in Piglit

Not browsing as part of any series.

Commit Message

Kenneth Graunke May 16, 2016, 10:31 p.m.
The multiplication precision tests were attempting to multiply
0.0 * -0.1 and expecting to get +0.0 (0x00000000), and failing
if they received -0.0 (0x80000000).  This seems fairly bogus.

According to the ARB_shader_precision specification:
"In general, correct signedness of 0 is not required."

To avoid this problem, remove the sign bit from both the results
and expected values when the value is equal to (+/-) 0.0.

Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
---
 generated_tests/gen_shader_precision_tests.py                | 7 ++++++-
 generated_tests/templates/gen_shader_precision_tests/fs.mako | 4 ++--
 generated_tests/templates/gen_shader_precision_tests/gs.mako | 4 ++--
 generated_tests/templates/gen_shader_precision_tests/vs.mako | 4 ++--
 4 files changed, 12 insertions(+), 7 deletions(-)

Patch hide | download patch | download mbox

diff --git a/generated_tests/gen_shader_precision_tests.py b/generated_tests/gen_shader_precision_tests.py
index 43b6a25..60a6f4f 100644
--- a/generated_tests/gen_shader_precision_tests.py
+++ b/generated_tests/gen_shader_precision_tests.py
@@ -206,6 +206,10 @@  def shader_runner_format(values):
 
     return retval
 
+def drop_signbit(xs):
+    if (np.isscalar(xs)):
+        return 0.0 if xs == 0.0 else xs
+    return np.array([0.0 if x == 0.0 else x for x in xs], xs.dtype)
 
 def main():
     """ Main function """
@@ -225,7 +229,8 @@  def main():
             complex_tol_type = signature.rettype
             for test_vector in test_vectors:
                 tolerance = _gen_tolerance(signature.name, signature.rettype, test_vector.arguments)
-                refined_test_vectors.append(TestVector(test_vector.arguments, test_vector.result, tolerance))
+                result = drop_signbit(test_vector.result)
+                refined_test_vectors.append(TestVector(test_vector.arguments, result, tolerance))
             # Then generate the shader_test scripts
             for shader_stage in ('vs', 'fs', 'gs'):
                 template = template_file('gen_shader_precision_tests', '{0}.mako'.format(shader_stage))
diff --git a/generated_tests/templates/gen_shader_precision_tests/fs.mako b/generated_tests/templates/gen_shader_precision_tests/fs.mako
index 0a66efa..a556bad 100644
--- a/generated_tests/templates/gen_shader_precision_tests/fs.mako
+++ b/generated_tests/templates/gen_shader_precision_tests/fs.mako
@@ -32,7 +32,7 @@  void main()
     ## build an array of bit-level representations of the floating point results calculated above
     ##
   int resultbits[${num_elements}] = int[${num_elements}](\
-${', '.join('floatBitsToInt(result{0})'.format(i) for i in indexers)}\
+${', '.join('result{0} == 0.0 ? 0x0 : floatBitsToInt(result{0})'.format(i) for i in indexers)}\
 );
     ##
     ## build an array of bit-level representations of the passed-in floating point expected results
@@ -84,7 +84,7 @@  max(ulps${indexers[len(indexers)-2]}, ulps${indexers[len(indexers)-1]})\
     ##
     ## if there is only a single result value generated, compare it directly
     ##
-  int resultbits = floatBitsToInt(result);
+  int resultbits = result == 0.0 ? 0x0 : floatBitsToInt(result);
   int expectedbits = floatBitsToInt(expected);
   bool signerr = resultbits>>31 != expectedbits>>31;
   float ulps = abs(resultbits - expectedbits);
diff --git a/generated_tests/templates/gen_shader_precision_tests/gs.mako b/generated_tests/templates/gen_shader_precision_tests/gs.mako
index a480594..d37a42b 100644
--- a/generated_tests/templates/gen_shader_precision_tests/gs.mako
+++ b/generated_tests/templates/gen_shader_precision_tests/gs.mako
@@ -43,7 +43,7 @@  void main()
     ## build an array of bit-level representations of the floating point results calculated above
     ##
   int resultbits[${num_elements}] = int[${num_elements}](\
-${', '.join('floatBitsToInt(result{0})'.format(i) for i in indexers)}\
+${', '.join('result{0} == 0.0 ? 0x0 : floatBitsToInt(result{0})'.format(i) for i in indexers)}\
 );
     ##
     ## build an array of bit-level representations of the passed-in floating point expected results
@@ -95,7 +95,7 @@  max(ulps${indexers[len(indexers)-2]}, ulps${indexers[len(indexers)-1]})\
     ##
     ## if there is only a single result value generated, compare it directly
     ##
-  int resultbits = floatBitsToInt(result);
+  int resultbits = result == 0.0 ? 0x0 : floatBitsToInt(result);
   int expectedbits = floatBitsToInt(expected);
   bool signerr = resultbits>>31 != expectedbits>>31;
   float ulps = abs(resultbits - expectedbits);
diff --git a/generated_tests/templates/gen_shader_precision_tests/vs.mako b/generated_tests/templates/gen_shader_precision_tests/vs.mako
index e7282f5..7ccdadc 100644
--- a/generated_tests/templates/gen_shader_precision_tests/vs.mako
+++ b/generated_tests/templates/gen_shader_precision_tests/vs.mako
@@ -33,7 +33,7 @@  void main()
     ## build an array of bit-level representations of the floating point results calculated above
     ##
   int resultbits[${num_elements}] = int[${num_elements}](\
-${', '.join('floatBitsToInt(result{0})'.format(i) for i in indexers)}\
+${', '.join('result{0} == 0.0 ? 0x0 : floatBitsToInt(result{0})'.format(i) for i in indexers)}\
 );
     ##
     ## build an array of bit-level representations of the passed-in floating point expected results
@@ -85,7 +85,7 @@  max(ulps${indexers[len(indexers)-2]}, ulps${indexers[len(indexers)-1]})\
     ##
     ## if there is only a single result value generated, compare it directly
     ##
-  int resultbits = floatBitsToInt(result);
+  int resultbits = result == 0.0 ? 0x0 : floatBitsToInt(result);
   int expectedbits = floatBitsToInt(expected);
   bool signerr = resultbits>>31 != expectedbits>>31;
   float ulps = abs(resultbits - expectedbits);

Comments

Quoting Kenneth Graunke (2016-05-16 15:31:39)
> The multiplication precision tests were attempting to multiply
> 0.0 * -0.1 and expecting to get +0.0 (0x00000000), and failing
> if they received -0.0 (0x80000000).  This seems fairly bogus.
> 
> According to the ARB_shader_precision specification:
> "In general, correct signedness of 0 is not required."
> 
> To avoid this problem, remove the sign bit from both the results
> and expected values when the value is equal to (+/-) 0.0.
> 
> Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
> ---
>  generated_tests/gen_shader_precision_tests.py                | 7 ++++++-
>  generated_tests/templates/gen_shader_precision_tests/fs.mako | 4 ++--
>  generated_tests/templates/gen_shader_precision_tests/gs.mako | 4 ++--
>  generated_tests/templates/gen_shader_precision_tests/vs.mako | 4 ++--
>  4 files changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/generated_tests/gen_shader_precision_tests.py b/generated_tests/gen_shader_precision_tests.py
> index 43b6a25..60a6f4f 100644
> --- a/generated_tests/gen_shader_precision_tests.py
> +++ b/generated_tests/gen_shader_precision_tests.py
> @@ -206,6 +206,10 @@ def shader_runner_format(values):
>  
>      return retval
>  
> +def drop_signbit(xs):
> +    if (np.isscalar(xs)):
> +        return 0.0 if xs == 0.0 else xs

Is it important to ensure that the returned type doesn't change? I think
for correctness we probably want to return xs.dtype(0.0) instead, but I
guess it doesn't strictly matter.

> +    return np.array([0.0 if x == 0.0 else x for x in xs], xs.dtype)
>  
>  def main():
>      """ Main function """
> @@ -225,7 +229,8 @@ def main():
>              complex_tol_type = signature.rettype
>              for test_vector in test_vectors:
>                  tolerance = _gen_tolerance(signature.name, signature.rettype, test_vector.arguments)
> -                refined_test_vectors.append(TestVector(test_vector.arguments, test_vector.result, tolerance))
> +                result = drop_signbit(test_vector.result)
> +                refined_test_vectors.append(TestVector(test_vector.arguments, result, tolerance))
>              # Then generate the shader_test scripts
>              for shader_stage in ('vs', 'fs', 'gs'):
>                  template = template_file('gen_shader_precision_tests', '{0}.mako'.format(shader_stage))
> diff --git a/generated_tests/templates/gen_shader_precision_tests/fs.mako b/generated_tests/templates/gen_shader_precision_tests/fs.mako
> index 0a66efa..a556bad 100644
> --- a/generated_tests/templates/gen_shader_precision_tests/fs.mako
> +++ b/generated_tests/templates/gen_shader_precision_tests/fs.mako
> @@ -32,7 +32,7 @@ void main()
>      ## build an array of bit-level representations of the floating point results calculated above
>      ##
>    int resultbits[${num_elements}] = int[${num_elements}](\
> -${', '.join('floatBitsToInt(result{0})'.format(i) for i in indexers)}\
> +${', '.join('result{0} == 0.0 ? 0x0 : floatBitsToInt(result{0})'.format(i) for i in indexers)}\
>  );
>      ##
>      ## build an array of bit-level representations of the passed-in floating point expected results
> @@ -84,7 +84,7 @@ max(ulps${indexers[len(indexers)-2]}, ulps${indexers[len(indexers)-1]})\
>      ##
>      ## if there is only a single result value generated, compare it directly
>      ##
> -  int resultbits = floatBitsToInt(result);
> +  int resultbits = result == 0.0 ? 0x0 : floatBitsToInt(result);
>    int expectedbits = floatBitsToInt(expected);
>    bool signerr = resultbits>>31 != expectedbits>>31;
>    float ulps = abs(resultbits - expectedbits);
> diff --git a/generated_tests/templates/gen_shader_precision_tests/gs.mako b/generated_tests/templates/gen_shader_precision_tests/gs.mako
> index a480594..d37a42b 100644
> --- a/generated_tests/templates/gen_shader_precision_tests/gs.mako
> +++ b/generated_tests/templates/gen_shader_precision_tests/gs.mako
> @@ -43,7 +43,7 @@ void main()
>      ## build an array of bit-level representations of the floating point results calculated above
>      ##
>    int resultbits[${num_elements}] = int[${num_elements}](\
> -${', '.join('floatBitsToInt(result{0})'.format(i) for i in indexers)}\
> +${', '.join('result{0} == 0.0 ? 0x0 : floatBitsToInt(result{0})'.format(i) for i in indexers)}\
>  );
>      ##
>      ## build an array of bit-level representations of the passed-in floating point expected results
> @@ -95,7 +95,7 @@ max(ulps${indexers[len(indexers)-2]}, ulps${indexers[len(indexers)-1]})\
>      ##
>      ## if there is only a single result value generated, compare it directly
>      ##
> -  int resultbits = floatBitsToInt(result);
> +  int resultbits = result == 0.0 ? 0x0 : floatBitsToInt(result);
>    int expectedbits = floatBitsToInt(expected);
>    bool signerr = resultbits>>31 != expectedbits>>31;
>    float ulps = abs(resultbits - expectedbits);
> diff --git a/generated_tests/templates/gen_shader_precision_tests/vs.mako b/generated_tests/templates/gen_shader_precision_tests/vs.mako
> index e7282f5..7ccdadc 100644
> --- a/generated_tests/templates/gen_shader_precision_tests/vs.mako
> +++ b/generated_tests/templates/gen_shader_precision_tests/vs.mako
> @@ -33,7 +33,7 @@ void main()
>      ## build an array of bit-level representations of the floating point results calculated above
>      ##
>    int resultbits[${num_elements}] = int[${num_elements}](\
> -${', '.join('floatBitsToInt(result{0})'.format(i) for i in indexers)}\
> +${', '.join('result{0} == 0.0 ? 0x0 : floatBitsToInt(result{0})'.format(i) for i in indexers)}\
>  );
>      ##
>      ## build an array of bit-level representations of the passed-in floating point expected results
> @@ -85,7 +85,7 @@ max(ulps${indexers[len(indexers)-2]}, ulps${indexers[len(indexers)-1]})\
>      ##
>      ## if there is only a single result value generated, compare it directly
>      ##
> -  int resultbits = floatBitsToInt(result);
> +  int resultbits = result == 0.0 ? 0x0 : floatBitsToInt(result);
>    int expectedbits = floatBitsToInt(expected);
>    bool signerr = resultbits>>31 != expectedbits>>31;
>    float ulps = abs(resultbits - expectedbits);
> -- 
> 2.8.2
> 
> _______________________________________________
> Piglit mailing list
> Piglit@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/piglit

There's the one nit, but either way,
Reviewed-by: Dylan Baker <dylan@pnwbakers.com>