Add support for Zhinxmin extension #753
Conversation
jordancarlin
force-pushed
the
zhinxmin
branch
2 times, most recently
from
6f90bb9 to
2374c9a
Compare
| @@ -7,6 +7,7 @@ | |||
| /*=======================================================================================*/ | |||
|
|
|||
| function clause currentlyEnabled(Ext_Zhinx) = hartSupports(Ext_Zhinx) & currentlyEnabled(Ext_Zfinx) | |||
| function clause currentlyEnabled(Ext_Zhinxmin) = (hartSupports(Ext_Zhinxmin) & currentlyEnabled(Ext_Zfinx))| currentlyEnabled(Ext_Zhinx) | |||
There was a problem hiding this comment.
| function clause currentlyEnabled(Ext_Zhinxmin) = (hartSupports(Ext_Zhinxmin) & currentlyEnabled(Ext_Zfinx))| currentlyEnabled(Ext_Zhinx) | |
| function clause currentlyEnabled(Ext_Zhinxmin) = (hartSupports(Ext_Zhinxmin) & currentlyEnabled(Ext_Zfinx)) |
I looked through the specification but couldn’t find anything indicating that Zhinx enables Zhinxmin.
There was a problem hiding this comment.
This is the same approach we took for Zfh and Zfhmin. Zhinx includes everything in Zhinxmin, so we gate the relevant instructions on Zhinxmin and enable Zhinxmin if Zhinx is enabled.
There was a problem hiding this comment.
Hmm, I don't know if this is a good approach to turn on extensions silently. Whats the advantage of enabling Zhinxmin automatically when Zhinx is active, given that Zhinx already includes all those instructions? Wouldn't it be cleaner to keep each extension's enabled status independent?
This may be confusing for the reader.
There was a problem hiding this comment.
There's been a lot of discussion surrounding these extensions where one implies another. See riscv/riscv-isa-manual#1121 and riscv/riscv-isa-manual#869 for a discussion about M/Zmmul. Right now, we do this for a number of other extensions (Zfhmin, Zmmul, possibly a few others), so I'd be in favor of keeping it consistent in this PR and we can open an issue to potentially change all of them later.
There was a problem hiding this comment.
Yeah I agree, lets keep it and handle this in a separate PR.
There was a problem hiding this comment.
I just noticed that the RISC-V GCC extension table explicitly states when Vector Crypto extensions are expanded https://gcc.gnu.org/onlinedocs/gcc/RISC-V-Options.html
For example like:
Vector NIST Algorithm Suite with GCM extension, ‘zvkng’ will expand to ‘zvkn’ and ‘zvkg’.
Maybe this is an option, and we could do the same in the README for Zhinxmin etc.
There was a problem hiding this comment.
I think the difference is that Zvkng is defined to be a shorthand for a group of several other extensions. It doesn’t define any instructions itself. This is different than the Zhinx/Zhinxmin relationship.
There was a problem hiding this comment.
@nadime15 Are you ok with merging this one as it is and then addressing this globally later? Adding it to the README could work, but there are several other extensions this applies to, and it probably makes sense to do them all together.
Timmmm
added
the
will be merged
No description provided.