Hi Roland,

It's indeed been long time since I wrote this and I'm no longer certain of what I intended to do, but the change looks overall.

Reviewed-by: Jose Fonseca<jfons...@vmware.com>

But is it really true rand returns 63 bits? I think it return RAND_MAX, which was actually often 31 bits. Perhaps we should add some static asserts in there.

BTW, it would be nice to replace `(unsigned long long)1` with 1ULL in this code while we're at it. I'm not sure if this was due to some MSVC limitation or thinko, but 1ULL should work everywhere and is much shorter and readable.

Jose


On 08/05/18 00:21, srol...@vmware.com wrote:
From: Roland Scheidegger <srol...@vmware.com>

We were never producing negative numbers for signed types.
Also fix only producing half the valid range for uint32, and
properly clamp signed values.

Because this now also properly tests snorm with actually negative
values, need to increase eps for such conversions. I believe these
cannot actually be hit in ordinary operation (e.g. if a snorm texture
is sampled and output to snorm RT, it will still go through snorm->float
and float->snorm conversion), so don't bother to do anything to fix
the bad accuracy (might be quite complex).
Basically, the issue is for something like snorm16->snorm8 that in the
end this will just use a 8 bit arithmetic right shift.
But the math behind it says we should actually do a division by 32767 / 127, 
which
is ~258, not 256. So the result can be one bit off (values have too large
magnitude), and furthermore, the shift has incorrect rounding (always rounds
down). For positive numbers, these errors have different direction, but
for negative ones they have the same, hence for some values the error will
be 2 bit in the end.

Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=106232
---
  src/gallium/drivers/llvmpipe/lp_test_conv.c |  8 ++++++++
  src/gallium/drivers/llvmpipe/lp_test_main.c | 13 +++++++++++--
  2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/src/gallium/drivers/llvmpipe/lp_test_conv.c 
b/src/gallium/drivers/llvmpipe/lp_test_conv.c
index 6e58a03..a4f313a 100644
--- a/src/gallium/drivers/llvmpipe/lp_test_conv.c
+++ b/src/gallium/drivers/llvmpipe/lp_test_conv.c
@@ -211,6 +211,14 @@ test_one(unsigned verbose,
     assert(src_type.length * num_srcs == dst_type.length * num_dsts);
eps = MAX2(lp_const_eps(src_type), lp_const_eps(dst_type));
+   if (dst_type.norm && dst_type.sign && src_type.sign && !src_type.floating) {
+      /*
+       * This is quite inaccurate due to shift being used.
+       * I don't think it's possible to hit such conversions with
+       * llvmpipe though.
+       */
+      eps *= 2;
+   }
context = LLVMContextCreate();
     gallivm = gallivm_create("test_module", context);
diff --git a/src/gallium/drivers/llvmpipe/lp_test_main.c 
b/src/gallium/drivers/llvmpipe/lp_test_main.c
index 518ca27..5ec0dd3 100644
--- a/src/gallium/drivers/llvmpipe/lp_test_main.c
+++ b/src/gallium/drivers/llvmpipe/lp_test_main.c
@@ -147,6 +147,7 @@ write_elem(struct lp_type type, void *dst, unsigned index, 
double value)
        if(type.sign) {
           long long lvalue = (long long)value;
           lvalue = MIN2(lvalue, ((long long)1 << (type.width - 1)) - 1);
+         lvalue = MAX2(lvalue, -((long long)1 << (type.width - 1)));
           switch(type.width) {
           case 8:
              *((int8_t *)dst + index) = (int8_t)lvalue;
@@ -200,16 +201,24 @@ random_elem(struct lp_type type, void *dst, unsigned 
index)
        }
        else {
           unsigned long long mask;
-        if (type.fixed)
+         if (type.fixed)
              mask = ((unsigned long long)1 << (type.width / 2)) - 1;
           else if (type.sign)
              mask = ((unsigned long long)1 << (type.width - 1)) - 1;
           else
              mask = ((unsigned long long)1 << type.width) - 1;
           value += (double)(mask & rand());
+         if (!type.fixed && !type.sign && type.width == 32) {
+            /*
+             * rand only returns half the possible range
+             * XXX 64bit values...
+             */
+            if(rand() & 1)
+               value += (double)0x80000000;
+         }
        }
     }
-   if(!type.sign)
+   if(type.sign)
        if(rand() & 1)
           value = -value;
     write_elem(type, dst, index, value);


_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to