Re: [Piglit] [PATCH] arb_get_program_binary: test restore of SSO program
Reviewed-by: Alejandro Piñeiro
On 11/7/19 2:38, Timothy Arceri wrote:
This tests that a restored SSO doesn't fail validation once
restored. This was happening in Mesa because we weren't storing
the state of the program parameter PROGRAM_SEPARABLE.
---
tests/opengl.py | 2 +
.../arb_get_program_binary/CMakeLists.gl.txt | 2 +
.../spec/arb_get_program_binary/gpb-common.c | 39
.../spec/arb_get_program_binary/gpb-common.h | 3 +
.../restore-sso-program.c | 94 +++
5 files changed, 140 insertions(+)
create mode 100644 tests/spec/arb_get_program_binary/restore-sso-program.c
diff --git a/tests/opengl.py b/tests/opengl.py
index 6dd27e9d4..adf5a5312 100644
--- a/tests/opengl.py
+++ b/tests/opengl.py
@@ -1715,6 +1715,8 @@ with profile.test_list.group_manager(
'xfb-varyings')
g(['arb_get_program_binary-restore-implicit-use-program'],
'restore-implicit-use-program')
+g(['arb_get_program_binary-restore-sso-program'],
+ 'restore-sso-program')
g(['arb_get_program_binary-reset-uniform'],
'reset-uniform')
diff --git a/tests/spec/arb_get_program_binary/CMakeLists.gl.txt b/tests/spec/arb_get_program_binary/CMakeLists.gl.txt
index ea43ba9ae..3ef48dbe0 100644
--- a/tests/spec/arb_get_program_binary/CMakeLists.gl.txt
+++ b/tests/spec/arb_get_program_binary/CMakeLists.gl.txt
@@ -14,6 +14,8 @@ piglit_add_executable
(arb_get_program_binary-retrievable_hint retrievable_hint.
piglit_add_executable (arb_get_program_binary-reset-uniform reset-uniform.c
gpb-common.c)
piglit_add_executable (arb_get_program_binary-restore-implicit-use-program
restore-implicit-use-program.c gpb-common.c)
+piglit_add_executable (arb_get_program_binary-restore-sso-program
+ restore-sso-program.c gpb-common.c)
piglit_add_executable (arb_get_program_binary-xfb-varyings xfb-varyings.c
gpb-common.c)
# vim: ft=cmake:
diff --git a/tests/spec/arb_get_program_binary/gpb-common.c
b/tests/spec/arb_get_program_binary/gpb-common.c
index 425b93673..438e4992d 100644
--- a/tests/spec/arb_get_program_binary/gpb-common.c
+++ b/tests/spec/arb_get_program_binary/gpb-common.c
@@ -127,3 +127,42 @@ gpb_save_restore(GLuint *prog)
return true;
}
+
+bool
+gpb_save_restore_sso(GLuint *prog, GLuint pipeline, GLbitfield stage)
+{
+ GLsizei bin_length;
+ void *binary;
+ GLenum bin_format;
+ GLuint new_prog;
+
+ if (!gpb_save_program(*prog, &binary, &bin_length, &bin_format)) {
+ fprintf(stderr,
+ "failed to save program with GetProgramBinary\n");
+ piglit_report_result(PIGLIT_FAIL);
+ }
+
+ new_prog = glCreateProgram();
+ if (!piglit_check_gl_error(GL_NO_ERROR)) {
+ free(binary);
+ piglit_report_result(PIGLIT_FAIL);
+ }
+
+ if (!gpb_restore_program(new_prog, binary, bin_length, bin_format)) {
+ free(binary);
+ fprintf(stderr, "failed to restore binary program\n");
+ piglit_report_result(PIGLIT_FAIL);
+ }
+ free(binary);
+
+ glUseProgramStages(pipeline, stage, new_prog);
+ if (!piglit_check_gl_error(GL_NO_ERROR))
+ piglit_report_result(PIGLIT_FAIL);
+
+ glDeleteProgram(*prog);
+ if (!piglit_check_gl_error(GL_NO_ERROR))
+ piglit_report_result(PIGLIT_FAIL);
+ *prog = new_prog;
+
+ return true;
+}
diff --git a/tests/spec/arb_get_program_binary/gpb-common.h
b/tests/spec/arb_get_program_binary/gpb-common.h
index f471241aa..ae10f9e3a 100644
--- a/tests/spec/arb_get_program_binary/gpb-common.h
+++ b/tests/spec/arb_get_program_binary/gpb-common.h
@@ -34,4 +34,7 @@ gpb_restore_program(GLuint prog, void *binary, GLsizei
length, GLenum format);
bool
gpb_save_restore(GLuint *prog);
+bool
+gpb_save_restore_sso(GLuint *prog, GLuint pipeline, GLbitfield stage);
+
#endif
diff --git a/tests/spec/arb_get_program_binary/restore-sso-program.c
b/tests/spec/arb_get_program_binary/restore-sso-program.c
new file mode 100644
index 0..23d9998c1
--- /dev/null
+++ b/tests/spec/arb_get_program_binary/restore-sso-program.c
@@ -0,0 +1,94 @@
+/*
+ * Copyright (c) 2019 Timothy Arceri
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROV
Re: [Piglit] [PATCH] ssbo/shared: fix min/max tests to specify std430
Reviewed-by: Alejandro Piñeiro
On 26/6/19 4:23, Dave Airlie wrote:
From: Dave Airlie
These tests preinit the ssbo contents but expect the driver to
be doing std430 packing by default, just specify std430 packing.
---
.../execution/shared-atomicMax-int.shader_test | 2 +-
.../execution/shared-atomicMax-uint.shader_test | 2 +-
.../execution/shared-atomicMin-int.shader_test | 2 +-
.../execution/shared-atomicMin-uint.shader_test | 2 +-
.../execution/ssbo-atomicMax-int.shader_test| 2 +-
.../execution/ssbo-atomicMax-uint.shader_test | 2 +-
.../execution/ssbo-atomicMin-int.shader_test| 2 +-
.../execution/ssbo-atomicMin-uint.shader_test | 2 +-
8 files changed, 8 insertions(+), 8 deletions(-)
diff --git
a/tests/spec/arb_compute_shader/execution/shared-atomicMax-int.shader_test
b/tests/spec/arb_compute_shader/execution/shared-atomicMax-int.shader_test
index 642e417d0..cc2abc2c0 100644
--- a/tests/spec/arb_compute_shader/execution/shared-atomicMax-int.shader_test
+++ b/tests/spec/arb_compute_shader/execution/shared-atomicMax-int.shader_test
@@ -13,7 +13,7 @@ GL_ARB_shader_atomic_counters
layout(local_size_x = 64) in;
-layout(binding = 0) buffer bufblock {
+layout(binding = 0, std430) buffer bufblock {
int source_array[64];
int source_value;
};
diff --git
a/tests/spec/arb_compute_shader/execution/shared-atomicMax-uint.shader_test
b/tests/spec/arb_compute_shader/execution/shared-atomicMax-uint.shader_test
index 8264653e0..a228ecc73 100644
--- a/tests/spec/arb_compute_shader/execution/shared-atomicMax-uint.shader_test
+++ b/tests/spec/arb_compute_shader/execution/shared-atomicMax-uint.shader_test
@@ -13,7 +13,7 @@ GL_ARB_shader_atomic_counters
layout(local_size_x = 64) in;
-layout(binding = 0) buffer bufblock {
+layout(binding = 0, std430) buffer bufblock {
uint source_array[64];
uint source_value;
};
diff --git
a/tests/spec/arb_compute_shader/execution/shared-atomicMin-int.shader_test
b/tests/spec/arb_compute_shader/execution/shared-atomicMin-int.shader_test
index c06bd0ed3..ddaa1588f 100644
--- a/tests/spec/arb_compute_shader/execution/shared-atomicMin-int.shader_test
+++ b/tests/spec/arb_compute_shader/execution/shared-atomicMin-int.shader_test
@@ -13,7 +13,7 @@ GL_ARB_shader_atomic_counters
layout(local_size_x = 64) in;
-layout(binding = 0) buffer bufblock {
+layout(binding = 0, std430) buffer bufblock {
int source_array[64];
int source_value;
};
diff --git
a/tests/spec/arb_compute_shader/execution/shared-atomicMin-uint.shader_test
b/tests/spec/arb_compute_shader/execution/shared-atomicMin-uint.shader_test
index 93b799327..d1dd6cdea 100644
--- a/tests/spec/arb_compute_shader/execution/shared-atomicMin-uint.shader_test
+++ b/tests/spec/arb_compute_shader/execution/shared-atomicMin-uint.shader_test
@@ -13,7 +13,7 @@ GL_ARB_shader_atomic_counters
layout(local_size_x = 64) in;
-layout(binding = 0) buffer bufblock {
+layout(binding = 0, std430) buffer bufblock {
uint source_array[64];
uint source_value;
};
diff --git
a/tests/spec/arb_shader_storage_buffer_object/execution/ssbo-atomicMax-int.shader_test
b/tests/spec/arb_shader_storage_buffer_object/execution/ssbo-atomicMax-int.shader_test
index b7d59a328..29f181799 100644
---
a/tests/spec/arb_shader_storage_buffer_object/execution/ssbo-atomicMax-int.shader_test
+++
b/tests/spec/arb_shader_storage_buffer_object/execution/ssbo-atomicMax-int.shader_test
@@ -11,7 +11,7 @@ GL_ARB_shader_atomic_counters
#extension GL_ARB_shader_storage_buffer_object: require
#extension GL_ARB_shader_atomic_counters: require
-layout(binding = 0) buffer bufblock {
+layout(binding = 0, std430) buffer bufblock {
int array[64];
int value;
};
diff --git
a/tests/spec/arb_shader_storage_buffer_object/execution/ssbo-atomicMax-uint.shader_test
b/tests/spec/arb_shader_storage_buffer_object/execution/ssbo-atomicMax-uint.shader_test
index c65600fad..e0da2bf88 100644
---
a/tests/spec/arb_shader_storage_buffer_object/execution/ssbo-atomicMax-uint.shader_test
+++
b/tests/spec/arb_shader_storage_buffer_object/execution/ssbo-atomicMax-uint.shader_test
@@ -11,7 +11,7 @@ GL_ARB_shader_atomic_counters
#extension GL_ARB_shader_storage_buffer_object: require
#extension GL_ARB_shader_atomic_counters: require
-layout(binding = 0) buffer bufblock {
+layout(binding = 0, std430) buffer bufblock {
uint array[64];
uint value;
};
diff --git
a/tests/spec/arb_shader_storage_buffer_object/execution/ssbo-atomicMin-int.shader_test
b/tests/spec/arb_shader_storage_buffer_object/execution/ssbo-atomicMin-int.shader_test
index ca83af48a..422782c2b 100644
---
a/tests/spec/arb_shader_storage_buffer_object/execution/ssbo-atomicMin-int.shader_test
+++
b/tests/spec/arb_shader_storage_buffer_object
Re: [Piglit] [PATCH] ssbo: fix min/max tests to specify std430
Reviewed-by: Alejandro Piñeiro
On 26/6/19 4:15, Dave Airlie wrote:
From: Dave Airlie
These tests preinit the ssbo contents but expect the driver to
be doing std430 packing by default, just specify std430 packing.
This fixes the tests on softpipe
---
.../execution/ssbo-atomicMax-int.shader_test| 2 +-
.../execution/ssbo-atomicMax-uint.shader_test | 2 +-
.../execution/ssbo-atomicMin-int.shader_test| 2 +-
.../execution/ssbo-atomicMin-uint.shader_test | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git
a/tests/spec/arb_shader_storage_buffer_object/execution/ssbo-atomicMax-int.shader_test
b/tests/spec/arb_shader_storage_buffer_object/execution/ssbo-atomicMax-int.shader_test
index b7d59a328..29f181799 100644
---
a/tests/spec/arb_shader_storage_buffer_object/execution/ssbo-atomicMax-int.shader_test
+++
b/tests/spec/arb_shader_storage_buffer_object/execution/ssbo-atomicMax-int.shader_test
@@ -11,7 +11,7 @@ GL_ARB_shader_atomic_counters
#extension GL_ARB_shader_storage_buffer_object: require
#extension GL_ARB_shader_atomic_counters: require
-layout(binding = 0) buffer bufblock {
+layout(binding = 0, std430) buffer bufblock {
int array[64];
int value;
};
diff --git
a/tests/spec/arb_shader_storage_buffer_object/execution/ssbo-atomicMax-uint.shader_test
b/tests/spec/arb_shader_storage_buffer_object/execution/ssbo-atomicMax-uint.shader_test
index c65600fad..e0da2bf88 100644
---
a/tests/spec/arb_shader_storage_buffer_object/execution/ssbo-atomicMax-uint.shader_test
+++
b/tests/spec/arb_shader_storage_buffer_object/execution/ssbo-atomicMax-uint.shader_test
@@ -11,7 +11,7 @@ GL_ARB_shader_atomic_counters
#extension GL_ARB_shader_storage_buffer_object: require
#extension GL_ARB_shader_atomic_counters: require
-layout(binding = 0) buffer bufblock {
+layout(binding = 0, std430) buffer bufblock {
uint array[64];
uint value;
};
diff --git
a/tests/spec/arb_shader_storage_buffer_object/execution/ssbo-atomicMin-int.shader_test
b/tests/spec/arb_shader_storage_buffer_object/execution/ssbo-atomicMin-int.shader_test
index ca83af48a..422782c2b 100644
---
a/tests/spec/arb_shader_storage_buffer_object/execution/ssbo-atomicMin-int.shader_test
+++
b/tests/spec/arb_shader_storage_buffer_object/execution/ssbo-atomicMin-int.shader_test
@@ -12,7 +12,7 @@ GL_ARB_shader_atomic_counters
#extension GL_ARB_shader_atomic_counters: require
#line 100
-layout(binding = 0) buffer bufblock {
+layout(binding = 0, std430) buffer bufblock {
int array[64];
int value;
};
diff --git
a/tests/spec/arb_shader_storage_buffer_object/execution/ssbo-atomicMin-uint.shader_test
b/tests/spec/arb_shader_storage_buffer_object/execution/ssbo-atomicMin-uint.shader_test
index fbae509d8..8af265c60 100644
---
a/tests/spec/arb_shader_storage_buffer_object/execution/ssbo-atomicMin-uint.shader_test
+++
b/tests/spec/arb_shader_storage_buffer_object/execution/ssbo-atomicMin-uint.shader_test
@@ -11,7 +11,7 @@ GL_ARB_shader_atomic_counters
#extension GL_ARB_shader_storage_buffer_object: require
#extension GL_ARB_shader_atomic_counters: require
-layout(binding = 0) buffer bufblock {
+layout(binding = 0, std430) buffer bufblock {
uint array[64];
uint value;
};
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [PATCH] tests/deqp: use --deqp-case when generating case-list
I have realized that this solution have a big problem. You can pass extra args to deqp, so several people are already using it to pass --deqp-case through extra args. This patch would affect them. So Im dropping this patch. sorry for the noise. On 6/6/19 14:48, Alejandro Piñeiro wrote: Without this commit, generating the case list is called without any parameter by default, so it would generate all the caselist, even if you don't need it, or if you have just called the method before (like in khr_gles where you call several time gen_caselist). For the latter an alternative would just check if the caselist files are already created, and return if they exist. But in that case, we would need to add a mechanism to force creating them. --- framework/test/deqp.py | 6 -- tests/cts_gl.py| 20 ++-- tests/cts_gl45.py | 2 +- tests/cts_gles.py | 9 - tests/gtf_gles.py | 6 +++--- tests/khr_gl.py| 22 +++--- tests/khr_gles.py | 11 +-- tests/khr_noctx.py | 2 +- 8 files changed, 39 insertions(+), 39 deletions(-) diff --git a/framework/test/deqp.py b/framework/test/deqp.py index 5db2a922f..7c1a82c2a 100644 --- a/framework/test/deqp.py +++ b/framework/test/deqp.py @@ -114,7 +114,7 @@ def gen_caselist_txt(bin_, caselist, extra_args): # differ then we cannot pre-generate the caselist on the build host: # we must *dynamically* generate it during the testrun. basedir = os.path.dirname(bin_) -caselist_path = os.path.join(basedir, caselist) +caselist_path = os.path.join(basedir, caselist + '-cases.txt') # TODO: need to catch some exceptions here... with open(os.devnull, 'w') as d: @@ -123,8 +123,10 @@ def gen_caselist_txt(bin_, caselist, extra_args): env['MESA_GLES_VERSION_OVERRIDE'] = '3.2' subprocess.check_call( -[bin_, '--deqp-runmode=txt-caselist'] + extra_args, cwd=basedir, +[bin_, '--deqp-runmode=txt-caselist' , '--deqp-case=' , caselist, '.*'] ++ extra_args, cwd=basedir, stdout=d, stderr=d, env=env) + assert os.path.exists(caselist_path) return caselist_path diff --git a/tests/cts_gl.py b/tests/cts_gl.py index 935df7a7c..fa36ea4f9 100644 --- a/tests/cts_gl.py +++ b/tests/cts_gl.py @@ -70,24 +70,24 @@ class DEQPCTSTest(deqp.DEQPBaseTest): profile = deqp.make_profile( # pylint: disable=invalid-name itertools.chain( deqp.iter_deqp_test_cases( -deqp.gen_caselist_txt(_CTS_BIN, 'GL30-CTS-cases.txt', _EXTRA_ARGS)), +deqp.gen_caselist_txt(_CTS_BIN, 'GL30-CTS', _EXTRA_ARGS)), deqp.iter_deqp_test_cases( -deqp.gen_caselist_txt(_CTS_BIN, 'GL31-CTS-cases.txt', _EXTRA_ARGS)), +deqp.gen_caselist_txt(_CTS_BIN, 'GL31-CTS', _EXTRA_ARGS)), deqp.iter_deqp_test_cases( -deqp.gen_caselist_txt(_CTS_BIN, 'GL32-CTS-cases.txt', _EXTRA_ARGS)), +deqp.gen_caselist_txt(_CTS_BIN, 'GL32-CTS', _EXTRA_ARGS)), deqp.iter_deqp_test_cases( -deqp.gen_caselist_txt(_CTS_BIN, 'GL33-CTS-cases.txt', _EXTRA_ARGS)), +deqp.gen_caselist_txt(_CTS_BIN, 'GL33-CTS', _EXTRA_ARGS)), deqp.iter_deqp_test_cases( -deqp.gen_caselist_txt(_CTS_BIN, 'GL40-CTS-cases.txt', _EXTRA_ARGS)), +deqp.gen_caselist_txt(_CTS_BIN, 'GL40-CTS', _EXTRA_ARGS)), deqp.iter_deqp_test_cases( -deqp.gen_caselist_txt(_CTS_BIN, 'GL41-CTS-cases.txt', _EXTRA_ARGS)), +deqp.gen_caselist_txt(_CTS_BIN, 'GL41-CTS', _EXTRA_ARGS)), deqp.iter_deqp_test_cases( -deqp.gen_caselist_txt(_CTS_BIN, 'GL42-CTS-cases.txt', _EXTRA_ARGS)), +deqp.gen_caselist_txt(_CTS_BIN, 'GL42-CTS', _EXTRA_ARGS)), deqp.iter_deqp_test_cases( -deqp.gen_caselist_txt(_CTS_BIN, 'GL43-CTS-cases.txt', _EXTRA_ARGS)), +deqp.gen_caselist_txt(_CTS_BIN, 'GL43-CTS', _EXTRA_ARGS)), deqp.iter_deqp_test_cases( -deqp.gen_caselist_txt(_CTS_BIN, 'GL44-CTS-cases.txt', _EXTRA_ARGS)), +deqp.gen_caselist_txt(_CTS_BIN, 'GL44-CTS', _EXTRA_ARGS)), deqp.iter_deqp_test_cases( -deqp.gen_caselist_txt(_CTS_BIN, 'GL45-CTS-cases.txt', _EXTRA_ARGS)), +deqp.gen_caselist_txt(_CTS_BIN, 'GL45-CTS', _EXTRA_ARGS)), ), DEQPCTSTest) diff --git a/tests/cts_gl45.py b/tests/cts_gl45.py index d033c3efc..070ed8026 100644 --- a/tests/cts_gl45.py +++ b/tests/cts_gl45.py @@ -64,6 +64,6 @@ class DEQPCTSTest(deqp.DEQPBaseTest): profile = deqp.make_profile( # pylint: disable=invalid-name itertools.chain( deqp.iter_deqp_test_cases( -deqp.gen_caselist_txt(_CTS_BIN, 'GL45-CTS-cases.txt', _EXTRA_ARGS)), +deqp.gen_caselist_txt(_CTS_BIN, 'GL45-CTS', _EXTRA_ARGS)), ), DEQPCTSTest) diff --git a/tests/cts_gles.py b/tests/cts_gles.p
[Piglit] [MR] ARB_gl_spirv full series
https://gitlab.freedesktop.org/mesa/piglit/merge_requests/1 MR equivalent to this: https://lists.freedesktop.org/archives/piglit/2019-February/025762.html But rebased against master, and with some tweaks on the patches. At the point of this MR being created, the detailed overview on that email is true. ___ Piglit mailing list Piglit@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [PATCH 00/63] ARB_gl_spirv full series
One of the bit advantages of MR is that you can keep updating big series
(ie: rebases against master) without spamning the list with tons of
patches. So now that piglit MR got enabled, I have just created one for
it. Consider this big series abandoned, and if anyone want to help with
the review (please!), go to the MR. I will send the MR email
announcement soon.
On 24/2/19 0:44, Alejandro Piñeiro wrote:
Hi,
so finally we finished the cleaning up of the piglit series for the
ARB_gl_spirv support that we send to Mesa some weeks ago (see [1]).
As with Mesa, we preferred to send all the patches we thought that
were finished, instead of keeping sending subseries with specific
sub-features.
Again, we want to thanks Nicolai Hähnle, that was the one that started
this work.
A high-level, TL;DR, overview of this series is the following one:
* Patches 01-47: more barebone tests for ARB_gl_spirv, including
more support on shader_runner if needed.
* Patches 48-58: support for generation of ARB_gl_spirv tests. This
includes direct generation of SPIR-V tests through templates, and
support to transform existing GLSL tests from other specs, through
glslang (using a wrapper).
* Patches 59-63: minor clean-ups and fixes.
And now going more into detail:
* Patches 01-02: support for GL_ACTIVE_UNIFORMS queries, plus
modifying existing ARB_gl_spirv tests to include this query.
* Patches 03-12: general (so both GLSL and SPIR-V) support for
running UBO/SSBO tests without using any name, plus UBO/SSBO
tests.
* Patches 13-22: added further support for several shader
introspection queries, like program interface queries, plus tests
related with them. Here we have a lot of SPIRV ONLY tests (so
tests that shouldn't be run on GLSL), because those are tests
stripped of names, so several NAME related queries should return
specific values defined by ARB_gl_spirv.
* Patches 23-27: added transform feedback support, transform
feedback related query object support, and the transform feedback
tests. One could wonder why it was added, as it was avoided for a
long time. On our experience, adding and editing tests was far
easier as soon as we added support for xfb on shader_runner, even
if it was really basic, and allowed to test several scenarios,
that would be more complex by adding C-programs like on existing
transform feedback tests.
* Patches 28-47: miscellaneous barebone tests, and some basic extra
support on shader runner. A little of everything.
* Patches 48-50: added support to load SPIR-V from a external
file. This would allow to have a GLSL test, and its SPIR-V
equivalent on a different file. This was useful on the developing
stages, in the case we wanted to reuse a existing GLSL test
without modifying it too much.
* Patches 51-53: added script that tries to generate a SPIR-V test,
or tests, from a existing GLSL shader runner test, or set of
tests. This script serves as a wrapper over glslang, providing
extra features, like automatic skipping, mark existing tests,
parse SPIR-V shader for locations, etc. The script can include the
SPIR-V on the same test or generate a different file, on the same
directory or on a mirror one (more about that below).
* Patches 54-55: integration of previous script with CMake. By
default OFF, as it adds a significant extra time while
building. On this integration, the SPIR-V tests are generated to a
new ("mirror") directory, as any other generated tests. As
mentioned, the script by default generates the SPIR-V on the same
directory, but that is only useful while developing. As a new
source of generated tests, it makes more sense on a new
directory. It is worth to note that it only converts tests from
the 'tests' directory, but we tested also converting tests from
'generated_tests'. But the latter can require even more time, ~45
min on an average machine, so we initially assume that that would
not be accepted even as an optional flag. We could be wrong
though.
* Patches 56-58: add scripts that directly generates SPIR-V
shader_runner, using templates, as some other existing GLSL
generators.
* Patches 59-64: as mentioned, minor clean-ups and fixes.
[1] https://gitlab.freedesktop.org/mesa/mesa/merge_requests/178
Alejandro Piñeiro (26):
shader_runner: add support for glGetProgram queries
arb_gl_spirv: add GL_ACTIVE_UNIFORMS checks
shader_runner: add force_no_names mode
arb_gl_spirv: add some simple ubo/ssbo tests
arb_gl_spirv: add ubo/ssbo tests with matrices
arb_gl_spirv: add a array of ubo test, with complex ubo content
arb_gl_spirv: add ubo tests with different matrix/array strides
arb_gl_spirv: add ssbo test using std140 and std430
arb_gl_spirv: add ubo array test with copy between arrays
arb_gl
Re: [Piglit] [PATCH] editorconfig: Add max_line_length property
On 22/2/19 21:03, Ian Romanick wrote:
On 2/22/19 8:25 AM, apinheiro wrote:
On 22/2/19 15:51, Andres Gomez wrote:
The property is supported by most of the editors, but not all:
https://github.com/editorconfig/editorconfig/wiki/EditorConfig-Properties#max_line_length
Cc: Eric Engestrom
Cc: Eric Anholt
Signed-off-by: Andres Gomez
It is really realistic to set 79 as max_line_length for piglit? Although
on mesa that limit is usually well respected, I found several source
files on piglit that are really loose on that limit, and Im not sure it
was considered as an error/problem.
A lot of patches land in piglit unreviewed. ;) I prefer lines < 80, but
there's always room for exceptions.
Fair enough, then I guess that I was assuming a looser rule that didn't
exist. Let's set then as 79, and let anyone else thinking that it's too
small complaining on the practice:
Reviewed-by: Alejandro Piñeiro
---
.editorconfig | 4
1 file changed, 4 insertions(+)
diff --git a/.editorconfig b/.editorconfig
index c614fcca7..e0f13a949 100644
--- a/.editorconfig
+++ b/.editorconfig
@@ -4,15 +4,19 @@ root = true
indent_style = space
indent_size = 4
trim_trailing_whitespace = true
+max_line_length = 79
[*.{c,cpp,h,hpp}]
indent_style = tab
tab_width = 8
+max_line_length = 78
[*.{cmake,txt}]
indent_style = tab
tab_width = 8
+max_line_length = 78
[{README,HACKING}]
indent_style = tab
tab_width = 8
+max_line_length = 78
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [PATCH] editorconfig: Add max_line_length property
On 22/2/19 15:51, Andres Gomez wrote:
The property is supported by most of the editors, but not all:
https://github.com/editorconfig/editorconfig/wiki/EditorConfig-Properties#max_line_length
Cc: Eric Engestrom
Cc: Eric Anholt
Signed-off-by: Andres Gomez
It is really realistic to set 79 as max_line_length for piglit? Although
on mesa that limit is usually well respected, I found several source
files on piglit that are really loose on that limit, and Im not sure it
was considered as an error/problem.
---
.editorconfig | 4
1 file changed, 4 insertions(+)
diff --git a/.editorconfig b/.editorconfig
index c614fcca7..e0f13a949 100644
--- a/.editorconfig
+++ b/.editorconfig
@@ -4,15 +4,19 @@ root = true
indent_style = space
indent_size = 4
trim_trailing_whitespace = true
+max_line_length = 79
[*.{c,cpp,h,hpp}]
indent_style = tab
tab_width = 8
+max_line_length = 78
[*.{cmake,txt}]
indent_style = tab
tab_width = 8
+max_line_length = 78
[{README,HACKING}]
indent_style = tab
tab_width = 8
+max_line_length = 78
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [PATCH] arb_shading_language_420pack: test invalid function return type
Perhaps a spec quote? With or without it:
Reviewed-by: Alejandro Piñeiro
On 21/2/19 12:50, Tapani Pälli wrote:
Extension adds implicit conversion for return types. This test checks
that driver does not incorrectly allow invalid return type.
Signed-off-by: Tapani Pälli
---
.../implicit-conversion-invalid-type.frag | 21 +++
1 file changed, 21 insertions(+)
create mode 100644
tests/spec/arb_shading_language_420pack/compiler/implicit-conversion-invalid-type.frag
diff --git
a/tests/spec/arb_shading_language_420pack/compiler/implicit-conversion-invalid-type.frag
b/tests/spec/arb_shading_language_420pack/compiler/implicit-conversion-invalid-type.frag
new file mode 100644
index 0..9fc24d9ee
--- /dev/null
+++
b/tests/spec/arb_shading_language_420pack/compiler/implicit-conversion-invalid-type.frag
@@ -0,0 +1,21 @@
+/* [config]
+ * expect_result: fail
+ * glsl_version: 1.30
+ * require_extensions: GL_ARB_shading_language_420pack
+ * [end config]
+ */
+#version 130
+#extension GL_ARB_shading_language_420pack: enable
+
+out vec4 color;
+
+int test()
+{
+ /* Return invalid type, this should not succeed. */
+ return ivec2(0);
+}
+
+void main()
+{
+color = vec4(test());
+}
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [PATCH] vulkan: Add some tests for block member decorations
On 24/1/19 16:28, Lionel Landwerlin wrote:
Thanks a lot for explaining.
I was wondering whether it would be worth checking something like this :
(layout location = 1) out block myBlock {
vec4 a;
vec4 b;
}
And verify whether a would bet location 1 and b location 2.
So do you mean adding a new test, or modify the current one to base on 0
instead of base on 1?
But this current version looks good to me :
Reviewed-by: Lionel Landwerlin
Thanks!
Side question that I cannot find the answer to in the spec, the
following would be illegal right? :
layout (location= 1) out block myOtherBlock {
layout (location = 1) vec4 a;
layout (location = 0) vec4 b;
layout (location = 3) vec4 c;
layout (location = 2) vec4 d;
};
Well, I don't think so. This is the spec quote that we included on the
first patch of the mesa merge request:
“If the structure type is a Block but without a Location, then each
of its members must have a Location decoration. If it is a Block
with a Location decoration, then its members are assigned
consecutive locations in declaration order, starting from the
first member which is initially the Block. Any member with its own
Location decoration is assigned that location. Each remaining
member is assigned the location after the immediately preceding
member in declaration order.”
What I understand from here, is that the location used for the block
would be the one used as "base_location" when assigning locations for
members that lacks it, specially at the beginning of the block. But if
the member has a location, then we use it, and we also use it for the
following members. So for example, in this case:
layout (location = 5) out myOtherBlock {
vec4 a;
layout (location = 0) vec4 b;
layout (location = 3) vec4 c;
layout (location = 2) vec4 d;
};
'a' gets assigned 5, and the rest would get assigned 0, 3 and 2
respectively. And I have just tested with glslang, and it is doing just
that.
Thanks again.
-Lionel
On 24/01/2019 13:17, apinheiro wrote:
On 24/1/19 12:28, Lionel Landwerlin wrote:
I'm not sure whether my understanding of
block-layout-location.vk_shader_test is correct.
Is the expectation that the location of %name (0) is added to the
location of its field (a, b, c, d)?
Not sure if added is the proper word, but derived. So for those
members, SPIR-V doesn't include the location. The block has the
location, so the members get a location based on it. So that specific
test comes from a block like this:
(layout location= 0) out block myBlock {
vec4 a;
vec4 b:
vec4 c;
vec4 d;
};
That glslang translates to SPIR-V with setting location 0 to
myBlock, but not setting a location for the members. Such members
would get locations 0, 1, 2, 3 respectively. Those locations are
assigned right now by the driver, at least on the anv case that I
tested. Note that as mentioned on the original email, this is the
case that is working right now. I only tested it with anv, but those
location assignment is done on the spirv to nir pass, so that would
affect other Vulkan drivers using it.
FWIW, what we are trying to fix with the MR I sent to review [1], and
tested with the tests on this patch, is basically the case where
there is a explicit location for any block member, that doesn't
follow the "default location assignment rule" based on the block base
location. So for example, this:
layout (location= 0) out block myOtherBlock {
layout (location = 1) vec4 a;
layout (location = 0) vec4 b;
layout (location = 3) vec4 c;
layout (location = 2) vec4 d;
};
[1] https://gitlab.freedesktop.org/mesa/mesa/merge_requests/142
Thanks,
-Lionel
On 23/01/2019 15:07, Alejandro Piñeiro wrote:
From: Neil Roberts
v2: imported to piglit from a example vkrunner examples branch, also
updated description on the top comment (Alejandro Piñeiro)
---
This tests are intended to test the patches at the following mesa MR:
https://gitlab.freedesktop.org/mesa/mesa/merge_requests/142
Although FWIW, block-layout-location.vk_shader_test is passing right
now with just master. The other two tests require the first patch
included on that MR.
.../block-layout-location.vk_shader_test | 121
+
...lock-member-layout-location.vk_shader_test | 69 ++
...block-mixed-layout-location.vk_shader_test | 126
++
3 files changed, 316 insertions(+)
create mode 100644
tests/vulkan/shaders/block-layout-location.vk_shader_test
create mode 100644
tests/vulkan/shaders/block-member-layout-location.vk_shader_test
create mode 100644
tests/vulkan/shaders/block-mixed-layout-location.vk_shader_test
diff --git
a/tests/vulkan/shaders/block-layout-location.vk_shader_test
b/tests/vulkan/shaders/block-layout-location.vk_shader_test
new file mode 100644
index 0..
Re: [Piglit] [PATCH] vulkan: Add some tests for block member decorations
On 24/1/19 12:28, Lionel Landwerlin wrote:
I'm not sure whether my understanding of
block-layout-location.vk_shader_test is correct.
Is the expectation that the location of %name (0) is added to the
location of its field (a, b, c, d)?
Not sure if added is the proper word, but derived. So for those members,
SPIR-V doesn't include the location. The block has the location, so the
members get a location based on it. So that specific test comes from a
block like this:
(layout location= 0) out block myBlock {
vec4 a;
vec4 b:
vec4 c;
vec4 d;
};
That glslang translates to SPIR-V with setting location 0 to myBlock,
but not setting a location for the members. Such members would get
locations 0, 1, 2, 3 respectively. Those locations are assigned right
now by the driver, at least on the anv case that I tested. Note that as
mentioned on the original email, this is the case that is working right
now. I only tested it with anv, but those location assignment is done on
the spirv to nir pass, so that would affect other Vulkan drivers using it.
FWIW, what we are trying to fix with the MR I sent to review [1], and
tested with the tests on this patch, is basically the case where there
is a explicit location for any block member, that doesn't follow the
"default location assignment rule" based on the block base location. So
for example, this:
layout (location= 0) out block myOtherBlock {
layout (location = 1) vec4 a;
layout (location = 0) vec4 b;
layout (location = 3) vec4 c;
layout (location = 2) vec4 d;
};
[1] https://gitlab.freedesktop.org/mesa/mesa/merge_requests/142
Thanks,
-Lionel
On 23/01/2019 15:07, Alejandro Piñeiro wrote:
From: Neil Roberts
v2: imported to piglit from a example vkrunner examples branch, also
updated description on the top comment (Alejandro Piñeiro)
---
This tests are intended to test the patches at the following mesa MR:
https://gitlab.freedesktop.org/mesa/mesa/merge_requests/142
Although FWIW, block-layout-location.vk_shader_test is passing right
now with just master. The other two tests require the first patch
included on that MR.
.../block-layout-location.vk_shader_test | 121 +
...lock-member-layout-location.vk_shader_test | 69 ++
...block-mixed-layout-location.vk_shader_test | 126 ++
3 files changed, 316 insertions(+)
create mode 100644
tests/vulkan/shaders/block-layout-location.vk_shader_test
create mode 100644
tests/vulkan/shaders/block-member-layout-location.vk_shader_test
create mode 100644
tests/vulkan/shaders/block-mixed-layout-location.vk_shader_test
diff --git
a/tests/vulkan/shaders/block-layout-location.vk_shader_test
b/tests/vulkan/shaders/block-layout-location.vk_shader_test
new file mode 100644
index 0..3eb2c0402
--- /dev/null
+++ b/tests/vulkan/shaders/block-layout-location.vk_shader_test
@@ -0,0 +1,121 @@
+# Test that interface block members are correctly matched by explicit
+# location, when only the main variable has a location, so the
+# location of the members should be derived from this.
+#
+# Note that we include the spirv assembly. This is because although we
+# used a GLSL shader as reference, we tweaked the SPIR-V generated
+
+[vertex shader spirv]
+ OpCapability Shader
+ %1 = OpExtInstImport "GLSL.std.450"
+ OpMemoryModel Logical GLSL450
+ OpEntryPoint Vertex %main "main" %name %_ %piglit_vertex
+ OpSource GLSL 440
+ OpName %main "main"
+ OpName %block "block"
+ OpMemberName %block 0 "a"
+ OpMemberName %block 1 "b"
+ OpMemberName %block 2 "c"
+ OpMemberName %block 3 "d"
+ OpName %name "name"
+ OpName %gl_PerVertex "gl_PerVertex"
+ OpMemberName %gl_PerVertex 0 "gl_Position"
+ OpMemberName %gl_PerVertex 1 "gl_PointSize"
+ OpMemberName %gl_PerVertex 2 "gl_ClipDistance"
+ OpName %_ ""
+ OpName %piglit_vertex "piglit_vertex"
+ OpDecorate %block Block
+; Only the main name variable has a location. The locations of the
members
+; should be derived from this.
+ OpDecorate %name Location 0
+ OpMemberDecorate %gl_PerVertex 0 BuiltIn Position
+ OpMemberDecorate %gl_PerVertex 1 BuiltIn PointSize
+ OpMemberDecorate %gl_PerVertex 2 BuiltIn ClipDistance
+ OpDecorate %gl_PerVertex Block
+ OpDecorate %piglit_vertex Location 0
+ %void = OpTypeVoid
+ %3 = OpTypeFunction %void
+ %float = OpTypeFloat 32
+ %v4float = OpTypeVector %float 4
+ %block = OpTypeStruct %v4float %v4float %v4float %v4float
+%_ptr_Output_block = OpTypePointer Output %block
+ %name = OpVariable %_ptr_Output_block Output
+ %int = OpTypeInt 32 1
+ %int_0 = OpConstant %int 0
+ %float_1 =
Re: [Piglit] [PATCH] fbo-3d: test both POT and NPOT depths
On 2/1/19 16:09, Ilia Mirkin wrote:
> On Wed, Jan 2, 2019 at 5:21 AM apinheiro wrote:
>> I have a nitpick comment below. You can ignore it in any case:
>>
>> Reviewed-by: Alejandro Piñeiro
>>
>> On 2/1/19 1:02, Ilia Mirkin wrote:
>>> This demonstrates issues on nv4x, which will use a different layout for
>>> POT vs NPOT sizes.
>>>
>>> Signed-off-by: Ilia Mirkin
>>> ---
>>> tests/fbo/fbo-3d.c | 31 ++-
>>> 1 file changed, 22 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/tests/fbo/fbo-3d.c b/tests/fbo/fbo-3d.c
>>> index e622c1df8..36dbed4e0 100644
>>> --- a/tests/fbo/fbo-3d.c
>>> +++ b/tests/fbo/fbo-3d.c
>>> @@ -58,16 +58,12 @@ float depth_color[NUM_DEPTHS][4] = {
>>> {0.0, 1.0, 1.0, 0.0},
>>> };
>>>
>>> -int pot_depth;
>>> -
>>> static int
>>> -create_3d_fbo(void)
>>> +create_3d_fbo(int pot_depth)
>>> {
>>> GLuint tex, fb;
>>> GLenum status;
>>> int depth;
>>> - pot_depth =
>>> piglit_is_extension_supported("GL_ARB_texture_non_power_of_two") ?
>>> - NUM_DEPTHS: POT_DEPTHS;
>>>
>>> glGenTextures(1, &tex);
>>> glBindTexture(GL_TEXTURE_3D, tex);
>>> @@ -109,7 +105,6 @@ create_3d_fbo(void)
>>> piglit_draw_rect(-2, -2, BUF_WIDTH + 2, BUF_HEIGHT + 2);
>>> }
>>>
>>> -
>> Is this new line removal really needed?
> Not _really_, but the two blank lines were jarring to me. Figured I'd
> fix it up while I was at it. Do you feel like 2 newlines is the
> appropriate quantity in this situation?
Ah ok, I didn't realize there was 2 newlines. Yes, I think that it is ok
to remove one.
>
>>
>>> done:
>>> glBindFramebufferEXT(GL_FRAMEBUFFER_EXT, piglit_winsys_fbo);
>>> glDeleteFramebuffersEXT(1, &fb);
>>> @@ -121,7 +116,7 @@ done:
pEpkey.asc
Description: application/pgp-keys
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [PATCH] fbo-3d: test both POT and NPOT depths
I have a nitpick comment below. You can ignore it in any case:
Reviewed-by: Alejandro Piñeiro
On 2/1/19 1:02, Ilia Mirkin wrote:
> This demonstrates issues on nv4x, which will use a different layout for
> POT vs NPOT sizes.
>
> Signed-off-by: Ilia Mirkin
> ---
> tests/fbo/fbo-3d.c | 31 ++-
> 1 file changed, 22 insertions(+), 9 deletions(-)
>
> diff --git a/tests/fbo/fbo-3d.c b/tests/fbo/fbo-3d.c
> index e622c1df8..36dbed4e0 100644
> --- a/tests/fbo/fbo-3d.c
> +++ b/tests/fbo/fbo-3d.c
> @@ -58,16 +58,12 @@ float depth_color[NUM_DEPTHS][4] = {
> {0.0, 1.0, 1.0, 0.0},
> };
>
> -int pot_depth;
> -
> static int
> -create_3d_fbo(void)
> +create_3d_fbo(int pot_depth)
> {
> GLuint tex, fb;
> GLenum status;
> int depth;
> - pot_depth =
> piglit_is_extension_supported("GL_ARB_texture_non_power_of_two") ?
> - NUM_DEPTHS: POT_DEPTHS;
>
> glGenTextures(1, &tex);
> glBindTexture(GL_TEXTURE_3D, tex);
> @@ -109,7 +105,6 @@ create_3d_fbo(void)
> piglit_draw_rect(-2, -2, BUF_WIDTH + 2, BUF_HEIGHT + 2);
> }
>
> -
Is this new line removal really needed?
> done:
> glBindFramebufferEXT(GL_FRAMEBUFFER_EXT, piglit_winsys_fbo);
> glDeleteFramebuffersEXT(1, &fb);
> @@ -121,7 +116,7 @@ done:
> * 3D texture.
> */
> static void
> -draw_depth(int x, int y, int depth)
> +draw_depth(int x, int y, int depth, int pot_depth)
> {
> float depth_coord = (float)depth / (pot_depth - 1);
>
> @@ -171,12 +166,12 @@ piglit_display(void)
> glClearColor(1.0, 1.0, 1.0, 1.0);
> glClear(GL_COLOR_BUFFER_BIT);
>
> - tex = create_3d_fbo();
> + tex = create_3d_fbo(POT_DEPTHS);
>
> for (depth = 0; depth < NUM_DEPTHS; depth++) {
> int x = 1 + depth * (BUF_WIDTH + 1);
> int y = 1;
> - draw_depth(x, y, depth);
> + draw_depth(x, y, depth, POT_DEPTHS);
> }
>
> for (depth = 0; depth < NUM_DEPTHS; depth++) {
> @@ -187,6 +182,24 @@ piglit_display(void)
>
> glDeleteTextures(1, &tex);
>
> + if (piglit_is_extension_supported("GL_ARB_texture_non_power_of_two")) {
> + tex = create_3d_fbo(NUM_DEPTHS);
> +
> + for (depth = 0; depth < NUM_DEPTHS; depth++) {
> + int x = 1 + depth * (BUF_WIDTH + 1);
> + int y = 2 + BUF_HEIGHT;
> + draw_depth(x, y, depth, NUM_DEPTHS);
> + }
> +
> + for (depth = 0; depth < NUM_DEPTHS; depth++) {
> + int x = 1 + depth * (BUF_WIDTH + 1);
> + int y = 2 + BUF_HEIGHT;
> + pass &= test_depth_drawing(x, y, depth_color[depth]);
> + }
> +
> + glDeleteTextures(1, &tex);
> + }
> +
> piglit_present_results();
>
> return pass ? PIGLIT_PASS : PIGLIT_FAIL;
pEpkey.asc
Description: application/pgp-keys
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [PATCH] arb_enhanced_layouts: explicit-offset: add more corner cases
LGTM
Reviewed-by: Alejandro Piñeiro
On 16/12/18 6:15, Niklas Haas wrote:
> From: Niklas Haas
>
> This adds a few tests:
> - testing offsets that immediately follow a member whose actual size is
> smaller than its actual alignment
> - testing confusing interactions between explicit alignment and explicit
> offsets, in particular when the former overrides the latter
> - test that overriding block-level alignments works as expected
>
> Notably, the first of the three test cases triggers a compile-time error
> in current mesa.
>
> Signed-off-by: Niklas Haas
> ---
> ...-explicit-offset-align-mismatch-error.vert | 39 +++
> .../ssbo-explicit-offset-align-mismatch.vert | 32 +++
> .../ssbo-explicit-offset-vec3.vert| 29 ++
> ...sbo-override-explicit-block-alignment.vert | 31 +++
> ...-explicit-offset-align-mismatch-error.vert | 38 ++
> .../ubo-explicit-offset-align-mismatch.vert | 31 +++
> .../ubo-explicit-offset-vec3.vert | 28 +
> ...ubo-override-explicit-block-alignment.vert | 30 ++
> 8 files changed, 258 insertions(+)
> create mode 100644
> tests/spec/arb_enhanced_layouts/compiler/explicit-offsets/ssbo-explicit-offset-align-mismatch-error.vert
> create mode 100644
> tests/spec/arb_enhanced_layouts/compiler/explicit-offsets/ssbo-explicit-offset-align-mismatch.vert
> create mode 100644
> tests/spec/arb_enhanced_layouts/compiler/explicit-offsets/ssbo-explicit-offset-vec3.vert
> create mode 100644
> tests/spec/arb_enhanced_layouts/compiler/explicit-offsets/ssbo-override-explicit-block-alignment.vert
> create mode 100644
> tests/spec/arb_enhanced_layouts/compiler/explicit-offsets/ubo-explicit-offset-align-mismatch-error.vert
> create mode 100644
> tests/spec/arb_enhanced_layouts/compiler/explicit-offsets/ubo-explicit-offset-align-mismatch.vert
> create mode 100644
> tests/spec/arb_enhanced_layouts/compiler/explicit-offsets/ubo-explicit-offset-vec3.vert
> create mode 100644
> tests/spec/arb_enhanced_layouts/compiler/explicit-offsets/ubo-override-explicit-block-alignment.vert
>
> diff --git
> a/tests/spec/arb_enhanced_layouts/compiler/explicit-offsets/ssbo-explicit-offset-align-mismatch-error.vert
>
> b/tests/spec/arb_enhanced_layouts/compiler/explicit-offsets/ssbo-explicit-offset-align-mismatch-error.vert
> new file mode 100644
> index 0..00d458b28
> --- /dev/null
> +++
> b/tests/spec/arb_enhanced_layouts/compiler/explicit-offsets/ssbo-explicit-offset-align-mismatch-error.vert
> @@ -0,0 +1,39 @@
> +// [config]
> +// expect_result: fail
> +// glsl_version: 1.40
> +// require_extensions: GL_ARB_enhanced_layouts
> GL_ARB_shader_storage_buffer_object
> +// check_link: false
> +// [end config]
> +//
> +// ARB_enhanced_layouts spec says:
> +//"The /actual alignment/ of a member will be the greater of the
> specified
> +//*align* alignment and the standard (e.g., *std140*) base alignment for
> the
> +//member's type. The /actual offset/ of a member is computed as follows:
> +//If *offset* was declared, start with that offset, otherwise start with
> the
> +//next available offset. If the resulting offset is not a multiple of
> the
> +///actual alignment/, increase it to the first offset that is a multiple
> of
> +//the /actual alignment/. This results in the /actual offset/ the member
> +//will have."
> +//
> +//"It is a compile-time error to
> +//specify an *offset* that is smaller than the offset of the previous
> +//member in the block or that lies within the previous member of the
> +//block."
> +//
> +// Tests whether a block with conflicting offset and alignment requirements
> +// followed by a field with an explicit offset that lies within the actual
> +// position of the previous member fails.
> +//
> +
> +#version 140
> +#extension GL_ARB_enhanced_layouts : enable
> +#extension GL_ARB_shader_storage_buffer_object : enable
> +
> +layout(std430) buffer b {
> + layout(offset = 8, align = 16) vec2 var1; // starts at actual offset
> 16
> + layout(offset = 20) float var2; // error: inside var1
> +};
> +
> +void main()
> +{
> +}
> diff --git
> a/tests/spec/arb_enhanced_layouts/compiler/explicit-offsets/ssbo-explicit-offset-align-mismatch.vert
>
> b/tests/spec/arb_enhanced_layouts/compiler/explicit-offsets/ssbo-explicit-offset-align-mismatch.vert
> new file mode 100644
> index 0..79e83ed1c
> --- /dev/null
> +++
> b/tests/spec/arb_enhanced_layouts/compiler/explicit-offsets/ssbo-explicit-offset-align-mismatch.vert
> @@ -0,0 +1,32 @@
> +// [config]
> +// expect_result: pass
> +// glsl_version: 1.40
> +// require_extensions: GL_ARB_enhanced_layouts
> GL_ARB_shader_storage_buffer_object
> +// check_link: false
> +// [end config]
> +//
> +// ARB_enhanced_layouts spec says:
> +//"The /actual alignment/ of a member will be the greater of the
> specified
> +//*align* ali
Re: [Piglit] [PATCH] arb_gl_spirv: add test for uniform blocks with the same structure
On 23/11/18 19:41, Józef Kucia wrote: > On Thu, Nov 22, 2018 at 11:21 AM apinheiro wrote: >> some weeks ago I sent a series with ubo/ssbo tests (still pending review) >> >> https://lists.freedesktop.org/archives/piglit/2018-September/025116.html >> >> All those has the name stripped. Could you try them and see if any of >> them triggers the NVIDIA bug you found? > arb_gl_spirv @ execution @ ubo @ matrix @ column-vs-row.shader_test > triggers the NVIDIA bug. Then do you think that your test is still needed? An alternative, as we didn't add any compute shader using ubo/ssbo, would be rename and update the description of your test (something like "compute shader using ubo"). Also if you are interested to get those tests integrated, you can just take a look an review them, wink wink > > BTW, where can I find the "shader_test_spirv.py" script? You can find it on our WIP (so several wip and fixme patches) ARB_gl_spirv piglit branch: https://github.com/Igalia/piglit/tree/igalia/arb_gl_spirv On the directory generated_spv We didn't send it to review yet because it needs a lot of cleaning and squashing. We will do that eventually, but we are focus first on getting ARB_gl_spirv finished. > > Thanks, > Józef > pEpkey.asc Description: application/pgp-keys ___ Piglit mailing list Piglit@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [PATCH] arb_gl_spirv: add test for uniform blocks with the same structure
Hi, some weeks ago I sent a series with ubo/ssbo tests (still pending review) https://lists.freedesktop.org/archives/piglit/2018-September/025116.html All those has the name stripped. Could you try them and see if any of them triggers the NVIDIA bug you found? BR On 21/11/18 16:23, Józef Kucia wrote: > This test reproduces a bug in Nvidia drivers: > > error: binding mismatch between shaders for UBO (named __defaultname) > error: struct type mismatch between shaders for uniform (named __defaultname) > error: binding mismatch between shaders for UBO (named __defaultname) > error: struct type mismatch between shaders for uniform (named __defaultname) > error: binding mismatch between shaders for UBO (named __defaultname) > error: struct type mismatch between shaders for uniform (named __defaultname) > error: binding mismatch between shaders for UBO (named __defaultname) > error: struct type mismatch between shaders for uniform (named __defaultname) > > The same issue is also present when SPIR-V debug names for uniform > blocks are the same. > --- > .../unnamed-uniform-blocks.shader_test| 67 +++ > 1 file changed, 67 insertions(+) > create mode 100644 > tests/spec/arb_gl_spirv/linker/uniform/unnamed-uniform-blocks.shader_test > > diff --git > a/tests/spec/arb_gl_spirv/linker/uniform/unnamed-uniform-blocks.shader_test > b/tests/spec/arb_gl_spirv/linker/uniform/unnamed-uniform-blocks.shader_test > new file mode 100644 > index ..9dba7d44c37d > --- /dev/null > +++ > b/tests/spec/arb_gl_spirv/linker/uniform/unnamed-uniform-blocks.shader_test > @@ -0,0 +1,67 @@ > +# Test unnamed uniform blocks with the same structure > + > +[require] > +SPIRV YES > +GL >= 3.3 > +GLSL >= 4.50 > + > +[compute shader spirv] > +; SPIR-V > +; Version: 1.0 > +; Generator: Khronos Glslang Reference Front End; 7 > +; Bound: 33 > +; Schema: 0 > + OpCapability Shader > + %1 = OpExtInstImport "GLSL.std.450" > + OpMemoryModel Logical GLSL450 > + OpEntryPoint GLCompute %4 "main" > + OpExecutionMode %4 LocalSize 1 1 1 > + OpDecorate %9 Location 2 > + OpDecorate %9 DescriptorSet 0 > + OpDecorate %_arr_v4uint_uint_4 ArrayStride 16 > + OpMemberDecorate %_struct_18 0 Offset 0 > + OpDecorate %_struct_18 Block > + OpDecorate %20 DescriptorSet 0 > + OpDecorate %20 Binding 0 > + OpDecorate %_arr_v4uint_uint_4_0 ArrayStride 16 > + OpMemberDecorate %_struct_26 0 Offset 0 > + OpDecorate %_struct_26 Block > + OpDecorate %28 DescriptorSet 0 > + OpDecorate %28 Binding 1 > + %void = OpTypeVoid > + %3 = OpTypeFunction %void > + %uint = OpTypeInt 32 0 > + %7 = OpTypeImage %uint 2D 0 0 0 2 Rgba32ui > +%_ptr_UniformConstant_7 = OpTypePointer UniformConstant %7 > + %9 = OpVariable %_ptr_UniformConstant_7 UniformConstant > +%int = OpTypeInt 32 1 > + %v2int = OpTypeVector %int 2 > + %int_0 = OpConstant %int 0 > + %14 = OpConstantComposite %v2int %int_0 %int_0 > + %v4uint = OpTypeVector %uint 4 > + %uint_4 = OpConstant %uint 4 > +%_arr_v4uint_uint_4 = OpTypeArray %v4uint %uint_4 > + %_struct_18 = OpTypeStruct %_arr_v4uint_uint_4 > +%_ptr_Uniform__struct_18 = OpTypePointer Uniform %_struct_18 > + %20 = OpVariable %_ptr_Uniform__struct_18 Uniform > + %uint_0 = OpConstant %uint 0 > +%_ptr_Uniform_uint = OpTypePointer Uniform %uint > +%_arr_v4uint_uint_4_0 = OpTypeArray %v4uint %uint_4 > + %_struct_26 = OpTypeStruct %_arr_v4uint_uint_4_0 > +%_ptr_Uniform__struct_26 = OpTypePointer Uniform %_struct_26 > + %28 = OpVariable %_ptr_Uniform__struct_26 Uniform > + %4 = OpFunction %void None %3 > + %5 = OpLabel > + %10 = OpLoad %7 %9 > + %23 = OpAccessChain %_ptr_Uniform_uint %20 %int_0 %int_0 %uint_0 > + %24 = OpLoad %uint %23 > + %29 = OpAccessChain %_ptr_Uniform_uint %28 %int_0 %int_0 %uint_0 > + %30 = OpLoad %uint %29 > + %31 = OpIAdd %uint %24 %30 > + %32 = OpCompositeConstruct %v4uint %31 %31 %31 %31 > + OpImageWrite %10 %14 %32 > + OpReturn > + OpFunctionEnd > + > +[test] > +link success pEpkey.asc Description: application/pgp-keys ___ Piglit mailing list Piglit@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/piglit
