vproc / vicuna

RISC-V Zve32x Vector Coprocessor
Other
162 stars 47 forks source link

Fix for #100 #107

Closed moimfeld closed 2 years ago

moimfeld commented 2 years ago

To solve #100 I added a signal to the vproc_core module called source_xreg_valid. The signal is only valid when all scalar source operands for an instruction are valid. source_xreg_valid is then used to gate instr_valid (which goes to the decoder) and xif_issue_if.issue_ready.

I made the decision to implement the scalar source registers validity check in the vproc_core module and not in the vproc_decoder module because it is more related to a the x-interface than to the decoding of instructions.

moimfeld commented 2 years ago

I will check what caused the CI error and push another commit once I fixed it.

michael-platzer commented 2 years ago

I will check what caused the CI error and push another commit once I fixed it.

Regarding the CI error, this does not seem to be related to your changes. The testcase that fails appears to be deadlocked due to an incorrect assignment in CV32E40X that I reported here: openhwgroup/cv32e40x#610, though I did not yet find the time to dig deeper into this and understand the exact sequence of instructions that triggers the issue.

moimfeld commented 2 years ago

Hi @michael-platzer Thanks a lot for reviewing! I incorporated your feedback and now the implementation looks a lot cleaner. If you want I can squash the commits before merging, let me know.

I wrote my changes into the commit message, but here is a short list of the changes:

Sidenote: I force-pushed to my fork because I accidentally put an AND instead of an OR in one line of the code. Of course this was wrong and caused the CI checks to fail (I should have tested locally!).

michael-platzer commented 2 years ago

LGTM; I squashed your commits before merging.