Skip to content

GLSL: implement SPV_NV_cooperative_vector#2489

Closed
theHamsta wants to merge 1 commit into
KhronosGroup:mainfrom
theHamsta:coopvec-nv
Closed

GLSL: implement SPV_NV_cooperative_vector#2489
theHamsta wants to merge 1 commit into
KhronosGroup:mainfrom
theHamsta:coopvec-nv

Conversation

@theHamsta

Copy link
Copy Markdown
Contributor

This is on top of #2488 since replicated composites often occur in the initialization of cooperative vectors. This should stay in draft status until all issues with the previous PR are resolved.

I tried to roughly follow the implementation of SPV_EXT_cooperative_matrix. Though I might not have tracked all dependencies of the instructions correctly. This is my first time working with spirv-cross, so some concepts are still unfamiliar to me. I tested the implementation on shaders in https://github.com/jeffbolznv/vk_cooperative_vector_perf

There is a roughly equivalent proposal for HLSL to which this SPV extension could be mapped: https://github.com/microsoft/hlsl-specs/blob/main/proposals/0026-hlsl-long-vector-type.md

@theHamsta

Copy link
Copy Markdown
Contributor Author

I also prepared a draft for https://registry.khronos.org/vulkan/specs/latest/man/html/VK_NV_cooperative_matrix2.html before working on coopvec-nv. But I discovered that interactions of VK_NV_cooperative_matrix2 with function calling might make it more complicated than coopvec-nv

@theHamsta theHamsta force-pushed the coopvec-nv branch 8 times, most recently from af644be to aedf69e Compare June 17, 2025 15:30
Comment thread spirv_common.hpp
uint32_t component_type_id = 0;
uint32_t component_count_id = 0;
} coopVecNv;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Should this be merged to one union or stay multiple consecutive struct fields?

Comment thread spirv_glsl.cpp
auto matrix_stride_id = length >= 6 ? ops[6] : 0;
//void coopVecOuterProductAccumulateNV(const coopvecNV<T, M> v1, const coopvecNV<T, N> v2,
//T[] buf, uint offset, uint stride,
//int matrixLayout, int matrixInterpretation);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Should I remove the verbatim parameters from SPIRV and GLSL spec?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Those comments should just be removed, yes.

Comment thread spirv_parser.cpp
// CoopVec-Nv can be used with integer operations like SMax where
// where spirv-opt does explicit checks on integer bitwidth
auto component_type = get<SPIRType>(type.coopVecNv.component_type_id);
type.width = component_type.width;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This could also be handled on the respective integer operations where it could defer to the bitwidth of the component type instead of the coop-vec

@theHamsta theHamsta marked this pull request as ready for review June 17, 2025 15:42
@theHamsta theHamsta changed the title Draft: GLSL: implement SPV_NV_cooperative_vector GLSL: implement SPV_NV_cooperative_vector Jun 17, 2025
https://github.khronos.org/SPIRV-Registry/extensions/NV/SPV_NV_cooperative_vector.html
The implementation tries to follow the code for SPV_EXT_cooperative_matrix.

The extension could be mapped in a follow-up to the following HLSL
proposal https://github.com/microsoft/hlsl-specs/blob/main/proposals/0026-hlsl-long-vector-type.md
Comment thread spirv_glsl.cpp
EXTRA_SUB_EXPRESSION_TYPE_AUX = 0x20000000
};

const std::vector<std::tuple<uint32_t, const char *>> COOPVEC_COMPONENT_TYPE_NAMES = {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This should just be a static const array of POD struct inside the function that uses it.
Having a global const of non-POD types requires global initialization which really should be avoided if we can help it.

@HansKristian-Work HansKristian-Work left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'll just pull this in locally and fix what I need to fix.

@HansKristian-Work HansKristian-Work mentioned this pull request Jul 28, 2025
@HansKristian-Work

Copy link
Copy Markdown
Contributor

Merged.

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.

2 participants