feat: Added operation fields for Zicfiss instructions. #1009
Conversation
sudo-apt-abdullah
requested review from
ThinkOpenly and
dhower-qc
as code owners
ThinkOpenly
left a comment
ThinkOpenly
left a comment
There was a problem hiding this comment.
One minor request that actually has nothing to do with your specific changes. Otherwise, looks good. :-)
There was a problem hiding this comment.
could you add:
base: 64
to this file?
There was a problem hiding this comment.
Sure, I've added this in ssamoswap.d.yaml file. One more question, am I supposed to add new files for C.SSPOPCHK and C.SSPUSH under the Zicfiss extension?
There was a problem hiding this comment.
These are present as sspopchk.x1.yaml and similar, although they is not really correct. I have PR #872 that needs attention which fixes these, but I haven't finished getting it to pass CI yet. (And haven't had a chance to do so recently.) You can proceed as you already have.
There was a problem hiding this comment.
In that PR, I can see that you merged sspopchk.x1.yaml & sspopchk.x5.yaml into single file sspopchk.yaml and sspush.x1.yaml & sspush.x5.yaml into file sspush.yaml, but can't see 16 bit version of sspush and sspopchk. Do we need to create separate files for that or I'm missing something?
You can proceed as you already have.
You mean I should pick the changes from there, right?
There was a problem hiding this comment.
Ah, I conflated the two. Yes, we do need C.SSPOPCHK and C.SSPUSH. Note that they're a little weird because their encoding overrides some C.MOP encodings.
There was a problem hiding this comment.
Ok, I'll give it some time to understand and then will try to write those files as well.
|
@ThinkOpenly, please have a look at your convinience, thanks! |
|
There are some CI failures. Are you able to look at those? |
2 participants
Closes #995 which depends on #944.