openhwgroup / core-v-xif

RISC-V eXtension interface that provides a generalized framework suitable to implement custom coprocessors and ISA extensions
https://docs.openhwgroup.org/projects/openhw-group-core-v-xif/en
Other
53 stars 23 forks source link

Added dedicated register interface #82

Closed christian-herber-nxp closed 7 months ago

christian-herber-nxp commented 8 months ago
- This change is intended to be compatible to the previous definition
- It adds an option for processors with deeper pipelines to signal the register values in a later pipeline stage than the issue handshake
- This reduces the register file accesses
- This can improve performance by allowing register operands to be supplied out of order
christian-herber-nxp commented 8 months ago

@Gchauvon , @zarubaf: Will you want to add your review to this proposal?

christian-herber-nxp commented 8 months ago

@Silabs-ArjanB: Thanks for your rigorous review and input. If you are satisfied with the resolution, please mark the comment as resolved so we can focus on the open ones.

christian-herber-nxp commented 8 months ago

@jklockars do you have review comments on this PR?

jklockars commented 8 months ago

@jklockars do you have review comments on this PR?

The case where register values are provided later seems clear and OK, from what I can tell.

But I think I might be missing something regarding the other case. It is supposed to be compatible, right, so that any core and any coprocessor should be able to talk to eachother no matter what?

Is it supposed to be decided at build time which type of interface will be used? If so, shouldn't that be part of the interface specification itself as some kind of static control bit? Would both types of interfaces need to be supported by the all coprocessors or all cores? Both?

It seems complicated (at best) to allow for different behaviours at run-.time. Such as allowing one instruction to issue (late register values) and then then forcing the next one to halt waiting for values before being accepted (with the previous one receiving its values before the second one).

christian-herber-nxp commented 8 months ago

@jklockars do you have review comments on this PR?

The case where register values are provided later seems clear and OK, from what I can tell.

But I think I might be missing something regarding the other case. It is supposed to be compatible, right, so that any core and any coprocessor should be able to talk to eachother no matter what?

A coprocessor could have an elaboration time parameter to be configurable into either mode (maybe it's a good idea to add one). However, there is no way to force an implementer of the interface to support both.

Is it supposed to be decided at build time which type of interface will be used? If so, shouldn't that be part of the interface specification itself as some kind of static control bit? Would both types of interfaces need to be supported by the all coprocessors or all cores? Both?

It seems complicated (at best) to allow for different behaviours at run-.time. Such as allowing one instruction to issue (late register values) and then then forcing the next one to halt waiting for values before being accepted (with the previous one receiving its values before the second one).

Clearly this would not be a run-time parameter

christian-herber-nxp commented 8 months ago

I have added a parameter for the split between issue and register interface. From my point of view, this completes this PR. I have squashed the commits, and the PR should be ready for approval

christian-herber-nxp commented 7 months ago

Final call. This PR has been under review for a month now. If nothing else comes up, I would merge this on Wednesday. So please let me know if anything else needs to be changed.