Start preparing for TypeJointMatrixINTEL switch #1935
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The patch adds TypeJointMatrixINTELv2 which maps to new type OpCode 6184. Under new OpCode matrix type no longer has Layout parameter. The patch also moved 'scope' to optional matrix muladd instruction. The changes are done only in the consumer part to prepare the switch and make E2E switch backward compatible by preparing consumers ahead of time. Unfortunately there is no way to add a test foe this unless it's binary test, but it seems to be a bit unsafe to add this, so the patch was tested localy. Spec change: intel/llvm#8175 Signed-off-by: Sidorov, Dmitry <[email protected]>
|
@asudarsa @maksimsab @jgstarIntel please take a look |
maksimsab
suggested changes
Apr 3, 2023
lib/SPIRV/libSPIRV/SPIRVType.cpp
Outdated
| SPIRVModule *M, SPIRVId TheId, SPIRVType *CompType, | ||
| std::vector<SPIRVValue *> Args) | ||
| : SPIRVType(M, FixedWC + Args.size(), internal::OpTypeJointMatrixINTEL, | ||
| TheId), CompType(CompType), Args(Args) {} |
There was a problem hiding this comment.
I would suggest Args(std::move(Args)) here. Although this is a copy of previous constructor, I wouldn't copy small flaws.
There was a problem hiding this comment.
Thanks! We probably should also scan other places in the translator to embrace this optimization
| return Args[2]; | ||
| return nullptr; | ||
| } | ||
| SPIRVValue *getScope() const { |
There was a problem hiding this comment.
Could you please separate this methods by blank lines?
MrSidims
commented
Apr 3, 2023
Comment on lines
+3333
to
+3336
| _SPIRV_OP(JointMatrixMad, true, 6, true) | ||
| _SPIRV_OP(JointMatrixSUMad, true, 6, true) | ||
| _SPIRV_OP(JointMatrixUSMad, true, 6, true) | ||
| _SPIRV_OP(JointMatrixUUMad, true, 6, true) |
There was a problem hiding this comment.
@maksimsab move of scope parameter happens here (see the second 'true').
Note, that 'scope' according to old spec is mandatory parameter and per new spec is removed at all. So to balance it here making both representations translatable we have to add is as optional for a while.
Signed-off-by: Sidorov, Dmitry <[email protected]>
Signed-off-by: Sidorov, Dmitry <[email protected]>
fda0
pushed a commit
to fda0/SPIRV-LLVM-Translator
that referenced
this pull request
May 22, 2025
…onosGroup#1935) The patch adds TypeJointMatrixINTELv2 which maps to new type OpCode 6184. Under new OpCode matrix type no longer has Layout parameter. The patch also moved 'scope' to optional matrix muladd instruction. The changes are done only in the consumer part to prepare the switch and make E2E switch backward compatible by preparing consumers ahead of time. Unfortunately there is no way to add a test foe this unless it's binary test, but it seems to be a bit unsafe to add this, so the patch was tested locally. Spec change: intel/llvm#8175 Signed-off-by: Sidorov, Dmitry <[email protected]> (cherry picked from commit a6fcade)
bokrzesi
added a commit
to bokrzesi/SPIRV-LLVM-Translator
that referenced
this pull request
Jun 10, 2025
…tch (KhronosGroup#1935)" This reverts commit e781a91.
bokrzesi
added a commit
to bokrzesi/SPIRV-LLVM-Translator
that referenced
this pull request
Jun 10, 2025
…tch (KhronosGroup#1935)" This reverts commit e781a91.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The patch adds TypeJointMatrixINTELv2 which maps to new type OpCode
6184. Under new OpCode matrix type no longer has Layout parameter. The patch also moved 'scope' to optional matrix muladd instruction.
The changes are done only in the consumer part to prepare the switch and make E2E switch backward compatible by preparing consumers ahead of time.
Unfortunately there is no way to add a test foe this unless it's binary test, but it seems to be a bit unsafe to add this, so the patch was tested localy.
Spec change:
intel/llvm#8175