Re: [Piglit] [PATCH] CL: Fix check of ULP when probing float/double results

2016-01-03 Thread Jan Vesely
Hi Aaron,

On Tue, 2015-12-29 at 12:01 -0600, Aaron Watry wrote:
> Hi Jan/Tom,
> 
> Sorry to resurrect an ancient thread, but I was poking at the piglit
> CL ULP
> issue last night, and thought I'd try to get us closer to a solution
> after
> dropping the matter for too long.
> 
> I've modified your test program with what I was thinking of trying,
> and I
> wouldn't mind your feedback.

thanks for picking it up.

> 
> I realize that we still have issues with discrepancies between
> python-generated expected results based on cpu/rounding mode and the
> , but
> I'd at least like for us to be able to nail down the C portion before
> we
> start redefining all of our expected test results.  In the long term,
> we
> probably want to hand-select our inputs/outputs instead of trusting
> python,
> but I don't necessarily think that we should block fixing our ULP
> calculations on getting that done.

I think the python issue is separate, although related. We should
assume that whatever provides the reference values is reliable.

> 
> For now, I'm still ignoring the half-ULP possibility, and just
> generating a
> minimum/maximum allowed value based on running nextafterf(expected,
> POS/NEG
> INFINITY) in a loop for ULP iterations. Does that sound like a
> tenable
> solution?

I originally considered the same approach, and it has probably the best
effort/correctness ratio. I think we should take something similar
without waiting for a perfect solution. for more details read below.



Here are couple of problems I see:
a) the approach assumes that the nextafter distances are always the
same, which is not always the case.

b) the specs say "The reference value used to compute the ULP value of
an arithmetic operation is the infinitely precise result." which in my
understanding means that we compare GPU (FP representable) result
against infinitely precise mathematical value.

the provided value (python reference - PRef) is representable and
rounded. thus the ULP reference is anywhere in , where Er_{a,b} represents rounding interval values for
above/below, and ULP_{a,b} the corresponding ULP values.

The nextafter approach really checks against N * ULP_a + Er_b, or N *
ULP_b + Er_a. 
Even if ULP_a == ULP_b (in all steps) the value PRef - N * ULP is
correct if the infinitely precise result is in  and
incorrect of it is in , and we have no way to
distinguish these cases.
Thus, if you don't use strict inequalities you already test (N + 0.5)
ULP (since Er_{a,b} <= 0.5 ULP, for round to nearest modes).

Given that we don't have any information about the infinitely precise
result we don't even know what ULP value to use for the test if PRef
happens to be a boundary value.


example:
2^24 is a boundary value for SP float; the neighbouring representable
values are (in order):
..., 2^24 -2, 2^24 -1, 2^24, 2^24 +2, 2^24 +4, ...
Thus if python provides us with PRef == 2^24 using RTNE(*) rounding,
the correct value (Ref) can be anything in <2^24 - 0.5, 2^25>.

If the required accuracy is 2 ULP and Ref == 2^24 - 0.3 (ULP = 1.0),
then the correct test is: GPU_result >= 2^24 - 2 && GPU_result < 2^24 +
2. ( |GPU_Result - Ref| <= 2.0)

if Ref == 2^24 + 0.5 (ULP = 2.0) then the correct test is: GPU_result >
2^24 - 4 && GPU_result <= 2^24 + 4. ( |GPU_Result - Ref| <= 4.0)

Note the strictness of inequalities in both cases. changing precision
to 2.5 ULP would just change the inequalities to non-strict 

regards,
Jan

(*) even is a bit misnomer since all SP floats >= 2.24 are even, I
assume it means that mantissa is even

> 
> --Aaron
> 
> On Sun, Jun 14, 2015 at 4:19 PM, Jan Vesely 
> wrote:
> 
> > On Sat, 2015-06-13 at 21:22 -0500, Aaron Watry wrote:
> > > Meh, this still feels broken.  Give me a bit longer.
> > 
> > and it is :). I don't think this can work based on abs(expected -
> > real),
> > since ULP depends on the magnitude of the numbers. This information
> > is
> > lost after subtraction.
> > 
> > I had an idea some time back to implement this using nextafterf in
> > both
> > directions and checking whether the result falls in that interval.
> > However, there is still a problem. Some of the expected values are
> > already rounded (I'm not sure what rounding mode is used by python
> > by
> > default), but unless it always rounds in one direction, we'll still
> > get
> > slight differences based on whether the actual value was rounded up
> > or
> > down.
> > 
> > I have attached a small test program that shows the deficiencies of
> > fabsf based approaches.
> > 
> > regards,
> > Jan
> > 
> > > 
> > > --Aaron
> > > 
> > > On Sat, Jun 13, 2015 at 2:28 PM, Aaron Watry 
> > > wrote:
> > > 
> > > > We need to actually check against the float value from the
> > > > union,
> > > > instead of just doing (diff > ulp), which seems to cast the
> > > > diff to
> > > > an int before checking against ulp.
> > > > 
> > > > Signed-off-by: Aaron Watry 
> > > > CC: Tom Stellard 
> > > > CC: Jan Vesely 
> > > > ---
> > > >  tests/util/piglit-util-cl.c 

Re: [Piglit] [PATCH 04/11] framework/test/opengl.py: Add FastSkipMixin which checks extensions

2016-01-03 Thread Marek Olšák
On Sun, Jan 3, 2016 at 12:44 AM, Dylan Baker  wrote:
> Not off the top of my head. But I do have a patch Jose reviewed that would
> probably cover the problem, (falling back to not fast skipping). What test
> is it you're running?

The issue seems to happen with all shader tests.

Marek
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH] counters/semantics: change the point emitted from TES that carries info

2016-01-03 Thread Kenneth Graunke
On Sunday, January 3, 2016 3:03:14 AM PST Ilia Mirkin wrote:
> Previously we emitted the point where x == y == 0. This was the first
> point and was always overwritten. Instead emit the x == 1, y == 0 point
> which is the last emitted in a ccw ordering.
> 
> Signed-off-by: Ilia Mirkin 
> ---
>  tests/spec/arb_shader_atomic_counters/semantics.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/spec/arb_shader_atomic_counters/semantics.c b/tests/spec/
arb_shader_atomic_counters/semantics.c
> index 921c0e8..931f030 100644
> --- a/tests/spec/arb_shader_atomic_counters/semantics.c
> +++ b/tests/spec/arb_shader_atomic_counters/semantics.c
> @@ -338,7 +338,7 @@ run_test_tess_evaluation(void)
>  "   gl_in[1].gl_Position * gl_TessCoord.y +\n"
>  "   gl_in[2].gl_Position * gl_TessCoord.z;\n"
>  "   \n"
> -"   if (gl_TessCoord.z == 1.0) {\n"
> +"   if (gl_TessCoord.y == 0.0 && gl_TessCoord.x == 1.0) 
{\n"
>  "   tecolor.x = int(atomicCounterDecrement(x));
\n"
>  "   tecolor.y = int(atomicCounterIncrement(x));
\n"
>  "   tecolor.z = int(atomicCounterIncrement(x));
\n"
> 

Thanks!  This makes sense.  It also fixes the test on i965.

This test was mentioned in Mesa bug 93542:
https://bugs.freedesktop.org/show_bug.cgi?id=93542
but I'm not sure if it's worth referencing.  Up to you.

Reviewed-and-tested-by: Kenneth Graunke 


signature.asc
Description: This is a digitally signed message part.
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


[Piglit] [PATCH] counters/semantics: change the point emitted from TES that carries info

2016-01-03 Thread Ilia Mirkin
Previously we emitted the point where x == y == 0. This was the first
point and was always overwritten. Instead emit the x == 1, y == 0 point
which is the last emitted in a ccw ordering.

Signed-off-by: Ilia Mirkin 
---
 tests/spec/arb_shader_atomic_counters/semantics.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/spec/arb_shader_atomic_counters/semantics.c 
b/tests/spec/arb_shader_atomic_counters/semantics.c
index 921c0e8..931f030 100644
--- a/tests/spec/arb_shader_atomic_counters/semantics.c
+++ b/tests/spec/arb_shader_atomic_counters/semantics.c
@@ -338,7 +338,7 @@ run_test_tess_evaluation(void)
 "   gl_in[1].gl_Position * gl_TessCoord.y +\n"
 "   gl_in[2].gl_Position * gl_TessCoord.z;\n"
 "   \n"
-"   if (gl_TessCoord.z == 1.0) {\n"
+"   if (gl_TessCoord.y == 0.0 && gl_TessCoord.x == 1.0) 
{\n"
 "   tecolor.x = int(atomicCounterDecrement(x));\n"
 "   tecolor.y = int(atomicCounterIncrement(x));\n"
 "   tecolor.z = int(atomicCounterIncrement(x));\n"
-- 
2.4.10

___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit