Fix for #100

[moimfeld]

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]

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

[michael-platzer]

Hi @moimfeld,

I finally got around to taking a look at your PR. Your implementation looks good to me, although I am wondering if you could move some of this to the decoder module which already uses similar nested case statements to differentiate between the different types of vector instructions. That might help to reduce code duplication.

[michael-platzer]

The decoder module uses similar nested case statements, maybe this could partially be moved to the decoder module. You could, for instance, add two output signals named needs_rs1 and needs_rs2 to the decoder and then assign source_xreg_valid in the core module as follows:

assign source_xreg_valid = (~needs_rs1 | xif_issue_if.issue_req.rs_valid[0]) & (~needs_rs2 | xif_issue_if.issue_req.rs_valid[1]);

[michael-platzer]

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]

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:

• moved most logic from core module to decoder module
• added a signal called xreg to the op_regs typedef
  • this signal is only high if the corresponding source register is scalar
  • Note: I could not reuse the vreg signal of op_regs because vreg is also low when the operand is an immediate or not used at all
  • xreg is assigned in the decoder and then passed to the core module via the rs1_o and rs2_o outputs

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]

LGTM; I squashed your commits before merging.