vulkan: Add some tests for block member decorations

Submitted by apinheiro on Jan. 23, 2019, 3:07 p.m.

Details

Message ID 20190123150721.22167-1-apinheiro@igalia.com
State New
Headers show
Series "vulkan: Add some tests for block member decorations" ( rev: 1 ) in Piglit

Not browsing as part of any series.

Commit Message

apinheiro Jan. 23, 2019, 3:07 p.m.
From: Neil Roberts <nroberts@igalia.com>

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

Patch hide | download patch | download mbox

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 000000000..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 = OpConstant %float 1
+    %float_0 = OpConstant %float 0
+         %15 = OpConstantComposite %v4float %float_1 %float_0 %float_0 %float_1
+%_ptr_Output_v4float = OpTypePointer Output %v4float
+      %int_1 = OpConstant %int 1
+         %19 = OpConstantComposite %v4float %float_0 %float_1 %float_0 %float_1
+      %int_2 = OpConstant %int 2
+         %22 = OpConstantComposite %v4float %float_0 %float_0 %float_1 %float_1
+      %int_3 = OpConstant %int 3
+         %25 = OpConstantComposite %v4float %float_1 %float_1 %float_1 %float_1
+       %uint = OpTypeInt 32 0
+     %uint_1 = OpConstant %uint 1
+%_arr_float_uint_1 = OpTypeArray %float %uint_1
+%gl_PerVertex = OpTypeStruct %v4float %float %_arr_float_uint_1
+%_ptr_Output_gl_PerVertex = OpTypePointer Output %gl_PerVertex
+          %_ = OpVariable %_ptr_Output_gl_PerVertex Output
+%_ptr_Input_v4float = OpTypePointer Input %v4float
+%piglit_vertex = OpVariable %_ptr_Input_v4float Input
+       %main = OpFunction %void None %3
+          %5 = OpLabel
+         %17 = OpAccessChain %_ptr_Output_v4float %name %int_0
+               OpStore %17 %15
+         %20 = OpAccessChain %_ptr_Output_v4float %name %int_1
+               OpStore %20 %19
+         %23 = OpAccessChain %_ptr_Output_v4float %name %int_2
+               OpStore %23 %22
+         %26 = OpAccessChain %_ptr_Output_v4float %name %int_3
+               OpStore %26 %25
+         %35 = OpLoad %v4float %piglit_vertex
+         %36 = OpAccessChain %_ptr_Output_v4float %_ %int_0
+               OpStore %36 %35
+               OpReturn
+               OpFunctionEnd
+
+[fragment shader]
+#version 440
+
+layout(location = 0) in vec4 a;
+layout(location = 1) in vec4 b;
+layout(location = 2) in vec4 c;
+layout(location = 3) in vec4 d;
+
+layout(std140, push_constant) uniform block {
+        int i;
+};
+
+layout(location = 0) out vec4 color;
+
+void main()
+{
+        if (i == 0) {
+                color = a;
+        } else if (i == 1) {
+                color = b;
+        } else if (i == 2) {
+                color = c;
+        } else if (i == 3) {
+                color = d;
+        }
+}
+
+[test]
+uniform int 0 0
+draw rect -1 -1 1 1
+relative probe rect rgb (0.0, 0.0, 0.5, 0.5) (1.0, 0.0, 0.0)
+
+uniform int 0 1
+draw rect 0 -1 1 1
+relative probe rect rgb (0.5, 0.0, 0.5, 0.5) (0.0, 1.0, 0.0)
+
+uniform int 0 2
+draw rect -1 0 1 1
+relative probe rect rgb (0.0, 0.5, 0.5, 0.5) (0.0, 0.0, 1.0)
+
+uniform int 0 3
+draw rect 0 0 1 1
+relative probe rect rgb (0.5, 0.5, 0.5, 0.5) (1.0, 1.0, 1.0)
diff --git a/tests/vulkan/shaders/block-member-layout-location.vk_shader_test b/tests/vulkan/shaders/block-member-layout-location.vk_shader_test
new file mode 100644
index 000000000..dd23331b8
--- /dev/null
+++ b/tests/vulkan/shaders/block-member-layout-location.vk_shader_test
@@ -0,0 +1,69 @@ 
+# Test that interface block members are correctly matched by explicit
+# location, when we assign explicit location for all members of a
+# block, and unsorted.
+
+[vertex shader]
+#version 440
+
+layout(location = 0) in vec4 piglit_vertex;
+
+out block {
+        layout(location = 2) vec4 c;
+        layout(location = 3) vec4 d;
+        layout(location = 0) vec4 a;
+        layout(location = 1) vec4 b;
+} name;
+
+void main()
+{
+        name.a = vec4(1.0, 0.0, 0.0, 1.0);
+        name.b = vec4(0.0, 1.0, 0.0, 1.0);
+        name.c = vec4(0.0, 0.0, 1.0, 1.0);
+        name.d = vec4(1.0, 1.0, 1.0, 1.0);
+
+        gl_Position = piglit_vertex;
+}
+
+[fragment shader]
+#version 440
+
+layout(location = 0) in vec4 a;
+layout(location = 1) in vec4 b;
+layout(location = 2) in vec4 c;
+layout(location = 3) in vec4 d;
+
+layout(std140, push_constant) uniform block {
+        int i;
+};
+
+layout(location = 0) out vec4 color;
+
+void main()
+{
+        if (i == 0) {
+                color = a;
+        } else if (i == 1) {
+                color = b;
+        } else if (i == 2) {
+                color = c;
+        } else if (i == 3) {
+                color = d;
+        }
+}
+
+[test]
+uniform int 0 0
+draw rect -1 -1 1 1
+relative probe rect rgb (0.0, 0.0, 0.5, 0.5) (1.0, 0.0, 0.0)
+
+uniform int 0 1
+draw rect 0 -1 1 1
+relative probe rect rgb (0.5, 0.0, 0.5, 0.5) (0.0, 1.0, 0.0)
+
+uniform int 0 2
+draw rect -1 0 1 1
+relative probe rect rgb (0.0, 0.5, 0.5, 0.5) (0.0, 0.0, 1.0)
+
+uniform int 0 3
+draw rect 0 0 1 1
+relative probe rect rgb (0.5, 0.5, 0.5, 0.5) (1.0, 1.0, 1.0)
diff --git a/tests/vulkan/shaders/block-mixed-layout-location.vk_shader_test b/tests/vulkan/shaders/block-mixed-layout-location.vk_shader_test
new file mode 100644
index 000000000..750dd2b29
--- /dev/null
+++ b/tests/vulkan/shaders/block-mixed-layout-location.vk_shader_test
@@ -0,0 +1,126 @@ 
+# Test that interface block members are correctly matched by explicit
+# location. In this case the main variable has a location value, but
+# there is also one member with an explicit value.
+#
+# Note that we include the spirv assembly. This is because although we
+# used a GLSL shader as reference, we tweak and add comments over 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 "c"
+               OpMemberName %block 1 "d"
+               OpMemberName %block 2 "a"
+               OpMemberName %block 3 "b"
+               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"
+; The block is given a location of 2. The first two members don’t
+; have a location so they should be derived from the block location.
+               OpDecorate %name Location 2
+; The 3rd member has an explicit location. The 4th member should be
+; derived from this.
+               OpMemberDecorate %block 2 Location 0
+               OpDecorate %block Block
+               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_2 = OpConstant %int 2
+    %float_1 = OpConstant %float 1
+    %float_0 = OpConstant %float 0
+         %15 = OpConstantComposite %v4float %float_1 %float_0 %float_0 %float_1
+%_ptr_Output_v4float = OpTypePointer Output %v4float
+      %int_3 = OpConstant %int 3
+         %19 = OpConstantComposite %v4float %float_0 %float_1 %float_0 %float_1
+      %int_0 = OpConstant %int 0
+         %22 = OpConstantComposite %v4float %float_0 %float_0 %float_1 %float_1
+      %int_1 = OpConstant %int 1
+         %25 = OpConstantComposite %v4float %float_1 %float_1 %float_1 %float_1
+       %uint = OpTypeInt 32 0
+     %uint_1 = OpConstant %uint 1
+%_arr_float_uint_1 = OpTypeArray %float %uint_1
+%gl_PerVertex = OpTypeStruct %v4float %float %_arr_float_uint_1
+%_ptr_Output_gl_PerVertex = OpTypePointer Output %gl_PerVertex
+          %_ = OpVariable %_ptr_Output_gl_PerVertex Output
+%_ptr_Input_v4float = OpTypePointer Input %v4float
+%piglit_vertex = OpVariable %_ptr_Input_v4float Input
+       %main = OpFunction %void None %3
+          %5 = OpLabel
+         %17 = OpAccessChain %_ptr_Output_v4float %name %int_2
+               OpStore %17 %15
+         %20 = OpAccessChain %_ptr_Output_v4float %name %int_3
+               OpStore %20 %19
+         %23 = OpAccessChain %_ptr_Output_v4float %name %int_0
+               OpStore %23 %22
+         %26 = OpAccessChain %_ptr_Output_v4float %name %int_1
+               OpStore %26 %25
+         %35 = OpLoad %v4float %piglit_vertex
+         %36 = OpAccessChain %_ptr_Output_v4float %_ %int_0
+               OpStore %36 %35
+               OpReturn
+               OpFunctionEnd
+
+[fragment shader]
+#version 440
+
+layout(location = 0) in vec4 a;
+layout(location = 1) in vec4 b;
+layout(location = 2) in vec4 c;
+layout(location = 3) in vec4 d;
+
+layout(std140, push_constant) uniform block {
+        int i;
+};
+
+layout(location = 0) out vec4 color;
+
+void main()
+{
+        if (i == 0) {
+                color = a;
+        } else if (i == 1) {
+                color = b;
+        } else if (i == 2) {
+                color = c;
+        } else if (i == 3) {
+                color = d;
+        }
+}
+
+[test]
+uniform int 0 0
+draw rect -1 -1 1 1
+relative probe rect rgb (0.0, 0.0, 0.5, 0.5) (1.0, 0.0, 0.0)
+
+uniform int 0 1
+draw rect 0 -1 1 1
+relative probe rect rgb (0.5, 0.0, 0.5, 0.5) (0.0, 1.0, 0.0)
+
+uniform int 0 2
+draw rect -1 0 1 1
+relative probe rect rgb (0.0, 0.5, 0.5, 0.5) (0.0, 0.0, 1.0)
+
+uniform int 0 3
+draw rect 0 0 1 1
+relative probe rect rgb (0.5, 0.5, 0.5, 0.5) (1.0, 1.0, 1.0)

Comments

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)?

Thanks,

-Lionel

On 23/01/2019 15:07, Alejandro Piñeiro wrote:
> From: Neil Roberts <nroberts@igalia.com>
>
> 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 000000000..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 = OpConstant %float 1
> +    %float_0 = OpConstant %float 0
> +         %15 = OpConstantComposite %v4float %float_1 %float_0 %float_0 %float_1
> +%_ptr_Output_v4float = OpTypePointer Output %v4float
> +      %int_1 = OpConstant %int 1
> +         %19 = OpConstantComposite %v4float %float_0 %float_1 %float_0 %float_1
> +      %int_2 = OpConstant %int 2
> +         %22 = OpConstantComposite %v4float %float_0 %float_0 %float_1 %float_1
> +      %int_3 = OpConstant %int 3
> +         %25 = OpConstantComposite %v4float %float_1 %float_1 %float_1 %float_1
> +       %uint = OpTypeInt 32 0
> +     %uint_1 = OpConstant %uint 1
> +%_arr_float_uint_1 = OpTypeArray %float %uint_1
> +%gl_PerVertex = OpTypeStruct %v4float %float %_arr_float_uint_1
> +%_ptr_Output_gl_PerVertex = OpTypePointer Output %gl_PerVertex
> +          %_ = OpVariable %_ptr_Output_gl_PerVertex Output
> +%_ptr_Input_v4float = OpTypePointer Input %v4float
> +%piglit_vertex = OpVariable %_ptr_Input_v4float Input
> +       %main = OpFunction %void None %3
> +          %5 = OpLabel
> +         %17 = OpAccessChain %_ptr_Output_v4float %name %int_0
> +               OpStore %17 %15
> +         %20 = OpAccessChain %_ptr_Output_v4float %name %int_1
> +               OpStore %20 %19
> +         %23 = OpAccessChain %_ptr_Output_v4float %name %int_2
> +               OpStore %23 %22
> +         %26 = OpAccessChain %_ptr_Output_v4float %name %int_3
> +               OpStore %26 %25
> +         %35 = OpLoad %v4float %piglit_vertex
> +         %36 = OpAccessChain %_ptr_Output_v4float %_ %int_0
> +               OpStore %36 %35
> +               OpReturn
> +               OpFunctionEnd
> +
> +[fragment shader]
> +#version 440
> +
> +layout(location = 0) in vec4 a;
> +layout(location = 1) in vec4 b;
> +layout(location = 2) in vec4 c;
> +layout(location = 3) in vec4 d;
> +
> +layout(std140, push_constant) uniform block {
> +        int i;
> +};
> +
> +layout(location = 0) out vec4 color;
> +
> +void main()
> +{
> +        if (i == 0) {
> +                color = a;
> +        } else if (i == 1) {
> +                color = b;
> +        } else if (i == 2) {
> +                color = c;
> +        } else if (i == 3) {
> +                color = d;
> +        }
> +}
> +
> +[test]
> +uniform int 0 0
> +draw rect -1 -1 1 1
> +relative probe rect rgb (0.0, 0.0, 0.5, 0.5) (1.0, 0.0, 0.0)
> +
> +uniform int 0 1
> +draw rect 0 -1 1 1
> +relative probe rect rgb (0.5, 0.0, 0.5, 0.5) (0.0, 1.0, 0.0)
> +
> +uniform int 0 2
> +draw rect -1 0 1 1
> +relative probe rect rgb (0.0, 0.5, 0.5, 0.5) (0.0, 0.0, 1.0)
> +
> +uniform int 0 3
> +draw rect 0 0 1 1
> +relative probe rect rgb (0.5, 0.5, 0.5, 0.5) (1.0, 1.0, 1.0)
> diff --git a/tests/vulkan/shaders/block-member-layout-location.vk_shader_test b/tests/vulkan/shaders/block-member-layout-location.vk_shader_test
> new file mode 100644
> index 000000000..dd23331b8
> --- /dev/null
> +++ b/tests/vulkan/shaders/block-member-layout-location.vk_shader_test
> @@ -0,0 +1,69 @@
> +# Test that interface block members are correctly matched by explicit
> +# location, when we assign explicit location for all members of a
> +# block, and unsorted.
> +
> +[vertex shader]
> +#version 440
> +
> +layout(location = 0) in vec4 piglit_vertex;
> +
> +out block {
> +        layout(location = 2) vec4 c;
> +        layout(location = 3) vec4 d;
> +        layout(location = 0) vec4 a;
> +        layout(location = 1) vec4 b;
> +} name;
> +
> +void main()
> +{
> +        name.a = vec4(1.0, 0.0, 0.0, 1.0);
> +        name.b = vec4(0.0, 1.0, 0.0, 1.0);
> +        name.c = vec4(0.0, 0.0, 1.0, 1.0);
> +        name.d = vec4(1.0, 1.0, 1.0, 1.0);
> +
> +        gl_Position = piglit_vertex;
> +}
> +
> +[fragment shader]
> +#version 440
> +
> +layout(location = 0) in vec4 a;
> +layout(location = 1) in vec4 b;
> +layout(location = 2) in vec4 c;
> +layout(location = 3) in vec4 d;
> +
> +layout(std140, push_constant) uniform block {
> +        int i;
> +};
> +
> +layout(location = 0) out vec4 color;
> +
> +void main()
> +{
> +        if (i == 0) {
> +                color = a;
> +        } else if (i == 1) {
> +                color = b;
> +        } else if (i == 2) {
> +                color = c;
> +        } else if (i == 3) {
> +                color = d;
> +        }
> +}
> +
> +[test]
> +uniform int 0 0
> +draw rect -1 -1 1 1
> +relative probe rect rgb (0.0, 0.0, 0.5, 0.5) (1.0, 0.0, 0.0)
> +
> +uniform int 0 1
> +draw rect 0 -1 1 1
> +relative probe rect rgb (0.5, 0.0, 0.5, 0.5) (0.0, 1.0, 0.0)
> +
> +uniform int 0 2
> +draw rect -1 0 1 1
> +relative probe rect rgb (0.0, 0.5, 0.5, 0.5) (0.0, 0.0, 1.0)
> +
> +uniform int 0 3
> +draw rect 0 0 1 1
> +relative probe rect rgb (0.5, 0.5, 0.5, 0.5) (1.0, 1.0, 1.0)
> diff --git a/tests/vulkan/shaders/block-mixed-layout-location.vk_shader_test b/tests/vulkan/shaders/block-mixed-layout-location.vk_shader_test
> new file mode 100644
> index 000000000..750dd2b29
> --- /dev/null
> +++ b/tests/vulkan/shaders/block-mixed-layout-location.vk_shader_test
> @@ -0,0 +1,126 @@
> +# Test that interface block members are correctly matched by explicit
> +# location. In this case the main variable has a location value, but
> +# there is also one member with an explicit value.
> +#
> +# Note that we include the spirv assembly. This is because although we
> +# used a GLSL shader as reference, we tweak and add comments over 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 "c"
> +               OpMemberName %block 1 "d"
> +               OpMemberName %block 2 "a"
> +               OpMemberName %block 3 "b"
> +               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"
> +; The block is given a location of 2. The first two members don’t
> +; have a location so they should be derived from the block location.
> +               OpDecorate %name Location 2
> +; The 3rd member has an explicit location. The 4th member should be
> +; derived from this.
> +               OpMemberDecorate %block 2 Location 0
> +               OpDecorate %block Block
> +               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_2 = OpConstant %int 2
> +    %float_1 = OpConstant %float 1
> +    %float_0 = OpConstant %float 0
> +         %15 = OpConstantComposite %v4float %float_1 %float_0 %float_0 %float_1
> +%_ptr_Output_v4float = OpTypePointer Output %v4float
> +      %int_3 = OpConstant %int 3
> +         %19 = OpConstantComposite %v4float %float_0 %float_1 %float_0 %float_1
> +      %int_0 = OpConstant %int 0
> +         %22 = OpConstantComposite %v4float %float_0 %float_0 %float_1 %float_1
> +      %int_1 = OpConstant %int 1
> +         %25 = OpConstantComposite %v4float %float_1 %float_1 %float_1 %float_1
> +       %uint = OpTypeInt 32 0
> +     %uint_1 = OpConstant %uint 1
> +%_arr_float_uint_1 = OpTypeArray %float %uint_1
> +%gl_PerVertex = OpTypeStruct %v4float %float %_arr_float_uint_1
> +%_ptr_Output_gl_PerVertex = OpTypePointer Output %gl_PerVertex
> +          %_ = OpVariable %_ptr_Output_gl_PerVertex Output
> +%_ptr_Input_v4float = OpTypePointer Input %v4float
> +%piglit_vertex = OpVariable %_ptr_Input_v4float Input
> +       %main = OpFunction %void None %3
> +          %5 = OpLabel
> +         %17 = OpAccessChain %_ptr_Output_v4float %name %int_2
> +               OpStore %17 %15
> +         %20 = OpAccessChain %_ptr_Output_v4float %name %int_3
> +               OpStore %20 %19
> +         %23 = OpAccessChain %_ptr_Output_v4float %name %int_0
> +               OpStore %23 %22
> +         %26 = OpAccessChain %_ptr_Output_v4float %name %int_1
> +               OpStore %26 %25
> +         %35 = OpLoad %v4float %piglit_vertex
> +         %36 = OpAccessChain %_ptr_Output_v4float %_ %int_0
> +               OpStore %36 %35
> +               OpReturn
> +               OpFunctionEnd
> +
> +[fragment shader]
> +#version 440
> +
> +layout(location = 0) in vec4 a;
> +layout(location = 1) in vec4 b;
> +layout(location = 2) in vec4 c;
> +layout(location = 3) in vec4 d;
> +
> +layout(std140, push_constant) uniform block {
> +        int i;
> +};
> +
> +layout(location = 0) out vec4 color;
> +
> +void main()
> +{
> +        if (i == 0) {
> +                color = a;
> +        } else if (i == 1) {
> +                color = b;
> +        } else if (i == 2) {
> +                color = c;
> +        } else if (i == 3) {
> +                color = d;
> +        }
> +}
> +
> +[test]
> +uniform int 0 0
> +draw rect -1 -1 1 1
> +relative probe rect rgb (0.0, 0.0, 0.5, 0.5) (1.0, 0.0, 0.0)
> +
> +uniform int 0 1
> +draw rect 0 -1 1 1
> +relative probe rect rgb (0.5, 0.0, 0.5, 0.5) (0.0, 1.0, 0.0)
> +
> +uniform int 0 2
> +draw rect -1 0 1 1
> +relative probe rect rgb (0.0, 0.5, 0.5, 0.5) (0.0, 0.0, 1.0)
> +
> +uniform int 0 3
> +draw rect 0 0 1 1
> +relative probe rect rgb (0.5, 0.5, 0.5, 0.5) (1.0, 1.0, 1.0)
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 <nroberts@igalia.com>
>>
>> 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 000000000..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 = OpConstant %float 1
>> +    %float_0 = OpConstant %float 0
>> +         %15 = OpConstantComposite %v4float %float_1 %float_0 
>> %float_0 %float_1
>> +%_ptr_Output_v4float = OpTypePointer Output %v4float
>> +      %int_1 = OpConstant %int 1
>> +         %19 = OpConstantComposite %v4float %float_0 %float_1 
>> %float_0 %float_1
>> +      %int_2 = OpConstant %int 2
>> +         %22 = OpConstantComposite %v4float %float_0 %float_0 
>> %float_1 %float_1
>> +      %int_3 = OpConstant %int 3
>> +         %25 = OpConstantComposite %v4float %float_1 %float_1 
>> %float_1 %float_1
>> +       %uint = OpTypeInt 32 0
>> +     %uint_1 = OpConstant %uint 1
>> +%_arr_float_uint_1 = OpTypeArray %float %uint_1
>> +%gl_PerVertex = OpTypeStruct %v4float %float %_arr_float_uint_1
>> +%_ptr_Output_gl_PerVertex = OpTypePointer Output %gl_PerVertex
>> +          %_ = OpVariable %_ptr_Output_gl_PerVertex Output
>> +%_ptr_Input_v4float = OpTypePointer Input %v4float
>> +%piglit_vertex = OpVariable %_ptr_Input_v4float Input
>> +       %main = OpFunction %void None %3
>> +          %5 = OpLabel
>> +         %17 = OpAccessChain %_ptr_Output_v4float %name %int_0
>> +               OpStore %17 %15
>> +         %20 = OpAccessChain %_ptr_Output_v4float %name %int_1
>> +               OpStore %20 %19
>> +         %23 = OpAccessChain %_ptr_Output_v4float %name %int_2
>> +               OpStore %23 %22
>> +         %26 = OpAccessChain %_ptr_Output_v4float %name %int_3
>> +               OpStore %26 %25
>> +         %35 = OpLoad %v4float %piglit_vertex
>> +         %36 = OpAccessChain %_ptr_Output_v4float %_ %int_0
>> +               OpStore %36 %35
>> +               OpReturn
>> +               OpFunctionEnd
>> +
>> +[fragment shader]
>> +#version 440
>> +
>> +layout(location = 0) in vec4 a;
>> +layout(location = 1) in vec4 b;
>> +layout(location = 2) in vec4 c;
>> +layout(location = 3) in vec4 d;
>> +
>> +layout(std140, push_constant) uniform block {
>> +        int i;
>> +};
>> +
>> +layout(location = 0) out vec4 color;
>> +
>> +void main()
>> +{
>> +        if (i == 0) {
>> +                color = a;
>> +        } else if (i == 1) {
>> +                color = b;
>> +        } else if (i == 2) {
>> +                color = c;
>> +        } else if (i == 3) {
>> +                color = d;
>> +        }
>> +}
>> +
>> +[test]
>> +uniform int 0 0
>> +draw rect -1 -1 1 1
>> +relative probe rect rgb (0.0, 0.0, 0.5, 0.5) (1.0, 0.0, 0.0)
>> +
>> +uniform int 0 1
>> +draw rect 0 -1 1 1
>> +relative probe rect rgb (0.5, 0.0, 0.5, 0.5) (0.0, 1.0, 0.0)
>> +
>> +uniform int 0 2
>> +draw rect -1 0 1 1
>> +relative probe rect rgb (0.0, 0.5, 0.5, 0.5) (0.0, 0.0, 1.0)
>> +
>> +uniform int 0 3
>> +draw rect 0 0 1 1
>> +relative probe rect rgb (0.5, 0.5, 0.5, 0.5) (1.0, 1.0, 1.0)
>> diff --git 
>> a/tests/vulkan/shaders/block-member-layout-location.vk_shader_test 
>> b/tests/vulkan/shaders/block-member-layout-location.vk_shader_test
>> new file mode 100644
>> index 000000000..dd23331b8
>> --- /dev/null
>> +++ b/tests/vulkan/shaders/block-member-layout-location.vk_shader_test
>> @@ -0,0 +1,69 @@
>> +# Test that interface block members are correctly matched by explicit
>> +# location, when we assign explicit location for all members of a
>> +# block, and unsorted.
>> +
>> +[vertex shader]
>> +#version 440
>> +
>> +layout(location = 0) in vec4 piglit_vertex;
>> +
>> +out block {
>> +        layout(location = 2) vec4 c;
>> +        layout(location = 3) vec4 d;
>> +        layout(location = 0) vec4 a;
>> +        layout(location = 1) vec4 b;
>> +} name;
>> +
>> +void main()
>> +{
>> +        name.a = vec4(1.0, 0.0, 0.0, 1.0);
>> +        name.b = vec4(0.0, 1.0, 0.0, 1.0);
>> +        name.c = vec4(0.0, 0.0, 1.0, 1.0);
>> +        name.d = vec4(1.0, 1.0, 1.0, 1.0);
>> +
>> +        gl_Position = piglit_vertex;
>> +}
>> +
>> +[fragment shader]
>> +#version 440
>> +
>> +layout(location = 0) in vec4 a;
>> +layout(location = 1) in vec4 b;
>> +layout(location = 2) in vec4 c;
>> +layout(location = 3) in vec4 d;
>> +
>> +layout(std140, push_constant) uniform block {
>> +        int i;
>> +};
>> +
>> +layout(location = 0) out vec4 color;
>> +
>> +void main()
>> +{
>> +        if (i == 0) {
>> +                color = a;
>> +        } else if (i == 1) {
>> +                color = b;
>> +        } else if (i == 2) {
>> +                color = c;
>> +        } else if (i == 3) {
>> +                color = d;
>> +        }
>> +}
>> +
>> +[test]
>> +uniform int 0 0
>> +draw rect -1 -1 1 1
>> +relative probe rect rgb (0.0, 0.0, 0.5, 0.5) (1.0, 0.0, 0.0)
>> +
>> +uniform int 0 1
>> +draw rect 0 -1 1 1
>> +relative probe rect rgb (0.5, 0.0, 0.5, 0.5) (0.0, 1.0, 0.0)
>> +
>> +uniform int 0 2
>> +draw rect -1 0 1 1
>> +relative probe rect rgb (0.0, 0.5, 0.5, 0.5) (0.0, 0.0, 1.0)
>> +
>> +uniform int 0 3
>> +draw rect 0 0 1 1
>> +relative probe rect rgb (0.5, 0.5, 0.5, 0.5) (1.0, 1.0, 1.0)
>> diff --git 
>> a/tests/vulkan/shaders/block-mixed-layout-location.vk_shader_test 
>> b/tests/vulkan/shaders/block-mixed-layout-location.vk_shader_test
>> new file mode 100644
>> index 000000000..750dd2b29
>> --- /dev/null
>> +++ b/tests/vulkan/shaders/block-mixed-layout-location.vk_shader_test
>> @@ -0,0 +1,126 @@
>> +# Test that interface block members are correctly matched by explicit
>> +# location. In this case the main variable has a location value, but
>> +# there is also one member with an explicit value.
>> +#
>> +# Note that we include the spirv assembly. This is because although we
>> +# used a GLSL shader as reference, we tweak and add comments over 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 "c"
>> +               OpMemberName %block 1 "d"
>> +               OpMemberName %block 2 "a"
>> +               OpMemberName %block 3 "b"
>> +               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"
>> +; The block is given a location of 2. The first two members don’t
>> +; have a location so they should be derived from the block location.
>> +               OpDecorate %name Location 2
>> +; The 3rd member has an explicit location. The 4th member should be
>> +; derived from this.
>> +               OpMemberDecorate %block 2 Location 0
>> +               OpDecorate %block Block
>> +               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_2 = OpConstant %int 2
>> +    %float_1 = OpConstant %float 1
>> +    %float_0 = OpConstant %float 0
>> +         %15 = OpConstantComposite %v4float %float_1 %float_0 
>> %float_0 %float_1
>> +%_ptr_Output_v4float = OpTypePointer Output %v4float
>> +      %int_3 = OpConstant %int 3
>> +         %19 = OpConstantComposite %v4float %float_0 %float_1 
>> %float_0 %float_1
>> +      %int_0 = OpConstant %int 0
>> +         %22 = OpConstantComposite %v4float %float_0 %float_0 
>> %float_1 %float_1
>> +      %int_1 = OpConstant %int 1
>> +         %25 = OpConstantComposite %v4float %float_1 %float_1 
>> %float_1 %float_1
>> +       %uint = OpTypeInt 32 0
>> +     %uint_1 = OpConstant %uint 1
>> +%_arr_float_uint_1 = OpTypeArray %float %uint_1
>> +%gl_PerVertex = OpTypeStruct %v4float %float %_arr_float_uint_1
>> +%_ptr_Output_gl_PerVertex = OpTypePointer Output %gl_PerVertex
>> +          %_ = OpVariable %_ptr_Output_gl_PerVertex Output
>> +%_ptr_Input_v4float = OpTypePointer Input %v4float
>> +%piglit_vertex = OpVariable %_ptr_Input_v4float Input
>> +       %main = OpFunction %void None %3
>> +          %5 = OpLabel
>> +         %17 = OpAccessChain %_ptr_Output_v4float %name %int_2
>> +               OpStore %17 %15
>> +         %20 = OpAccessChain %_ptr_Output_v4float %name %int_3
>> +               OpStore %20 %19
>> +         %23 = OpAccessChain %_ptr_Output_v4float %name %int_0
>> +               OpStore %23 %22
>> +         %26 = OpAccessChain %_ptr_Output_v4float %name %int_1
>> +               OpStore %26 %25
>> +         %35 = OpLoad %v4float %piglit_vertex
>> +         %36 = OpAccessChain %_ptr_Output_v4float %_ %int_0
>> +               OpStore %36 %35
>> +               OpReturn
>> +               OpFunctionEnd
>> +
>> +[fragment shader]
>> +#version 440
>> +
>> +layout(location = 0) in vec4 a;
>> +layout(location = 1) in vec4 b;
>> +layout(location = 2) in vec4 c;
>> +layout(location = 3) in vec4 d;
>> +
>> +layout(std140, push_constant) uniform block {
>> +        int i;
>> +};
>> +
>> +layout(location = 0) out vec4 color;
>> +
>> +void main()
>> +{
>> +        if (i == 0) {
>> +                color = a;
>> +        } else if (i == 1) {
>> +                color = b;
>> +        } else if (i == 2) {
>> +                color = c;
>> +        } else if (i == 3) {
>> +                color = d;
>> +        }
>> +}
>> +
>> +[test]
>> +uniform int 0 0
>> +draw rect -1 -1 1 1
>> +relative probe rect rgb (0.0, 0.0, 0.5, 0.5) (1.0, 0.0, 0.0)
>> +
>> +uniform int 0 1
>> +draw rect 0 -1 1 1
>> +relative probe rect rgb (0.5, 0.0, 0.5, 0.5) (0.0, 1.0, 0.0)
>> +
>> +uniform int 0 2
>> +draw rect -1 0 1 1
>> +relative probe rect rgb (0.0, 0.5, 0.5, 0.5) (0.0, 0.0, 1.0)
>> +
>> +uniform int 0 3
>> +draw rect 0 0 1 1
>> +relative probe rect rgb (0.5, 0.5, 0.5, 0.5) (1.0, 1.0, 1.0)
>
>
>
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.

But this current version looks good to me :

Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>

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;
};

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 <nroberts@igalia.com>
>>>
>>> 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 000000000..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 = OpConstant %float 1
>>> +    %float_0 = OpConstant %float 0
>>> +         %15 = OpConstantComposite %v4float %float_1 %float_0 
>>> %float_0 %float_1
>>> +%_ptr_Output_v4float = OpTypePointer Output %v4float
>>> +      %int_1 = OpConstant %int 1
>>> +         %19 = OpConstantComposite %v4float %float_0 %float_1 
>>> %float_0 %float_1
>>> +      %int_2 = OpConstant %int 2
>>> +         %22 = OpConstantComposite %v4float %float_0 %float_0 
>>> %float_1 %float_1
>>> +      %int_3 = OpConstant %int 3
>>> +         %25 = OpConstantComposite %v4float %float_1 %float_1 
>>> %float_1 %float_1
>>> +       %uint = OpTypeInt 32 0
>>> +     %uint_1 = OpConstant %uint 1
>>> +%_arr_float_uint_1 = OpTypeArray %float %uint_1
>>> +%gl_PerVertex = OpTypeStruct %v4float %float %_arr_float_uint_1
>>> +%_ptr_Output_gl_PerVertex = OpTypePointer Output %gl_PerVertex
>>> +          %_ = OpVariable %_ptr_Output_gl_PerVertex Output
>>> +%_ptr_Input_v4float = OpTypePointer Input %v4float
>>> +%piglit_vertex = OpVariable %_ptr_Input_v4float Input
>>> +       %main = OpFunction %void None %3
>>> +          %5 = OpLabel
>>> +         %17 = OpAccessChain %_ptr_Output_v4float %name %int_0
>>> +               OpStore %17 %15
>>> +         %20 = OpAccessChain %_ptr_Output_v4float %name %int_1
>>> +               OpStore %20 %19
>>> +         %23 = OpAccessChain %_ptr_Output_v4float %name %int_2
>>> +               OpStore %23 %22
>>> +         %26 = OpAccessChain %_ptr_Output_v4float %name %int_3
>>> +               OpStore %26 %25
>>> +         %35 = OpLoad %v4float %piglit_vertex
>>> +         %36 = OpAccessChain %_ptr_Output_v4float %_ %int_0
>>> +               OpStore %36 %35
>>> +               OpReturn
>>> +               OpFunctionEnd
>>> +
>>> +[fragment shader]
>>> +#version 440
>>> +
>>> +layout(location = 0) in vec4 a;
>>> +layout(location = 1) in vec4 b;
>>> +layout(location = 2) in vec4 c;
>>> +layout(location = 3) in vec4 d;
>>> +
>>> +layout(std140, push_constant) uniform block {
>>> +        int i;
>>> +};
>>> +
>>> +layout(location = 0) out vec4 color;
>>> +
>>> +void main()
>>> +{
>>> +        if (i == 0) {
>>> +                color = a;
>>> +        } else if (i == 1) {
>>> +                color = b;
>>> +        } else if (i == 2) {
>>> +                color = c;
>>> +        } else if (i == 3) {
>>> +                color = d;
>>> +        }
>>> +}
>>> +
>>> +[test]
>>> +uniform int 0 0
>>> +draw rect -1 -1 1 1
>>> +relative probe rect rgb (0.0, 0.0, 0.5, 0.5) (1.0, 0.0, 0.0)
>>> +
>>> +uniform int 0 1
>>> +draw rect 0 -1 1 1
>>> +relative probe rect rgb (0.5, 0.0, 0.5, 0.5) (0.0, 1.0, 0.0)
>>> +
>>> +uniform int 0 2
>>> +draw rect -1 0 1 1
>>> +relative probe rect rgb (0.0, 0.5, 0.5, 0.5) (0.0, 0.0, 1.0)
>>> +
>>> +uniform int 0 3
>>> +draw rect 0 0 1 1
>>> +relative probe rect rgb (0.5, 0.5, 0.5, 0.5) (1.0, 1.0, 1.0)
>>> diff --git 
>>> a/tests/vulkan/shaders/block-member-layout-location.vk_shader_test 
>>> b/tests/vulkan/shaders/block-member-layout-location.vk_shader_test
>>> new file mode 100644
>>> index 000000000..dd23331b8
>>> --- /dev/null
>>> +++ b/tests/vulkan/shaders/block-member-layout-location.vk_shader_test
>>> @@ -0,0 +1,69 @@
>>> +# Test that interface block members are correctly matched by explicit
>>> +# location, when we assign explicit location for all members of a
>>> +# block, and unsorted.
>>> +
>>> +[vertex shader]
>>> +#version 440
>>> +
>>> +layout(location = 0) in vec4 piglit_vertex;
>>> +
>>> +out block {
>>> +        layout(location = 2) vec4 c;
>>> +        layout(location = 3) vec4 d;
>>> +        layout(location = 0) vec4 a;
>>> +        layout(location = 1) vec4 b;
>>> +} name;
>>> +
>>> +void main()
>>> +{
>>> +        name.a = vec4(1.0, 0.0, 0.0, 1.0);
>>> +        name.b = vec4(0.0, 1.0, 0.0, 1.0);
>>> +        name.c = vec4(0.0, 0.0, 1.0, 1.0);
>>> +        name.d = vec4(1.0, 1.0, 1.0, 1.0);
>>> +
>>> +        gl_Position = piglit_vertex;
>>> +}
>>> +
>>> +[fragment shader]
>>> +#version 440
>>> +
>>> +layout(location = 0) in vec4 a;
>>> +layout(location = 1) in vec4 b;
>>> +layout(location = 2) in vec4 c;
>>> +layout(location = 3) in vec4 d;
>>> +
>>> +layout(std140, push_constant) uniform block {
>>> +        int i;
>>> +};
>>> +
>>> +layout(location = 0) out vec4 color;
>>> +
>>> +void main()
>>> +{
>>> +        if (i == 0) {
>>> +                color = a;
>>> +        } else if (i == 1) {
>>> +                color = b;
>>> +        } else if (i == 2) {
>>> +                color = c;
>>> +        } else if (i == 3) {
>>> +                color = d;
>>> +        }
>>> +}
>>> +
>>> +[test]
>>> +uniform int 0 0
>>> +draw rect -1 -1 1 1
>>> +relative probe rect rgb (0.0, 0.0, 0.5, 0.5) (1.0, 0.0, 0.0)
>>> +
>>> +uniform int 0 1
>>> +draw rect 0 -1 1 1
>>> +relative probe rect rgb (0.5, 0.0, 0.5, 0.5) (0.0, 1.0, 0.0)
>>> +
>>> +uniform int 0 2
>>> +draw rect -1 0 1 1
>>> +relative probe rect rgb (0.0, 0.5, 0.5, 0.5) (0.0, 0.0, 1.0)
>>> +
>>> +uniform int 0 3
>>> +draw rect 0 0 1 1
>>> +relative probe rect rgb (0.5, 0.5, 0.5, 0.5) (1.0, 1.0, 1.0)
>>> diff --git 
>>> a/tests/vulkan/shaders/block-mixed-layout-location.vk_shader_test 
>>> b/tests/vulkan/shaders/block-mixed-layout-location.vk_shader_test
>>> new file mode 100644
>>> index 000000000..750dd2b29
>>> --- /dev/null
>>> +++ b/tests/vulkan/shaders/block-mixed-layout-location.vk_shader_test
>>> @@ -0,0 +1,126 @@
>>> +# Test that interface block members are correctly matched by explicit
>>> +# location. In this case the main variable has a location value, but
>>> +# there is also one member with an explicit value.
>>> +#
>>> +# Note that we include the spirv assembly. This is because although we
>>> +# used a GLSL shader as reference, we tweak and add comments over 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 "c"
>>> +               OpMemberName %block 1 "d"
>>> +               OpMemberName %block 2 "a"
>>> +               OpMemberName %block 3 "b"
>>> +               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"
>>> +; The block is given a location of 2. The first two members don’t
>>> +; have a location so they should be derived from the block location.
>>> +               OpDecorate %name Location 2
>>> +; The 3rd member has an explicit location. The 4th member should be
>>> +; derived from this.
>>> +               OpMemberDecorate %block 2 Location 0
>>> +               OpDecorate %block Block
>>> +               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_2 = OpConstant %int 2
>>> +    %float_1 = OpConstant %float 1
>>> +    %float_0 = OpConstant %float 0
>>> +         %15 = OpConstantComposite %v4float %float_1 %float_0 
>>> %float_0 %float_1
>>> +%_ptr_Output_v4float = OpTypePointer Output %v4float
>>> +      %int_3 = OpConstant %int 3
>>> +         %19 = OpConstantComposite %v4float %float_0 %float_1 
>>> %float_0 %float_1
>>> +      %int_0 = OpConstant %int 0
>>> +         %22 = OpConstantComposite %v4float %float_0 %float_0 
>>> %float_1 %float_1
>>> +      %int_1 = OpConstant %int 1
>>> +         %25 = OpConstantComposite %v4float %float_1 %float_1 
>>> %float_1 %float_1
>>> +       %uint = OpTypeInt 32 0
>>> +     %uint_1 = OpConstant %uint 1
>>> +%_arr_float_uint_1 = OpTypeArray %float %uint_1
>>> +%gl_PerVertex = OpTypeStruct %v4float %float %_arr_float_uint_1
>>> +%_ptr_Output_gl_PerVertex = OpTypePointer Output %gl_PerVertex
>>> +          %_ = OpVariable %_ptr_Output_gl_PerVertex Output
>>> +%_ptr_Input_v4float = OpTypePointer Input %v4float
>>> +%piglit_vertex = OpVariable %_ptr_Input_v4float Input
>>> +       %main = OpFunction %void None %3
>>> +          %5 = OpLabel
>>> +         %17 = OpAccessChain %_ptr_Output_v4float %name %int_2
>>> +               OpStore %17 %15
>>> +         %20 = OpAccessChain %_ptr_Output_v4float %name %int_3
>>> +               OpStore %20 %19
>>> +         %23 = OpAccessChain %_ptr_Output_v4float %name %int_0
>>> +               OpStore %23 %22
>>> +         %26 = OpAccessChain %_ptr_Output_v4float %name %int_1
>>> +               OpStore %26 %25
>>> +         %35 = OpLoad %v4float %piglit_vertex
>>> +         %36 = OpAccessChain %_ptr_Output_v4float %_ %int_0
>>> +               OpStore %36 %35
>>> +               OpReturn
>>> +               OpFunctionEnd
>>> +
>>> +[fragment shader]
>>> +#version 440
>>> +
>>> +layout(location = 0) in vec4 a;
>>> +layout(location = 1) in vec4 b;
>>> +layout(location = 2) in vec4 c;
>>> +layout(location = 3) in vec4 d;
>>> +
>>> +layout(std140, push_constant) uniform block {
>>> +        int i;
>>> +};
>>> +
>>> +layout(location = 0) out vec4 color;
>>> +
>>> +void main()
>>> +{
>>> +        if (i == 0) {
>>> +                color = a;
>>> +        } else if (i == 1) {
>>> +                color = b;
>>> +        } else if (i == 2) {
>>> +                color = c;
>>> +        } else if (i == 3) {
>>> +                color = d;
>>> +        }
>>> +}
>>> +
>>> +[test]
>>> +uniform int 0 0
>>> +draw rect -1 -1 1 1
>>> +relative probe rect rgb (0.0, 0.0, 0.5, 0.5) (1.0, 0.0, 0.0)
>>> +
>>> +uniform int 0 1
>>> +draw rect 0 -1 1 1
>>> +relative probe rect rgb (0.5, 0.0, 0.5, 0.5) (0.0, 1.0, 0.0)
>>> +
>>> +uniform int 0 2
>>> +draw rect -1 0 1 1
>>> +relative probe rect rgb (0.0, 0.5, 0.5, 0.5) (0.0, 0.0, 1.0)
>>> +
>>> +uniform int 0 3
>>> +draw rect 0 0 1 1
>>> +relative probe rect rgb (0.5, 0.5, 0.5, 0.5) (1.0, 1.0, 1.0)
>>
>>
>>
>
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 <lionel.g.landwerlin@intel.com>


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 <nroberts@igalia.com>
>>>>
>>>> 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 000000000..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 = OpConstant %float 1
>>>> +    %float_0 = OpConstant %float 0
>>>> +         %15 = OpConstantComposite %v4float %float_1 %float_0 
>>>> %float_0 %float_1
>>>> +%_ptr_Output_v4float = OpTypePointer Output %v4float
>>>> +      %int_1 = OpConstant %int 1
>>>> +         %19 = OpConstantComposite %v4float %float_0 %float_1 
>>>> %float_0 %float_1
>>>> +      %int_2 = OpConstant %int 2
>>>> +         %22 = OpConstantComposite %v4float %float_0 %float_0 
>>>> %float_1 %float_1
>>>> +      %int_3 = OpConstant %int 3
>>>> +         %25 = OpConstantComposite %v4float %float_1 %float_1 
>>>> %float_1 %float_1
>>>> +       %uint = OpTypeInt 32 0
>>>> +     %uint_1 = OpConstant %uint 1
>>>> +%_arr_float_uint_1 = OpTypeArray %float %uint_1
>>>> +%gl_PerVertex = OpTypeStruct %v4float %float %_arr_float_uint_1
>>>> +%_ptr_Output_gl_PerVertex = OpTypePointer Output %gl_PerVertex
>>>> +          %_ = OpVariable %_ptr_Output_gl_PerVertex Output
>>>> +%_ptr_Input_v4float = OpTypePointer Input %v4float
>>>> +%piglit_vertex = OpVariable %_ptr_Input_v4float Input
>>>> +       %main = OpFunction %void None %3
>>>> +          %5 = OpLabel
>>>> +         %17 = OpAccessChain %_ptr_Output_v4float %name %int_0
>>>> +               OpStore %17 %15
>>>> +         %20 = OpAccessChain %_ptr_Output_v4float %name %int_1
>>>> +               OpStore %20 %19
>>>> +         %23 = OpAccessChain %_ptr_Output_v4float %name %int_2
>>>> +               OpStore %23 %22
>>>> +         %26 = OpAccessChain %_ptr_Output_v4float %name %int_3
>>>> +               OpStore %26 %25
>>>> +         %35 = OpLoad %v4float %piglit_vertex
>>>> +         %36 = OpAccessChain %_ptr_Output_v4float %_ %int_0
>>>> +               OpStore %36 %35
>>>> +               OpReturn
>>>> +               OpFunctionEnd
>>>> +
>>>> +[fragment shader]
>>>> +#version 440
>>>> +
>>>> +layout(location = 0) in vec4 a;
>>>> +layout(location = 1) in vec4 b;
>>>> +layout(location = 2) in vec4 c;
>>>> +layout(location = 3) in vec4 d;
>>>> +
>>>> +layout(std140, push_constant) uniform block {
>>>> +        int i;
>>>> +};
>>>> +
>>>> +layout(location = 0) out vec4 color;
>>>> +
>>>> +void main()
>>>> +{
>>>> +        if (i == 0) {
>>>> +                color = a;
>>>> +        } else if (i == 1) {
>>>> +                color = b;
>>>> +        } else if (i == 2) {
>>>> +                color = c;
>>>> +        } else if (i == 3) {
>>>> +                color = d;
>>>> +        }
>>>> +}
>>>> +
>>>> +[test]
>>>> +uniform int 0 0
>>>> +draw rect -1 -1 1 1
>>>> +relative probe rect rgb (0.0, 0.0, 0.5, 0.5) (1.0, 0.0, 0.0)
>>>> +
>>>> +uniform int 0 1
>>>> +draw rect 0 -1 1 1
>>>> +relative probe rect rgb (0.5, 0.0, 0.5, 0.5) (0.0, 1.0, 0.0)
>>>> +
>>>> +uniform int 0 2
>>>> +draw rect -1 0 1 1
>>>> +relative probe rect rgb (0.0, 0.5, 0.5, 0.5) (0.0, 0.0, 1.0)
>>>> +
>>>> +uniform int 0 3
>>>> +draw rect 0 0 1 1
>>>> +relative probe rect rgb (0.5, 0.5, 0.5, 0.5) (1.0, 1.0, 1.0)
>>>> diff --git 
>>>> a/tests/vulkan/shaders/block-member-layout-location.vk_shader_test 
>>>> b/tests/vulkan/shaders/block-member-layout-location.vk_shader_test
>>>> new file mode 100644
>>>> index 000000000..dd23331b8
>>>> --- /dev/null
>>>> +++ b/tests/vulkan/shaders/block-member-layout-location.vk_shader_test
>>>> @@ -0,0 +1,69 @@
>>>> +# Test that interface block members are correctly matched by explicit
>>>> +# location, when we assign explicit location for all members of a
>>>> +# block, and unsorted.
>>>> +
>>>> +[vertex shader]
>>>> +#version 440
>>>> +
>>>> +layout(location = 0) in vec4 piglit_vertex;
>>>> +
>>>> +out block {
>>>> +        layout(location = 2) vec4 c;
>>>> +        layout(location = 3) vec4 d;
>>>> +        layout(location = 0) vec4 a;
>>>> +        layout(location = 1) vec4 b;
>>>> +} name;
>>>> +
>>>> +void main()
>>>> +{
>>>> +        name.a = vec4(1.0, 0.0, 0.0, 1.0);
>>>> +        name.b = vec4(0.0, 1.0, 0.0, 1.0);
>>>> +        name.c = vec4(0.0, 0.0, 1.0, 1.0);
>>>> +        name.d = vec4(1.0, 1.0, 1.0, 1.0);
>>>> +
>>>> +        gl_Position = piglit_vertex;
>>>> +}
>>>> +
>>>> +[fragment shader]
>>>> +#version 440
>>>> +
>>>> +layout(location = 0) in vec4 a;
>>>> +layout(location = 1) in vec4 b;
>>>> +layout(location = 2) in vec4 c;
>>>> +layout(location = 3) in vec4 d;
>>>> +
>>>> +layout(std140, push_constant) uniform block {
>>>> +        int i;
>>>> +};
>>>> +
>>>> +layout(location = 0) out vec4 color;
>>>> +
>>>> +void main()
>>>> +{
>>>> +        if (i == 0) {
>>>> +                color = a;
>>>> +        } else if (i == 1) {
>>>> +                color = b;
>>>> +        } else if (i == 2) {
>>>> +                color = c;
>>>> +        } else if (i == 3) {
>>>> +                color = d;
>>>> +        }
>>>> +}
>>>> +
>>>> +[test]
>>>> +uniform int 0 0
>>>> +draw rect -1 -1 1 1
>>>> +relative probe rect rgb (0.0, 0.0, 0.5, 0.5) (1.0, 0.0, 0.0)
>>>> +
>>>> +uniform int 0 1
>>>> +draw rect 0 -1 1 1
>>>> +relative probe rect rgb (0.5, 0.0, 0.5, 0.5) (0.0, 1.0, 0.0)
>>>> +
>>>> +uniform int 0 2
>>>> +draw rect -1 0 1 1
>>>> +relative probe rect rgb (0.0, 0.5, 0.5, 0.5) (0.0, 0.0, 1.0)
>>>> +
>>>> +uniform int 0 3
>>>> +draw rect 0 0 1 1
>>>> +relative probe rect rgb (0.5, 0.5, 0.5, 0.5) (1.0, 1.0, 1.0)
>>>> diff --git 
>>>> a/tests/vulkan/shaders/block-mixed-layout-location.vk_shader_test 
>>>> b/tests/vulkan/shaders/block-mixed-layout-location.vk_shader_test
>>>> new file mode 100644
>>>> index 000000000..750dd2b29
>>>> --- /dev/null
>>>> +++ b/tests/vulkan/shaders/block-mixed-layout-location.vk_shader_test
>>>> @@ -0,0 +1,126 @@
>>>> +# Test that interface block members are correctly matched by explicit
>>>> +# location. In this case the main variable has a location value, but
>>>> +# there is also one member with an explicit value.
>>>> +#
>>>> +# Note that we include the spirv assembly. This is because 
>>>> although we
>>>> +# used a GLSL shader as reference, we tweak and add comments over 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 "c"
>>>> +               OpMemberName %block 1 "d"
>>>> +               OpMemberName %block 2 "a"
>>>> +               OpMemberName %block 3 "b"
>>>> +               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"
>>>> +; The block is given a location of 2. The first two members don’t
>>>> +; have a location so they should be derived from the block location.
>>>> +               OpDecorate %name Location 2
>>>> +; The 3rd member has an explicit location. The 4th member should be
>>>> +; derived from this.
>>>> +               OpMemberDecorate %block 2 Location 0
>>>> +               OpDecorate %block Block
>>>> +               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_2 = OpConstant %int 2
>>>> +    %float_1 = OpConstant %float 1
>>>> +    %float_0 = OpConstant %float 0
>>>> +         %15 = OpConstantComposite %v4float %float_1 %float_0 
>>>> %float_0 %float_1
>>>> +%_ptr_Output_v4float = OpTypePointer Output %v4float
>>>> +      %int_3 = OpConstant %int 3
>>>> +         %19 = OpConstantComposite %v4float %float_0 %float_1 
>>>> %float_0 %float_1
>>>> +      %int_0 = OpConstant %int 0
>>>> +         %22 = OpConstantComposite %v4float %float_0 %float_0 
>>>> %float_1 %float_1
>>>> +      %int_1 = OpConstant %int 1
>>>> +         %25 = OpConstantComposite %v4float %float_1 %float_1 
>>>> %float_1 %float_1
>>>> +       %uint = OpTypeInt 32 0
>>>> +     %uint_1 = OpConstant %uint 1
>>>> +%_arr_float_uint_1 = OpTypeArray %float %uint_1
>>>> +%gl_PerVertex = OpTypeStruct %v4float %float %_arr_float_uint_1
>>>> +%_ptr_Output_gl_PerVertex = OpTypePointer Output %gl_PerVertex
>>>> +          %_ = OpVariable %_ptr_Output_gl_PerVertex Output
>>>> +%_ptr_Input_v4float = OpTypePointer Input %v4float
>>>> +%piglit_vertex = OpVariable %_ptr_Input_v4float Input
>>>> +       %main = OpFunction %void None %3
>>>> +          %5 = OpLabel
>>>> +         %17 = OpAccessChain %_ptr_Output_v4float %name %int_2
>>>> +               OpStore %17 %15
>>>> +         %20 = OpAccessChain %_ptr_Output_v4float %name %int_3
>>>> +               OpStore %20 %19
>>>> +         %23 = OpAccessChain %_ptr_Output_v4float %name %int_0
>>>> +               OpStore %23 %22
>>>> +         %26 = OpAccessChain %_ptr_Output_v4float %name %int_1
>>>> +               OpStore %26 %25
>>>> +         %35 = OpLoad %v4float %piglit_vertex
>>>> +         %36 = OpAccessChain %_ptr_Output_v4float %_ %int_0
>>>> +               OpStore %36 %35
>>>> +               OpReturn
>>>> +               OpFunctionEnd
>>>> +
>>>> +[fragment shader]
>>>> +#version 440
>>>> +
>>>> +layout(location = 0) in vec4 a;
>>>> +layout(location = 1) in vec4 b;
>>>> +layout(location = 2) in vec4 c;
>>>> +layout(location = 3) in vec4 d;
>>>> +
>>>> +layout(std140, push_constant) uniform block {
>>>> +        int i;
>>>> +};
>>>> +
>>>> +layout(location = 0) out vec4 color;
>>>> +
>>>> +void main()
>>>> +{
>>>> +        if (i == 0) {
>>>> +                color = a;
>>>> +        } else if (i == 1) {
>>>> +                color = b;
>>>> +        } else if (i == 2) {
>>>> +                color = c;
>>>> +        } else if (i == 3) {
>>>> +                color = d;
>>>> +        }
>>>> +}
>>>> +
>>>> +[test]
>>>> +uniform int 0 0
>>>> +draw rect -1 -1 1 1
>>>> +relative probe rect rgb (0.0, 0.0, 0.5, 0.5) (1.0, 0.0, 0.0)
>>>> +
>>>> +uniform int 0 1
>>>> +draw rect 0 -1 1 1
>>>> +relative probe rect rgb (0.5, 0.0, 0.5, 0.5) (0.0, 1.0, 0.0)
>>>> +
>>>> +uniform int 0 2
>>>> +draw rect -1 0 1 1
>>>> +relative probe rect rgb (0.0, 0.5, 0.5, 0.5) (0.0, 0.0, 1.0)
>>>> +
>>>> +uniform int 0 3
>>>> +draw rect 0 0 1 1
>>>> +relative probe rect rgb (0.5, 0.5, 0.5, 0.5) (1.0, 1.0, 1.0)
>>>
>>>
>>>
>>
>
>
On 24/01/2019 15:45, apinheiro wrote:
>
> 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?


I figured the location =1 on myBlock would cover better the issue 
because we would know that the 0 wasn't selected by accident.

So the same test, just modified a bit.


>
>
>>
>> But this current version looks good to me :
>>
>> Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
>
>
> 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! :)


-Lionel

>
>
>>
>> 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 <nroberts@igalia.com>
>>>>>
>>>>> 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 000000000..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 = OpConstant %float 1
>>>>> +    %float_0 = OpConstant %float 0
>>>>> +         %15 = OpConstantComposite %v4float %float_1 %float_0 
>>>>> %float_0 %float_1
>>>>> +%_ptr_Output_v4float = OpTypePointer Output %v4float
>>>>> +      %int_1 = OpConstant %int 1
>>>>> +         %19 = OpConstantComposite %v4float %float_0 %float_1 
>>>>> %float_0 %float_1
>>>>> +      %int_2 = OpConstant %int 2
>>>>> +         %22 = OpConstantComposite %v4float %float_0 %float_0 
>>>>> %float_1 %float_1
>>>>> +      %int_3 = OpConstant %int 3
>>>>> +         %25 = OpConstantComposite %v4float %float_1 %float_1 
>>>>> %float_1 %float_1
>>>>> +       %uint = OpTypeInt 32 0
>>>>> +     %uint_1 = OpConstant %uint 1
>>>>> +%_arr_float_uint_1 = OpTypeArray %float %uint_1
>>>>> +%gl_PerVertex = OpTypeStruct %v4float %float %_arr_float_uint_1
>>>>> +%_ptr_Output_gl_PerVertex = OpTypePointer Output %gl_PerVertex
>>>>> +          %_ = OpVariable %_ptr_Output_gl_PerVertex Output
>>>>> +%_ptr_Input_v4float = OpTypePointer Input %v4float
>>>>> +%piglit_vertex = OpVariable %_ptr_Input_v4float Input
>>>>> +       %main = OpFunction %void None %3
>>>>> +          %5 = OpLabel
>>>>> +         %17 = OpAccessChain %_ptr_Output_v4float %name %int_0
>>>>> +               OpStore %17 %15
>>>>> +         %20 = OpAccessChain %_ptr_Output_v4float %name %int_1
>>>>> +               OpStore %20 %19
>>>>> +         %23 = OpAccessChain %_ptr_Output_v4float %name %int_2
>>>>> +               OpStore %23 %22
>>>>> +         %26 = OpAccessChain %_ptr_Output_v4float %name %int_3
>>>>> +               OpStore %26 %25
>>>>> +         %35 = OpLoad %v4float %piglit_vertex
>>>>> +         %36 = OpAccessChain %_ptr_Output_v4float %_ %int_0
>>>>> +               OpStore %36 %35
>>>>> +               OpReturn
>>>>> +               OpFunctionEnd
>>>>> +
>>>>> +[fragment shader]
>>>>> +#version 440
>>>>> +
>>>>> +layout(location = 0) in vec4 a;
>>>>> +layout(location = 1) in vec4 b;
>>>>> +layout(location = 2) in vec4 c;
>>>>> +layout(location = 3) in vec4 d;
>>>>> +
>>>>> +layout(std140, push_constant) uniform block {
>>>>> +        int i;
>>>>> +};
>>>>> +
>>>>> +layout(location = 0) out vec4 color;
>>>>> +
>>>>> +void main()
>>>>> +{
>>>>> +        if (i == 0) {
>>>>> +                color = a;
>>>>> +        } else if (i == 1) {
>>>>> +                color = b;
>>>>> +        } else if (i == 2) {
>>>>> +                color = c;
>>>>> +        } else if (i == 3) {
>>>>> +                color = d;
>>>>> +        }
>>>>> +}
>>>>> +
>>>>> +[test]
>>>>> +uniform int 0 0
>>>>> +draw rect -1 -1 1 1
>>>>> +relative probe rect rgb (0.0, 0.0, 0.5, 0.5) (1.0, 0.0, 0.0)
>>>>> +
>>>>> +uniform int 0 1
>>>>> +draw rect 0 -1 1 1
>>>>> +relative probe rect rgb (0.5, 0.0, 0.5, 0.5) (0.0, 1.0, 0.0)
>>>>> +
>>>>> +uniform int 0 2
>>>>> +draw rect -1 0 1 1
>>>>> +relative probe rect rgb (0.0, 0.5, 0.5, 0.5) (0.0, 0.0, 1.0)
>>>>> +
>>>>> +uniform int 0 3
>>>>> +draw rect 0 0 1 1
>>>>> +relative probe rect rgb (0.5, 0.5, 0.5, 0.5) (1.0, 1.0, 1.0)
>>>>> diff --git 
>>>>> a/tests/vulkan/shaders/block-member-layout-location.vk_shader_test 
>>>>> b/tests/vulkan/shaders/block-member-layout-location.vk_shader_test
>>>>> new file mode 100644
>>>>> index 000000000..dd23331b8
>>>>> --- /dev/null
>>>>> +++ 
>>>>> b/tests/vulkan/shaders/block-member-layout-location.vk_shader_test
>>>>> @@ -0,0 +1,69 @@
>>>>> +# Test that interface block members are correctly matched by 
>>>>> explicit
>>>>> +# location, when we assign explicit location for all members of a
>>>>> +# block, and unsorted.
>>>>> +
>>>>> +[vertex shader]
>>>>> +#version 440
>>>>> +
>>>>> +layout(location = 0) in vec4 piglit_vertex;
>>>>> +
>>>>> +out block {
>>>>> +        layout(location = 2) vec4 c;
>>>>> +        layout(location = 3) vec4 d;
>>>>> +        layout(location = 0) vec4 a;
>>>>> +        layout(location = 1) vec4 b;
>>>>> +} name;
>>>>> +
>>>>> +void main()
>>>>> +{
>>>>> +        name.a = vec4(1.0, 0.0, 0.0, 1.0);
>>>>> +        name.b = vec4(0.0, 1.0, 0.0, 1.0);
>>>>> +        name.c = vec4(0.0, 0.0, 1.0, 1.0);
>>>>> +        name.d = vec4(1.0, 1.0, 1.0, 1.0);
>>>>> +
>>>>> +        gl_Position = piglit_vertex;
>>>>> +}
>>>>> +
>>>>> +[fragment shader]
>>>>> +#version 440
>>>>> +
>>>>> +layout(location = 0) in vec4 a;
>>>>> +layout(location = 1) in vec4 b;
>>>>> +layout(location = 2) in vec4 c;
>>>>> +layout(location = 3) in vec4 d;
>>>>> +
>>>>> +layout(std140, push_constant) uniform block {
>>>>> +        int i;
>>>>> +};
>>>>> +
>>>>> +layout(location = 0) out vec4 color;
>>>>> +
>>>>> +void main()
>>>>> +{
>>>>> +        if (i == 0) {
>>>>> +                color = a;
>>>>> +        } else if (i == 1) {
>>>>> +                color = b;
>>>>> +        } else if (i == 2) {
>>>>> +                color = c;
>>>>> +        } else if (i == 3) {
>>>>> +                color = d;
>>>>> +        }
>>>>> +}
>>>>> +
>>>>> +[test]
>>>>> +uniform int 0 0
>>>>> +draw rect -1 -1 1 1
>>>>> +relative probe rect rgb (0.0, 0.0, 0.5, 0.5) (1.0, 0.0, 0.0)
>>>>> +
>>>>> +uniform int 0 1
>>>>> +draw rect 0 -1 1 1
>>>>> +relative probe rect rgb (0.5, 0.0, 0.5, 0.5) (0.0, 1.0, 0.0)
>>>>> +
>>>>> +uniform int 0 2
>>>>> +draw rect -1 0 1 1
>>>>> +relative probe rect rgb (0.0, 0.5, 0.5, 0.5) (0.0, 0.0, 1.0)
>>>>> +
>>>>> +uniform int 0 3
>>>>> +draw rect 0 0 1 1
>>>>> +relative probe rect rgb (0.5, 0.5, 0.5, 0.5) (1.0, 1.0, 1.0)
>>>>> diff --git 
>>>>> a/tests/vulkan/shaders/block-mixed-layout-location.vk_shader_test 
>>>>> b/tests/vulkan/shaders/block-mixed-layout-location.vk_shader_test
>>>>> new file mode 100644
>>>>> index 000000000..750dd2b29
>>>>> --- /dev/null
>>>>> +++ b/tests/vulkan/shaders/block-mixed-layout-location.vk_shader_test
>>>>> @@ -0,0 +1,126 @@
>>>>> +# Test that interface block members are correctly matched by 
>>>>> explicit
>>>>> +# location. In this case the main variable has a location value, but
>>>>> +# there is also one member with an explicit value.
>>>>> +#
>>>>> +# Note that we include the spirv assembly. This is because 
>>>>> although we
>>>>> +# used a GLSL shader as reference, we tweak and add comments over 
>>>>> 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 "c"
>>>>> +               OpMemberName %block 1 "d"
>>>>> +               OpMemberName %block 2 "a"
>>>>> +               OpMemberName %block 3 "b"
>>>>> +               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"
>>>>> +; The block is given a location of 2. The first two members don’t
>>>>> +; have a location so they should be derived from the block location.
>>>>> +               OpDecorate %name Location 2
>>>>> +; The 3rd member has an explicit location. The 4th member should be
>>>>> +; derived from this.
>>>>> +               OpMemberDecorate %block 2 Location 0
>>>>> +               OpDecorate %block Block
>>>>> +               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_2 = OpConstant %int 2
>>>>> +    %float_1 = OpConstant %float 1
>>>>> +    %float_0 = OpConstant %float 0
>>>>> +         %15 = OpConstantComposite %v4float %float_1 %float_0 
>>>>> %float_0 %float_1
>>>>> +%_ptr_Output_v4float = OpTypePointer Output %v4float
>>>>> +      %int_3 = OpConstant %int 3
>>>>> +         %19 = OpConstantComposite %v4float %float_0 %float_1 
>>>>> %float_0 %float_1
>>>>> +      %int_0 = OpConstant %int 0
>>>>> +         %22 = OpConstantComposite %v4float %float_0 %float_0 
>>>>> %float_1 %float_1
>>>>> +      %int_1 = OpConstant %int 1
>>>>> +         %25 = OpConstantComposite %v4float %float_1 %float_1 
>>>>> %float_1 %float_1
>>>>> +       %uint = OpTypeInt 32 0
>>>>> +     %uint_1 = OpConstant %uint 1
>>>>> +%_arr_float_uint_1 = OpTypeArray %float %uint_1
>>>>> +%gl_PerVertex = OpTypeStruct %v4float %float %_arr_float_uint_1
>>>>> +%_ptr_Output_gl_PerVertex = OpTypePointer Output %gl_PerVertex
>>>>> +          %_ = OpVariable %_ptr_Output_gl_PerVertex Output
>>>>> +%_ptr_Input_v4float = OpTypePointer Input %v4float
>>>>> +%piglit_vertex = OpVariable %_ptr_Input_v4float Input
>>>>> +       %main = OpFunction %void None %3
>>>>> +          %5 = OpLabel
>>>>> +         %17 = OpAccessChain %_ptr_Output_v4float %name %int_2
>>>>> +               OpStore %17 %15
>>>>> +         %20 = OpAccessChain %_ptr_Output_v4float %name %int_3
>>>>> +               OpStore %20 %19
>>>>> +         %23 = OpAccessChain %_ptr_Output_v4float %name %int_0
>>>>> +               OpStore %23 %22
>>>>> +         %26 = OpAccessChain %_ptr_Output_v4float %name %int_1
>>>>> +               OpStore %26 %25
>>>>> +         %35 = OpLoad %v4float %piglit_vertex
>>>>> +         %36 = OpAccessChain %_ptr_Output_v4float %_ %int_0
>>>>> +               OpStore %36 %35
>>>>> +               OpReturn
>>>>> +               OpFunctionEnd
>>>>> +
>>>>> +[fragment shader]
>>>>> +#version 440
>>>>> +
>>>>> +layout(location = 0) in vec4 a;
>>>>> +layout(location = 1) in vec4 b;
>>>>> +layout(location = 2) in vec4 c;
>>>>> +layout(location = 3) in vec4 d;
>>>>> +
>>>>> +layout(std140, push_constant) uniform block {
>>>>> +        int i;
>>>>> +};
>>>>> +
>>>>> +layout(location = 0) out vec4 color;
>>>>> +
>>>>> +void main()
>>>>> +{
>>>>> +        if (i == 0) {
>>>>> +                color = a;
>>>>> +        } else if (i == 1) {
>>>>> +                color = b;
>>>>> +        } else if (i == 2) {
>>>>> +                color = c;
>>>>> +        } else if (i == 3) {
>>>>> +                color = d;
>>>>> +        }
>>>>> +}
>>>>> +
>>>>> +[test]
>>>>> +uniform int 0 0
>>>>> +draw rect -1 -1 1 1
>>>>> +relative probe rect rgb (0.0, 0.0, 0.5, 0.5) (1.0, 0.0, 0.0)
>>>>> +
>>>>> +uniform int 0 1
>>>>> +draw rect 0 -1 1 1
>>>>> +relative probe rect rgb (0.5, 0.0, 0.5, 0.5) (0.0, 1.0, 0.0)
>>>>> +
>>>>> +uniform int 0 2
>>>>> +draw rect -1 0 1 1
>>>>> +relative probe rect rgb (0.0, 0.5, 0.5, 0.5) (0.0, 0.0, 1.0)
>>>>> +
>>>>> +uniform int 0 3
>>>>> +draw rect 0 0 1 1
>>>>> +relative probe rect rgb (0.5, 0.5, 0.5, 0.5) (1.0, 1.0, 1.0)
>>>>
>>>>
>>>>
>>>
>>
>>
>