-
Notifications
You must be signed in to change notification settings - Fork 808
[SM6.10][LinAlg] Impl MatrixRef, CreateMatrix builtins #7883
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
| @@ -0,0 +1,8 @@ | |||
| // RUN: %dxc -T cs_6_9 -E main %s -verify | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pretty sure I also want a .ll test but I don't know the magic incantation to generate the IR upto but right before the dxilgen pass
damyanp
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've read through this and didn't spot any obvious typos.
Do we need to apply the same "experimental DXIL" stuff to this that we're talking about for other SM 6.10 features?
| LICOMPTYPE_VK_BUFFER_POINTER = 55, | ||
| LICOMPTYPE_COUNT = 56 | ||
| #else | ||
| LICOMPTYPE_COUNT = 54 | ||
| LICOMPTYPE_COUNT = 55 | ||
| #endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it matter that these values have changed?
I've no reason to think it does, just checking.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm assuming no but maybe someone else can weigh in. The ifdef basically forces them to change though since the last number is only incremented for SPIRV. Beyond that this seems to be an internal enum so changing it shouldn't be observable to end users.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are mostly internal details, but the built-in intrinsic extension mechanism relies on this header (and this enum) as well. That's currently only used by Xbox backend, and wouldn't be impacted by these changes unless updated to depend on them, and even then, using the latest header would keep it in sync.
Ideally, I think we should get rid of the ifdef here (so LICOMPTYPE_VK_BUFFER_POINTER is always part of the enumeration), and update the corresponding table g_LegalIntrinsicCompTypes in SemaHLSL.cpp. But that should be a separate change.
Yep |
Fixes #7846
Implments:
__builtin_la_MatrixRef__builtin_la_CreateMatrix()%dx.types.MatrixRef@dx.op.createMatrix(i32 312)Note: The DXIL op codes will change in response to microsoft/hlsl-specs#698