cva6 icon indicating copy to clipboard operation
cva6 copied to clipboard

[BUG] csr_regfile is not compilant with SystemVerilog Standard

Open cathales opened this issue 8 months ago • 0 comments

Is there an existing CVA6 bug for this?

  • [x] I have searched the existing bug issues

Bug Description

According to IEEE Std 1800-2017 SystemVerilog 3.1a Language Reference Manual, Accellera’s Extensions to Verilog®, Section 3.14 Casting, the syntax before the ' can be:

  • a decimal-based literal number >0 (the size as a literal)
  • the signing (signed or unsigned)
  • a type identifier, optionally package-scoped (with ::)

Unless I missed something (a newer version of the standard?), the casts used to build IsaCode in core/csr_regfile.sv are not covered by the official specification of the language. If we do not comply with the standard, we have no guarantee that the tools perform as we expect. So we should probably find a way to comply with the standard.

Here is an example from csr_regfile:

https://github.com/openhwgroup/cva6/blob/c3fe25aeda97187fa8d9829ab924e9513c78fd8b/core/csr_regfile.sv#L282-L295

Do we really need these casts? All the values around the casts can fit in a signed 32 bit number, which is IIRC the default size for integers. The one which is the closest to overflow is MXL, which does not have the cast. So we could probably remove all these casts?

Later in the same file, we use MIP values, which are little integer constants (at most 4096). So these casts should probably be removed there too. To avoid implicit cast warnings, we could probably make the constants unsized (e.g. int unsigned). After all, if we always re-size them, there is no real reason to have them sized.

If we really need these casts, the standard suggests to use a local typedef <the type> the_type_alias; (types with generic parameters do not exist in SystemVerilog so we cannot put them in a package) and then cast with the_type_alias'(expression).

cathales avatar Mar 12 '25 12:03 cathales