vicuna icon indicating copy to clipboard operation
vicuna copied to clipboard

Fix for #100

Open moimfeld opened this issue 3 years ago • 1 comments

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 avatar Oct 18 '22 12:10 moimfeld

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

moimfeld avatar Oct 18 '22 13:10 moimfeld

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.

michael-platzer avatar Oct 31 '22 20:10 michael-platzer

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!).

moimfeld avatar Nov 08 '22 18:11 moimfeld

LGTM; I squashed your commits before merging.

michael-platzer avatar Nov 16 '22 17:11 michael-platzer