[0029][0031]In linalg Mul/MulAdd functions, the memory backing the Matrix and Bias buffers must be read-only - #507
Conversation
Anupama Chandrasekhar (anupamachandra)
changed the title
Gregory Roth (pow2clk)
left a comment
Gregory Roth (pow2clk)
left a comment
There was a problem hiding this comment.
I don't have anything against this apart from the minor quibble about example code comments.
When I took an initial run at this, I had significantly more changes to the API itself as a result of disallowing RWMatrixRefs which results in much better error messsages when RW types are used: pow2clk@ea67c02 Sorry I didn't link this to you sooner.
| matrix is stored in memory, while the vector comes from a variable. The Mul | ||
| function expects the memory backing the matrix object to be read-only | ||
| (for efficient loads and hardware optimizations) and, therefore, the matrix | ||
| must be of MatrixRef type (and not RWMatrixRef type). |
There was a problem hiding this comment.
I expected this to result in changing the interface of these functions so they take MatrixRef and not MatrixRefImpl.
| bias vector (loaded from memory) to the result. | ||
| bias vector (loaded from memory) to the result. Similar to the the matrix | ||
| operand, the memory backing the bias vector must be read-only, and therefore an | ||
| object of `VectorRef` type (and not `RWVectorRef` type). |
There was a problem hiding this comment.
As above except without the ability to specify RWVectorRef here, the type is not needed at all.
| ByteAddressBuffer inputMatrix0; // note read-only buffer | ||
| ByteAddressBuffer inputMatrix1; // note read-only buffer | ||
| ByteAddressBuffer biasVector0; // note read-only buffer | ||
| ByteAddressBuffer biasVector1; // note read-only buffer |
There was a problem hiding this comment.
I think one comment at the top of the block pointing out that they are all read-only buffers is sufficient.
There was a problem hiding this comment.
Honestly since ByteAddressBuffer is already a read only buffer I don't think any of these comments are needed. Its belaboring the point
| matrix is stored in memory, while the vector comes from a variable. The Mul | ||
| function expects the memory backing the matrix object to be read-only | ||
| (for efficient loads and hardware optimizations) and, therefore, the matrix | ||
| must be of MatrixRef type (and not RWMatrixRef type). |
| bias vector (loaded from memory) to the result. | ||
| bias vector (loaded from memory) to the result. Similar to the the matrix | ||
| operand, the memory backing the bias vector must be read-only, and therefore an | ||
| object of `VectorRef` type (and not `RWVectorRef` type). |
5 participants
Issue: microsoft/DirectXShaderCompiler#7490