[Piglit] [PATCH] vulkan: Add some tests for block member decorations

Lionel Landwerlin lionel.g.landwerlin at intel.com
Thu Jan 24 15:28:38 UTC 2019


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



More information about the Piglit mailing list