[Piglit] [PATCH] vulkan: Add some tests for block member decorations
Lionel Landwerlin
lionel.g.landwerlin at intel.com
Thu Jan 24 15:59:28 UTC 2019
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 at 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 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