Skip to content

Generate vector constructions more efficiently when sizes match#3628

Merged
arcady-lunarg merged 2 commits intoKhronosGroup:mainfrom
arcady-lunarg:vector-copy-codegen
Jun 24, 2024
Merged

Generate vector constructions more efficiently when sizes match#3628
arcady-lunarg merged 2 commits intoKhronosGroup:mainfrom
arcady-lunarg:vector-copy-codegen

Conversation

@arcady-lunarg
Copy link
Copy Markdown
Contributor

When two vectors are the same size, there is no need to extract the components and construct a new vector.

When two vectors are the same size, there is no need to extract the
components and construct a new vector.
A type can't have a negative number of constituents, components, rows,
or columns. Therefore, the utility functions to query said information
on types and values should return unsigned int rather than int.
@ravi688
Copy link
Copy Markdown
Contributor

ravi688 commented Jun 23, 2024

This change leads to an assertion failure for the following shader:

#version 460

#extension GL_EXT_shader_8bit_storage : require
#extension GL_EXT_shader_16bit_storage : require

layout(binding = 1 ) uniform _16bit_storage
{
        int16_t i16;
	i16vec4 i16v4;
};

// This is read back and checked on the CPU side to verify the converions
layout(binding = 2 ) writeonly buffer ConversionOutBuffer
{
	int8_t i8;
	i8vec4 i8v4;
} cob;

out vec4 fcolor;

void main()
{
        // Conversions
        {
                cob.i8   = int8_t(i16);
		cob.i8v4 = i8vec4(i16v4);
        }

        bool RED = true;
        bool GREEN = false;

        fcolor = vec4( (RED) ? 1.0f : 0.0f,
                                   (GREEN) ? 1.0f : 0.0f,
                                   0.0f, 1.0f);
}

Assertion:
glslang: /home/ravi/OpenSource/glslang/SPIRV/SpvBuilder.cpp:3429: spv::Id spv::Builder::createConstructor(spv::Decoration, const std::vector&, spv::Id): Assertion `resultTypeId == getTypeId(sources[0])' failed.

ravi688 added a commit to ravi688/glslang that referenced this pull request Jun 23, 2024
…esponding 32-bit types first

 - this fixes KhronosGroup#3607
 - and this also fixes assertion failure in the PR KhronosGroup#3628

 - this change emits appropriate Op{S|U}Convert instructions instead of OpCompositeExtract followed by OpCompositeConstruct
   for 8/16-bit integer types.
@ravi688 ravi688 mentioned this pull request Jun 23, 2024
@ravi688
Copy link
Copy Markdown
Contributor

ravi688 commented Jun 23, 2024

The assertion failure is fixed in PR #3631.

Copy link
Copy Markdown
Contributor

@jeremy-lunarg jeremy-lunarg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. We should follow up on #3631 after this lands.

@arcady-lunarg
Copy link
Copy Markdown
Contributor Author

The assertion is in place of generating invalid SPIR-V so I wouldn't count it as a regression.

@arcady-lunarg arcady-lunarg merged commit 0c15a28 into KhronosGroup:main Jun 24, 2024
@arcady-lunarg arcady-lunarg deleted the vector-copy-codegen branch October 14, 2024 17:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants