[SM6.10] Add built-in type for LinAlg Matrix handle #8090
Conversation
Adds a built-in type `__builtin_LinAlg_Matrix` that will be used in LinAlg Matrix implementation.
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
damyanp
left a comment
damyanp
left a comment
There was a problem hiding this comment.
LGTM, but you probably want an approval from someone more knowledgeable.
|
About to start a deeper review but wanted to get this out there as quickly as possible. How do you feel about Looking back on chat history it looks like I +1'd My thoughts on why are below:
If you are okay with the change I'm more than happy to do the chore work of renaming everything |
| Out << "$ui8_4pk@"; | ||
| break; | ||
| case BuiltinType::LinAlgMatrix: | ||
| Out << "$linalg_matrix@"; |
There was a problem hiding this comment.
As before, don't we need to capture matrix properties into mangling?
There was a problem hiding this comment.
This is mangling of just the base built-in type, which can be spell out in the code, although the __builtin prefix should discourage people from doing that. The attributes are added in a separate PR.
| case BuiltinType::OCLSampler: ID = PREDEF_TYPE_SAMPLER_ID; break; | ||
| case BuiltinType::OCLEvent: ID = PREDEF_TYPE_EVENT_ID; break; | ||
| case BuiltinType::LinAlgMatrix: | ||
| ID = PREDEF_TYPE_LINALG_MATRIX_ID; |
There was a problem hiding this comment.
Fine, but we never supported AST serialization in this code base, and there are many missing pieces already. That's why we never added the other HLSL types here.
V-FEXrt
left a comment
V-FEXrt
left a comment
There was a problem hiding this comment.
LGTM*, like Damyan I'm going to leave someone else to approve since this shape of adding a type is very new looking to me :)
*of course I'd like to discuss the time name as mentioned above
I agree, |
4 participants
Adds a built-in type
__builtin_LinAlgMatrixthat will be used in LinAlg Matrix implementation for SM 6.10.Closes #8121