[LSQ] generation of both verilog and VHDL

[JasmijnB]

A modified LSQ generation script which is able to generate the LSQ in both VHDL and Verilog.

The structure of the generated code is the exact same as the original, however statements are now constructed using a simple IR and then written using two separate emitters which output vhdl and verilog. A small report can be found here :)

Unfortunately there is a huge amount of changes because it was necessary to change all the Op calls.

[Jiahui17]

@JasmijnB Thanks for the effort! We should add you to the GitHub project so we know if your implementation passes all the CI tests.

---

This PR is way too long for us to give a meaningful evaluation---could you please break this PR into smaller PRs that we can easily map to the description in your report?

For instance, this paragraph in your report

The arguments passed to Op need to be adjusted as they are language specific. Take for example
the when and else in the previous example, which would be : and ? in Verilog. So the way to pass
the structure of the statements is done using a simple intermediate representation (found in IR.
py)

describes a very self-contained change and is within our reviewing workload.

[murphe67]

Hey @JasmijnB this looks like a very cool experiment/idea 😁😁- unfortunately we have modifications to the LSQ python script that we intend to merge soon, and we cannot merge both sets of changes due to how extensive this PR is.

Once we merge the new features, this PR will then be unfortunately out of date.

Are you going to be working more on this in the future, and can adapt to the incoming changes? Otherwise I am not sure how we can proceed...

[JasmijnB]

@Jiahui17 This is less straightforward than it seems, since e.g. the replacement of Op already involves updating all the generators and supporting functions + declaration of the IR + implementation of the emitter, so that is already 90% of the changed lines. I can maybe try splitting the updates into smaller parts however, for example a separate pull request which only updates the group allocator?

This would mean that for the partial pull requests it is not possible to test the verilog generation, but I suppose this is fine since the verilog generation only involves the implementation of one extra emitter.

[JasmijnB]

@murphe67 Depending on the size and nature of the changes I think it should be pretty straightforward to adapt, I can't guarantee that I'll have the time but probably

let's first wait for the PR to merge and then I can take a look :)

[Jiahui17]

@JasmijnB Number of lines changed should not matter if you can convince the reviewers that those changes are easy to review and easy to rebase

For instance, I noticed that there are hardly any differences besides one variable is replaced with the new IR (which is a great change that can be independently contributed):

One corresponding PR would be [LSQ] Switch from hard-coded opcode string literals to IR object. Hopefully, here we will only need to scroll through changes like above

[Jiahui17]

One extra thing: maybe you used a different line break (the windows' one?) than what's being consistently used in the project (only LF i think

https://stackoverflow.com/questions/1552749/difference-between-cr-lf-lf-and-cr-line-break-types

Normally GitHub is able to correspond the changed lines much better but now it is completely clueless

[JasmijnB]

@Jiahui17 I think the fundamental issue is that I moved the files from vhdl_gen to core_gen, and git is not recognizing the move when showing the diff as the file contents are too different. If you run e.g. git diff main:tools/backend/lsq-generator-python/vhdl_gen/generators/group_allocator.py HfunctionEAD:tools/backend/lsq-generator-python/core_gen/generators/group_allocator.py it compares fine. If you want to compare the changes you can temporarily move core_gen back to vhdl_gen. I can also undo/reorder this move to a later commit if that makes things easier.

The first PR replacing the Op would account for (almost) all changed lines in the existing code (generators signals, and operators). All these changes are indeed just replacing the Op statements with their equivalent IR representation, and moving some duplicate code to an emitter class method.
I suppose I could leave lsq.py as is, and put the removal of the duplicate signal classes in utils.py in a second PR (not generating verilog in the first PR), and then lastly have a third PR introducing the VerilogEmitter?

[Jiahui17]

Not sure about the last one but the first two seem easy to review and react to

@MaxWipfli what do you think

[MaxWipfli]

Thanks for asking, @Jiahui17.

From an admin POV, the timing of this is pretty unfortunate. As @murphe67 already said, we may apply some pretty broad changes to the LSQ as part of my current thesis work. I won't have time to completely refactor and (importantly) retest all the changes. So either this PR has to wait, or someone else (@murphe67, I guess) would have to do the final PRs to upstream my work. This has to be decided by @murphe67.

From a technical POV, I think this is a good improvement (assuming the need to generate both Verilog and VHDL is given).

From a workflow POV, I agree that this PR should probably be split into multiple smaller PRs to be more reviewable. I am also happy to do some reviewing on this, as I am probably the person most familiar with the LSQ generator right now.

[JasmijnB]

@MaxWipfli I think for now it's smart to wait for your PR to be merged. If your changes don't use any new VHDL features, and depend on the same existing generation framework (so used Op calls etc.) it shouldn't be too much work to rebase my branch.