GLSL: Implement SPV_NV_cooperative_matrix2, SPV_NV_tensor_addressing. #2529
Conversation
|
Is there something I need to review here? It's marked as draft and assuming it's WIP until undrafted. |
|
Callback functions are complicated. Should I split them off to a separate PR? Do you agree with the overall approach on how to handle them? I basically stopped at WIP because I wanted to know how you would handle the "all parameters must be |
theHamsta
force-pushed
the
coopmat2
branch
2 times, most recently
from
cf931c6 to
96ecacb
Compare
HansKristian-Work
left a comment
HansKristian-Work
left a comment
There was a problem hiding this comment.
call callback functions require const in for all parameters which is a unusual requirement. I'm not sure yet how to best implement this (flag on the parameters, flag on the functions)
What do you mean here? Do you literally have to annotate the parameters as void my_lambda(const int a), or do you mean just not using in/out/inout?
spirv_common.hpp
Outdated
| #include "spirv_cross_containers.hpp" | ||
| #include "spirv_cross_error_handling.hpp" | ||
| #include <functional> | ||
| #include <array> |
There was a problem hiding this comment.
isn't really used in the codebase.
spirv_common.hpp
Outdated
| AccelerationStructure, | ||
| RayQuery, | ||
| CoopVecNV, | ||
| TensorLayoutNv, |
There was a problem hiding this comment.
NV for consistency.
spirv_common.hpp
Outdated
| // read and write counts as access to the function arguments | ||
| // is not local to the function in question. | ||
| bool alias_global_variable; | ||
| spv::StorageClass storage; |
spirv_parser.cpp
Outdated
| } | ||
| if (length - 3 > type.ext.tensorViewNv.dim_ids.size() || dims.scalar() > type.ext.tensorViewNv.dim_ids.size()) | ||
| { | ||
| SPIRV_CROSS_THROW("Can't have more than 5 dimensions for tensor view!"); |
There was a problem hiding this comment.
spec limitation or SPIRV-Cross limitation?
There was a problem hiding this comment.
rephrased the message to make clear that this is a spec limitation
spirv_common.hpp
Outdated
| uint32_t dim_id; | ||
| uint32_t has_dimensions_id; | ||
| // From spec on Dims: "The value must be greater than zero and less than or equal to 5." | ||
| std::array<int32_t, 5> dim_ids; |
There was a problem hiding this comment.
Non-POT inside union will lead to subtle UB.
There was a problem hiding this comment.
replaced with plain array and top-level constant for array length
spirv_glsl.cpp
Outdated
|
|
||
| auto called_func = maybe_get_called_function(instruction); | ||
| if(called_func) { | ||
| for(auto& par: called_func->arguments) { |
There was a problem hiding this comment.
reference and pointer ties right.
yes, they need to be explicitly annotated with |
|
Sorry for not getting back to this sooner. For the "const in" part, I suppose a simple bool in SPIRFunction is sufficient? A simple "used as lambda function" is probably fine. |
theHamsta
force-pushed
the
coopmat2
branch
5 times, most recently
from
ca6c40f to
4c84008
Compare
I moved the info for the special usage from the SPIRParameter to SPIRFunction in the last commit. |
theHamsta
force-pushed
the
coopmat2
branch
4 times, most recently
from
9b1efe3 to
2df866a
Compare
theHamsta
force-pushed
the
coopmat2
branch
2 times, most recently
from
9509800 to
52d8cc7
Compare
2 participants
This adds support for more operations SPV_NV_cooperative_matrix2 and SPV_NV_tensor_addressing
Marking as draft because:
intspec constant (instead of uint) for createTensorLayoutNV leads to null pointer dereference glslang#4011 makes a workaround in this PR necessary.createTensorLayoutNVexpects auintargument while all names constants for the arguments areint. When casting a spec constant tointand passing it tocreateTensorLayoutNVone would trigger Using aintspec constant (instead of uint) for createTensorLayoutNV leads to null pointer dereference glslang#4011OpFunctionCallwas assumed to be the only op code being able to call functionshandler.begin_function_scope/handler.end_function_scopewould need to be refactored to not assumeOpFunctionCall(currently just skipped since implementers currently still assumeOpFunctionCallespecially the operand index where they would find the called function)const infor all parameters which is a unusual requirement. I'm not sure yet how to best implement this (flag on the parameters, flag on the functions)