Replace all asserts & assumes with macros from `assertions.svh`

[michael-platzer]

This PR replaces all assertions (and assumes) in the common cells sources with macros from assertions.svh.

Additionally, the assertions.svh itself is modified as follows:

• Some defines (such as the guard block at the start of the header) are renamed for consistency and to avoid name collisions in the same style as in registers.svh.
• The `ifndef VERILATOR is removed because Verilator has supported assertions for a while now.
• An option to override any of the default defines that cause assertions to be turned off (such as SYNTHESIS) is added (so even if a tool defines, e.g., SYNTHESIS, it still has the option to forcefully enable assertions by also defining ASSERTS_OVERRIDE_ON now — yes, there are tools out there that require that :roll_eyes:).
• Helper macros are undefined at the end of the file (a comment did claim that this was already the case, but it was not).
• An optional extra argument is added to all assertions macros, which allows to specify a custom error message to be displayed when the assertion fails. For backwards compatibility, this extra argument is added at the end and defaults to "".

In order to replace all the assertions in the sources while avoiding typos and other oversights as best as possible, I have replaced almost all of them using sed commands. The specific commands used for each change are part of the respective commit messages.

Some noteworthy subtle changes that go along with this move:

• The macros in assertions.svh require that all assertions have a name, so I added one for all those who did not have one (and I tried to hard to keep them consistent).
• So far, assertions used a mix of $fatal, $error, and sometimes $warning to report failing assertions. The macros in assertions.svh universally use $error and all instances of $fatal are now changed to $error. I reviewed all those that used $warning but since they all appear to be pretty critical issues as well, they now also use the macros which report an $error (the affected files are src/addr_decode_dync.sv, src/cdc_fifo_gray_clearable.sv, src/multiaddr_decode.sv, and src/spill_register_flushable.sv, so the authors of these @WRoenninger, @fabianschuiki, @zarubaf, @meggiman, and @colluca might want to take a look).
• Assertions that were previously declared as assert initial or assert final are now immediate assertions in a initial or final block, respectively.
• src/rr_arb_tree.sv, src/stream_omega_net.sv, and src/stream_xbar.sv previously used a default disable (e.g., default disable iff (~rst_ni);), which btw is also not supported by Verilator. Each assertion macro has an disable iff !rst_ni instead, so I removed those.

Also, the following files make use of assertions in a pretty advanced/adventurous way (which btw also causes Verilator 5.018 to fail as reported in #229):

• src/addr_decode_dync.sv: Includes a huge always block with assumes all over the place.
• src/multiaddr_decode.sv: Again an always block with an assume in it.

I'd like to ask the respective authors (@WRoenninger and @colluca) to review the changes to these files (you are of course welcome to review changes to the other files as well).

Merging this PR closes #232.

[colluca]

@michael-platzer thanks for the PR :)

I will not be able to review this before Monday, but I will definitely have a look then.

[michael-platzer]

I reckon that this is a pretty large PR. @niwis Would you rather have me split this into multiple smaller PRs? Perhaps starting with the changes to assertions.svh only?

[michael-platzer]

Thanks a lot @colluca and @phsauter for the reviews!

One thing I would perhaps change, is to move all include statements at the beginning of the files, rather than within the modules. I believe this is more consistent with other include statements and better aligned with one's expectation on where to find included files. Also, for files defining multiple modules e.g. cdc_2phase_clearable this would mean we only need to include it once.

Agree, for the assertion macros this makes more sense, since they are intended to be defined globally anyways.

Could we also move COMMON_CELLS_ASSERTS_OFF to assertions.svh for consistency? As of now, some but not all assertions would be disabled, which I think would go against expectation.

We could, but IMHO that is not consistent and would defeat the purpose to disable assertions in the common_cells sources only while allowing the assertion macros to be used in other places as well. See my comments above.

Finally, in the PR description (and later in the changelog) I would mention that some assertions which did not define a disable iff condition now do after aligning them to use the ASSERT macro, and I would list the files affected by this change (e.g. stream_to_mem.sv).

Good point, will add that.

[michael-platzer]

With the help of sed I have identified that only stream_intf.sv and stream_to_mem.sv contain assertions that previously did not use disable iff (or some equivalent default disable statement) and do now due to the switch to the assertion macros. This also made me realize that the stream_intf has no reset port. I have substituted the !rst_ni with 1'b0 to keep the assertions always enabled, which should be equivalent to the previous behavior.

The compiled list of changes that could be added to next release changelog is as follows:

• stream_to_mem: Assertions are now disabled during reset.
• addr_decode_dync: $warning on assertion violation has been promoted to $error.
• cdc_fifo_gray_clearable: $warning on assertion violation has been promoted to $error.
• multiaddr_decode: $warning on assertion violation has been promoted to $error.
• spill_register_flushable: $warning on assertion violation has been promoted to $error.
• rr_arb_tree: Default assertions disable (default disable iff (~rst_ni);) has been removed in favor of disable iff on each assertion.
• stream_omega_net: Default assertions disable (default disable iff (~rst_ni);) has been removed in favor of disable iff on each assertion.
• stream_xbar: Default assertions disable (default disable iff (~rst_ni);) has been removed in favor of disable iff on each assertion.

[michael-platzer]

Please note that addr_decode_dync.sv fails to elaborate with the newest Verilator release 5.028 because of issues with the assertion constructs that are unrelated to the changes in this PR. See https://github.com/pulp-platform/common_cells/pull/229#issuecomment-2382908002

[michael-platzer]

As far as I can tell, all changes are backwards-compatible.

To the best of my knowledge, they should be.

In particular, no new fatals were introduced.

That, I can confirm with certainty. Some $fatals have been converted to $errors but there are no new $fatals for sure.