[SM6.10] Add dx.types.MatrixRef to DXIL #8027
Conversation
|
✅ With the latest revision this PR passed the Python code formatter. |
bob80905
left a comment
bob80905
left a comment
There was a problem hiding this comment.
Understood that this type can't be emitted yet, though I wonder if there are IR-level validation tests that can test the new type.
| // Operand indexes | ||
| enum OperandIdx { | ||
| arg_matrixRef = 1, | ||
| arg_matrix = 1, |
There was a problem hiding this comment.
I understand you've changed the underlying type from i32 to matrixref, a new dxil type.
But why should these names change from matrixref to matrix?
Wouldn't it be better to leave the function/variable names as is, they are accurate no?
There was a problem hiding this comment.
No real technical reason, just a style sense/gut feeling I guess?
the signature @dx.op.createMatrix(MatrixRef matrix) looks nicer to me than @dx.op.createMatrix(MatrixRef matrixRef) must I'm happy to revert if anyone strongly disagrees
Yeah, I was thinking about this too. I could hand write some DXIL IR with the type in it but since that's the final output what do I do after that? Maybe I could pass the IR into the DXIL Validator but that's not actually going to test any of the code changes in this PR which doesn't feel all that useful either especially when the very next commit has all the tests needed to hit this functionality properly. If its useful to see those tests early they are available at c689b9a Honestly I still see this PR as another "pre-reserve stuff for the actual work to come" type of PR but its getting to the fuzzy parts and I'd fully understand if someone disagreed with that claim |
Introduces
dx.types.MatrixRefas a legal DXIL type without using it anywhere. This doesn't expose the type to higher levels yet so it's not possible to write a meaningful test for the PR. The very next PR in the stack will have tests that emit the type and test this code path.The DXIL ops that operate over the
MatrixReftype have been updated to use the new typeinclude/dxc/DXIL/DxilInstructions.his entirely generated code and all thecaseupdates tolib/DXIL/DxilOperations.cppis generated code.