search

esl-epfl / x-heep

eXtendable Heterogeneous Energy-Efficient Platform based on RISC-V
Other
129 stars 69 forks source link

regtool generates a `reg_steer` signal that is one bit too short #507

Open cousteaulecommandant opened 1 month ago

cousteaulecommandant commented 1 month ago

https://github.com/esl-epfl/x-heep/blob/a639054570419fb706da38fd0c054e401dfb14d4/hw/vendor/pulp_platform_register_interface/vendor/lowrisc_opentitan/util/reggen/reg_top.sv.tpl#L192

The above line should say ${num_wins_width}, not ${num_wins_width-1}, since the definition of num_wins_width already includes the -1 (and in newer OpenTitan versions has been renamed to steer_msb which is a less misleading name):

https://github.com/esl-epfl/x-heep/blob/a639054570419fb706da38fd0c054e401dfb14d4/hw/vendor/pulp_platform_register_interface/vendor/lowrisc_opentitan/util/reggen/reg_top.sv.tpl#L16

The following files are affected:

cousteaulecommandant commented 1 month ago

https://github.com/lowRISC/opentitan/blob/bfed7763827d338e014aba80884273c23355009d/util/reggen/reg_top.sv.tpl#L26

In fact, in OpenTitan they have ditched the +1 in num_wins+1 since reg_steer only ever needs to take a value between 0 and num_wins (0 to num_wins-1 for a window, or num_wins for the plain register interface, i.e., num_wins+1 possible values). This difference made the code work for num_wins=1 (as well as 3, 7, 15, etc) since it was erroneously allocating a larger than needed range, and then cutting down that range by 1 bit, and the two errors cancelled out each other in the case of powers of 2 minus 1 (1, 3, 7...) but not for any other number of windows.

In short, this error does not surface if you only have 1 window, which I guess was the most widely tested use case, but it does surface e.g. for 2 or 4 windows.