Re: [Piglit] [PATCH] draw-pixels: fix KHR_no_error logic

[Timothy Arceri]

On 23/08/17 17:23, Samuel Pitoiset wrote:



On 08/23/2017 06:11 AM, Timothy Arceri wrote:

---

  This was my fault. The flaw was in my suggestion from the code
  review.


You probably need to use PIGLIT_HAS_ERRORS as well.


I don't think so. This skips testing draws with invalid types, the test 
ran fine for me with this change.






  tests/general/draw-pixels.c | 6 --
  1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tests/general/draw-pixels.c b/tests/general/draw-pixels.c
index 40b4c0b0f..333bb7f86 100644
--- a/tests/general/draw-pixels.c
+++ b/tests/general/draw-pixels.c
@@ -730,22 +730,24 @@ piglit_display(void)
  glPixelStorei(GL_UNPACK_ALIGNMENT, 1);
  for (i = 0; i < ARRAY_SIZE(data_types); i++) {
  for (k = 0; k < ARRAY_SIZE(pixel_ops); k++) {
  for (j = 0; j < ARRAY_SIZE(pixel_formats); j++) {
  format = pixel_formats[j];
  type = data_types[i];
-if (!piglit_khr_no_error &&
-is_format_type_mismatch(format, type)) {
+if (is_format_type_mismatch(format, type)) {
+if (piglit_khr_no_error)
+continue;
+
  glDrawPixels(piglit_width, piglit_height,
   format, type, pixels);
  /* Here GL_INVALID_OPERATION is an
   * expected GL error
   */
  pass = piglit_check_gl_error(
 GL_INVALID_OPERATION)
 && pass;
  continue;
  }


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

[Piglit] [PATCH] image_load_store: add a new test for a bug seen on AMD VEGA.

[Dave Airlie]

From: Dave Airlie 

This is ported from a vulkan cts test showing the same brokenness.
---
 .../execution/image_checkerboard.shader_test   | 67 ++
 1 file changed, 67 insertions(+)
 create mode 100644 
tests/spec/arb_shader_image_load_store/execution/image_checkerboard.shader_test

diff --git 
a/tests/spec/arb_shader_image_load_store/execution/image_checkerboard.shader_test
 
b/tests/spec/arb_shader_image_load_store/execution/image_checkerboard.shader_test
new file mode 100644
index 000..b8d67b0
--- /dev/null
+++ 
b/tests/spec/arb_shader_image_load_store/execution/image_checkerboard.shader_test
@@ -0,0 +1,67 @@
+# This is a port of one of the Vulkan CTS tests to piglit GL
+# to test a bug seen developing radv for vega.
+#
+# 
dEQP-VK.binding_model.shader_access.primary_cmd_buf.storage_image.fragment.single_descriptor.2d
 
+# it isn't a direct port but it shows the same bug.
+#
+# This just copies 4 squares from the image into the final image and probes 
them.
+# On vega some bits are corrupted.
+[require]
+GL >= 3.3
+GLSL >= 3.30
+GL_ARB_shader_image_load_store
+SIZE 256 256
+
+[vertex shader]
+#version 330
+flat out highp int frag_quadrant_id;
+void main (void)
+{
+   highp vec4 result_position;
+   highp int quadrant_id;
+   highp int quadPhase = gl_VertexID % 6;
+   highp int quadXcoord = int(quadPhase == 1 || quadPhase == 4 || 
quadPhase == 5);
+   highp int quadYcoord = int(quadPhase == 2 || quadPhase == 3 || 
quadPhase == 5);
+   highp int quadOriginX = (gl_VertexID / 6) % 2;
+   highp int quadOriginY = (gl_VertexID / 6) / 2;
+   quadrant_id = gl_VertexID / 6;
+   result_position = vec4(float(quadOriginX + quadXcoord - 1), 
float(quadOriginY + quadYcoord - 1), 0.0, 1.0);
+   gl_Position = result_position;
+   frag_quadrant_id = quadrant_id;
+}
+   
+
+[fragment shader]
+#version 330
+#extension GL_ARB_shader_image_load_store: enable
+
+layout(rgba8) readonly uniform highp image2D u_image;
+flat in highp int frag_quadrant_id;
+out mediump vec4 o_color;
+void main (void)
+{
+   highp int quadrant_id = frag_quadrant_id;
+   highp vec4 result_color;
+   if (quadrant_id == 0)
+   result_color = imageLoad(u_image, ivec2(6, 13));
+   else if (quadrant_id == 1)
+   result_color = imageLoad(u_image, ivec2(51, 40));
+   else if (quadrant_id == 2)
+   result_color = imageLoad(u_image, ivec2(42, 26));
+   else
+   result_color = imageLoad(u_image, ivec2(25, 35));
+   o_color.xyzw = result_color.xyzw;
+}
+   
+[test]
+# Use textures that are large enough to actually trigger the use
+# of compression.
+texture rgbw 0 (64, 64) GL_RGBA8
+image texture 0 GL_RGBA8
+
+draw arrays GL_TRIANGLES 0 24
+probe rect rgba (0, 0, 128, 128) (1.0, 0.0, 0.0, 1.0)
+probe rect rgba (128, 0, 128, 128) (1.0, 1.0, 1.0, 1.0)
+probe rect rgba (0, 128, 128, 128) (0.0, 1.0, 0.0, 1.0)
+probe rect rgba (128, 128, 128, 128) (0.0, 0.0, 1.0, 1.0)
+
-- 
2.9.4

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

Re: [Piglit] [PATCH v2 3/3] cl: Replace handwritten vload tests with a generator

[Jan Vesely]

On Wed, 2017-08-23 at 14:54 -0700, Dylan Baker wrote:
> Quoting Jan Vesely (2017-08-23 12:38:53)
> > Hi Dylan,
> > 
> > do you mind taking a look at the python parts?
> > 
> > thanks,
> > Jan
> 
> Sure, I have a few comments, but mostly this looks okay, my comments are 
> mostly
> style nits anyway.
> 
> Dylan
> 
> > On Wed, 2017-08-16 at 20:39 -0400, Jan Vesely wrote:
> > > v2: simplify
> > > mark local storage volatile
> > > Passes on beignet(IVB), and intel CPU
> > > 
> > > Signed-off-by: Jan Vesely 
> > > ---
> > > clover on carrizo passes as well, apart from vload_half tests, because
> > > the function is missing in libclc
> > > 
> > > +
> 
> [snip]
> 
> > > +from __future__ import print_function, division, absolute_import
> > > +import os
> > > +import textwrap
> > > +import random
> 
> please sort the os, textwrap, and radom imports
> 
> > > +
> > > +from six.moves import range
> > > +
> > > +from modules import utils
> > > +
> > > +TYPES = ['char', 'uchar', 'short', 'ushort', 'int', 'uint', 'long', 
> > > 'ulong', 'half', 'float', 'double']
> > > +VEC_SIZES = ['2', '4', '8', '16']
> > > +
> > > +dirName = os.path.join("cl", "vload")
> 
> module level constants should be all caps with underscores like TYPES and
> VEC_SIZES
> 
> > > +
> > > +
> > > +def gen_array(size):
> > > +random.seed(size)
> > > +return [str(random.randint(0, 255)) for i in range(size)]
> > > +
> > > +
> > > +def ext_req(type_name):
> > > +if type_name[:6] == "double":
> > > +return "require_device_extensions: cl_khr_fp64"
> > > +if type_name[:6] == "half":
> 
> Should this be [:4]?
> 
> > > +return "require_device_extensions: cl_khr_fp16"
> > > +return ""
> > > +
> > > +def begin_test(suffix, type_name, mem_type, vec_sizes, addr_space):
> > > +fileName = os.path.join(dirName, 'vload'+ suffix + '-' + type_name + 
> > > '-' + addr_space + '.cl')
> 
> I think that using str.format here is much more readable:
> fileName = os.path.join(dirName, "vload{}-{}-{}.cl".format(suffix, type_name, 
> addr_space))
> 
> Also, could you use file_name instead of fileName, in keeping with our python
> style?
> 
> > > +print(fileName)
> > > +f = open(fileName, 'w')
> > > +f.write(textwrap.dedent(("""
> 
> You can add a \ to the end of """ to avoid an extra newline at the top of the
> file, if you like
> 
> > > +/*!
> > > +[config]
> > > +name: Vector load{suffix} {addr_space} {type_name}2,4,8,16
> > > +clc_version_min: 10
> > > +
> > > +dimensions: 1
> > > +global_size: 1 0 0
> > > +""" + ext_req(type_name))
> > > +.format(type_name=type_name, addr_space=addr_space, suffix=suffix)))
> > > +for s in vec_sizes:
> > > +size = int(s) if s != '' else 1
> > > +data_array = gen_array(size)
> > > +ty_name = type_name + s
> > > +f.write(textwrap.dedent("""
> > > +[test]
> > > +name: vector load{suffix} {addr_space} {type_name}
> > > +kernel_name: vload{suffix}{n}_{addr_space}
> > > +arg_in:  0 buffer {mem_type}[{size}] 0 {gen_array}
> > > +arg_out: 1 buffer {type_name}[2] {first_array} {gen_array}
> > > +
> > > +[test]
> > > +name: vector load{suffix} {addr_space} offset {type_name}
> > > +kernel_name: vload{suffix}{n}_{addr_space}_offset
> > > +arg_in:  0 buffer {mem_type}[{offset_size}] {zeros}{gen_array}
> > > +arg_out: 1 buffer {type_name}[2] {first_array} {gen_array}
> > > +""".format(type_name=ty_name, mem_type=mem_type, size=size + 1,
> > > +   zeros=("0 " * (size + 1)), offset_size=size*2 + 1, 
> > > n=s,
> 
> Spaces around the * operator please (offset_size=size * 2 + 1,)
> 
> > > +   gen_array=' '.join(data_array), suffix=suffix,
> > > +   addr_space=addr_space,
> > > +   first_array="0 " + ' '.join(data_array[:-1]
> > > +
> > > +f.write(textwrap.dedent("""
> > > +!*/
> > > +"""))
> > > +if type_name == "double":
> > > +f.write(textwrap.dedent("""
> > > +#pragma OPENCL EXTENSION cl_khr_fp64: enable
> > > +"""))
> > > +if type_name == "half":
> > > +f.write(textwrap.dedent("""
> > > +#pragma OPENCL EXTENSION cl_khr_fp16: enable
> > > +"""))
> > > +return f
> 
> Two new lines between top level functions and classes please.
> 
> > > +
> > > +def gen_test_constant_global(suffix, t, mem_type, vec_sizes, addr_space):
> > > +f = begin_test(suffix, t, mem_type, vec_sizes, addr_space)
> > > +for s in vec_sizes:
> > > +type_name = t + s
> > > +f.write(textwrap.dedent("""
> > > +kernel void vload{suffix}{n}_{addr_space}({addr_space} 
> > > {mem_type} *in,
> > > + global {type_name} *out) {{
> > > +out[0] = vload{suffix}{n}(0, in);
> > > +out[1] = vload{suffix}{n}(0, in + 1);
> > > +}}
> > 

Re: [Piglit] [PATCH 1/1] cl: Add failing case to exp test

[Jan Vesely]

On Thu, 2017-08-24 at 12:54 -0400, Jan Vesely wrote:
> From: Tom Stellard 

Hi Tom,

this patch was never pushed. Do you remember what hw failed in this
case? Do you mind if I push it?

thanks,
Jan

> 
> ---
>  generated_tests/gen_cl_math_builtins.py | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/generated_tests/gen_cl_math_builtins.py 
> b/generated_tests/gen_cl_math_builtins.py
> index 29b4e1a3f..d1e0e3c24 100644
> --- a/generated_tests/gen_cl_math_builtins.py
> +++ b/generated_tests/gen_cl_math_builtins.py
> @@ -261,8 +261,8 @@ tests = {
>  'arg_types' : [F, F],
>  'function_type': 'ttt',
>  'values' : [
> -[1.0, exp(0.95), exp(pi), exp(-pi), float("inf") ], # Result
> -[0.0, 0.95, pi, -pi, float("inf")]  # Arg0
> +[1.0, exp(0.95), exp(pi), exp(-pi), float("inf"), 
> float.fromhex('0x1.66fe8ap+4')], # Result
> +[0.0, 0.95, pi, -pi, float("inf"), float.fromhex('0x1.8e2cp+1')] 
>  # Arg0
>  ],
>  'tolerance' : 3
>  },


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

[Piglit] [PATCH 1/1] cl: Add failing case to exp test

[Jan Vesely]

From: Tom Stellard 

---
 generated_tests/gen_cl_math_builtins.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/generated_tests/gen_cl_math_builtins.py 
b/generated_tests/gen_cl_math_builtins.py
index 29b4e1a3f..d1e0e3c24 100644
--- a/generated_tests/gen_cl_math_builtins.py
+++ b/generated_tests/gen_cl_math_builtins.py
@@ -261,8 +261,8 @@ tests = {
 'arg_types' : [F, F],
 'function_type': 'ttt',
 'values' : [
-[1.0, exp(0.95), exp(pi), exp(-pi), float("inf") ], # Result
-[0.0, 0.95, pi, -pi, float("inf")]  # Arg0
+[1.0, exp(0.95), exp(pi), exp(-pi), float("inf"), 
float.fromhex('0x1.66fe8ap+4')], # Result
+[0.0, 0.95, pi, -pi, float("inf"), float.fromhex('0x1.8e2cp+1')]  
# Arg0
 ],
 'tolerance' : 3
 },
-- 
2.13.5

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