riscv / riscv-p-spec

RISC-V Packed SIMD Extension
https://jira.riscv.org/browse/RVG-129
Creative Commons Attribution 4.0 International
141 stars 38 forks source link

Load-pair and store-pair instructions #152

Closed christian-herber-nxp closed 5 months ago

christian-herber-nxp commented 1 year ago

With a good definition of pair instructions in place with the reworked P proposal, it would make sense to add load-pair and store-pair instructions. This could be its own subextension, as it only really makes sense with the appropriate data interface.

See also this mailing list discussion: https://lists.riscv.org/g/tech-unprivileged/topic/97421962#443

load/store-pair for RV32 can reuse the double width load/stores defined for RV64 only, thus not requiring additional encoding space.

christian-herber-nxp commented 1 year ago

Some additional information: I discovered that @tovine has made a very similar proposal before: https://lists.riscv.org/g/tech-unprivileged/message/235, which went without response. The topic is highly relevant, and there are custom implementations of load/store-double already out in the wild (e.g. https://github.com/T-head-Semi/thead-extension-spec/tree/master/xtheadmempair)

As there seems to be a high demand for such a solution, there should be a standard way of doing it. Especially considering that the proposal does not consume additional encoding space.

tariqkurd-repo commented 1 year ago

I would say that double width load/store is useful, but they should be in their own small sub-extension within P, as other custom extensions (such as CHERI) have already reused the double width load/store encodings which would make them imcompatible if these became mandatory in P

christian-herber-nxp commented 1 year ago

agree that this should be isolated into an optional extension, as there will also be lower end implementations which don't have a sufficiently wide memory interface to perform those operations. Really, these should be considered an extension to 'I', but I do see P as the biggest benefactor of such an extension.

jhauser-us commented 1 year ago

The members of the Architecture Review Committee I've spoken with want to consider this only as a separate Zi extension, not as part of base P. I don't see the ARC having any real objections to such an extension, but there have been plenty of other matters occupying the committee's attention in the meantime. As far as I know, getting this extension through the system just needs some motivated RVIA members to pursue it formally and do most of the work.

tovine commented 1 year ago

Sounds good - I have the proposal draft right here! :) https://github.com/tovine/riscv-rv32zild/blob/master/rv32zild_proposal.adoc

I'd be happy to resume work on this one and potentially get it through as a fast track extension.

It could also potentially use the encodings intended for RV128 load/store to enable load/store pairs in RV64 as well if that's something people need, but it could also just be mentioned as potential "future work". Let me know what you think, I'm motivated! 😄

tariqkurd-repo commented 1 year ago

go for it @tovine! I'd also include the RV64 version, it would be a shame not to do all the work at once.

christian-herber-nxp commented 1 year ago

great news. +1 for including RV64 pair support in this extension, its a very low hanging fruit. RV64 implementers can still chose to not support this extension and go for macro op fusion instead. I think compressed can also be included in this extension, in a sense that the compressed version of load/store pair would become available when the load/store pair extension is present. You spec mentions the LSB of the register operands and potential use for it. that is something that i think would be better done in a subsequent extension, to keep things uncontentious and consistent with Zdinx, which states LSB!=0 is reserved.

tovine commented 1 year ago

Took a while to get back to this but I finally managed to push a new version with RV64 included (and a few other changes). Not done yet but at least it's a (re)start 😄 https://github.com/tovine/riscv-rv32zild/commit/017a9aebd24ef6ee4423fb6b82387a2997580ad9

christian-herber-nxp commented 5 months ago

As the load store pair extension is well under way, it is a good idea to close the issue here.