Unnecessary Vector/FP Extension Dependency

[JosephMoore25]

When compiling a minimal RISC-V Sail source into Jib (though I expect this is the same when compiling the emulators) I'm finding that riscv_sys_control.sail is dependent on the following files:

• riscv_flen_{F/D}.sail
• riscv_vlen.sail
• riscv_vreg_type.sail
• riscv_vext_regs.sail

In the provided sail-riscv makefile, these are included in the default sources so are always compiled. As I understand, some discussion of a more configurable model (choosing what extensions to enable) has been underway. Of course, not all extensions are segregated, but this means that a relatively large portion of the vector extension is a dependency for compiling the "base" model.

The offending code is defining vector CSRs in init_sys() and uses some functions defined in the vector files. This is also the case with the floating point extension (either F or D), though this is less impactful as riscv_flen_D.sail is very small. I was able to compile without the above dependencies after removing this code.

Separating these definitions into a new (or other existing) file would help reduce the dependencies and make it a bit easier to only (or at least mostly) include subsets of extensions.

[Alasdair]

Yes, this is an issue I've also been dealing with. I think the solution is to make init_sys a scattered definition so each extension has a function clause init_sys(Ext_Name) that deals with its own state.

If you see my PR here: #572 the dependency that the riscv core module on the vector extension has to be spelled out - that's one of the big motivators for introducing a simple module system, as it will ensure that these kinds of circular dependencies don't exist and extensions are easily separable from the base ISA.

[JosephMoore25]

Ah yep I've seen the PR but hadn't read in detail so didn't make the link. Should perfectly solve this issue and would be really useful work.

Looking forward to the release, thanks!

[Timmmm]

Maybe hold off on changing init_sys for a few days. I literally just made a change to refactor a lot of the init functions so that they follow the spec properly for reset (currently the model conflates initialisation and reset, and resets way more than it should). Just waiting for internal approval to make a PR.

I like the scattered function solution; let's do that.

[jrtc27]

I think TestRIG currently relies on reset being the same as initialisation so it can keep reusing the same instance.

[Timmmm]

I think TestRIG uses the RVFI-DII interface which calls model_fini(); model_init(); between tests. I checked the generated code and it looks like that does a full reset of the model state, so registers get their initial value. I.e. if you have

register foo : xlenbits = legalize_foo(zeros())
register bar : xlenbits

It will reset foo to the legalised value and bar to 0 again between tests, so I think it should still work.

[jrtc27]

Thanks for checking, I couldn't remember the exact implementation details.

[Timmmm]

Here's the reset refactor I mentioned: #597