arch-vega: Template MFMA instructions #1113
Conversation
|
Hi @mjkpolo, None of the commits in this pull request contains a Change-ID, which we require for any changes made to gem5. To automatically insert one, run the following: Then, amend the commit with |
There was a problem hiding this comment.
In addition to the bureaucratic issue @ivanaamit highlighted, a few others things.
| getOperandSize(int opIdx) override | ||
| { | ||
| switch (opIdx) { | ||
| case 0: // src0 "A" |
There was a problem hiding this comment.
might be useful to have slightly more detailed comments about what A, B, and C mean
| // src2 is a VGPR/AccGPR, but it might also be constant. In order to | ||
| // use operator[] and handle constants, check for VGPR here and set | ||
| // a delta for each of the src2 GPRs. | ||
|
|
There was a problem hiding this comment.
extra line of space, should be removed
| } // execute | ||
| // --- Inst_VOP3P_MAI__V_MFMA_F64_16X16X4F64 class methods --- | ||
|
|
||
| Inst_VOP3P_MAI__V_MFMA_F64_16X16X4F64:: |
There was a problem hiding this comment.
@abmerop should we be moving vop3p_mai from this file to others like seems to be happening here?
There was a problem hiding this comment.
My opinion is we should make a new vop3p_mai.hh header with the template and aliases. The instructions.hh already has 45k lines of code
There was a problem hiding this comment.
Right, this is exactly what I meant.
There was a problem hiding this comment.
There are a few style errors which should have been caught by pre-commit. I'd also recommend making a new branch on your repo. I'd like to add some commits to this, but I don't want to put them on top of your develop branch as it would be too easy to accidently git push -f over them
| typename std::aligned_storage<sizeof(T1), alignof(T1)>::type _src0[gprs_a]; | ||
| typename std::aligned_storage<sizeof(T1), alignof(T1)>::type _src1[gprs_b]; | ||
| typename std::aligned_storage<sizeof(T1), alignof(T1)>::type _src2[gprs_c_d]; | ||
| typename std::aligned_storage<sizeof(T2), alignof(T2)>::type _vdst[gprs_c_d]; |
There was a problem hiding this comment.
I've recently discovered this is going to be deprecated in C++23: https://stackoverflow.com/questions/71828288/why-is-stdaligned-storage-to-be-deprecated-in-c23-and-what-to-use-instead
I've already made a change to fix this myself.
@mjkpolo let me know if you can't figure out how to install pre-commit and the hooks and need help. Likewise let @abmerop and I know once you've made a separate branch. |
templated
added support for
formula for gprs needed
formulas for register layouts and lanes used in computation