Avoid using System Verilog defines in RTL code

[Silabs-ArjanB]

The CVA6 uses the following defines (used in ifdef/ifndef statements):

• DROMAJO
• FIRESIM_TRACE
• FORMAL
• INSTR_TRACER_IF_SV
• PITON_ARIANE
• SYNTHESIS
• VERILATOR
• WT_DCACHE

Similar to the request made in openhwgroup/cv32e40p#301 all such System Verilog defines should (ideally) be completely avoided in RTL code (and for example be replaced by localparam or parameter, avoided by a cleaner split between RTL and non-RTL code (in separate files), etc.). On CV32E40P all defines were avoided by rewrites performed in openhwgroup/cv32e40p#389 (actually the define related to assertions is still there, but will soon be removed as well as assertions will move into seperate files outside of the RTL files). If for some reason defines will still be used, it would be cleaner if they use a uniform prefix (e.g. CVA6_) to reduce the chance of conflicts with other RTL code.

[Jbalkind]

PITON_ARIANE is already prefixed with PITON_ as it is related to OpenPiton. That is our prefix. Please do not remove these macros.

[MikeOpenHWGroup]

Thank you for your comments @Silabs-ArjanB, and FWIW, I completely agree with you.

@JeanRochCoulon, let's solve this one-and-for-all. Let's first tackle the non PITON_ARIANE macros. Once we have a solution in place, we can raise the topic with @Jbalkind.

[JeanRochCoulon]

@MikeOpenHWGroup @Jbalkind
I have started to remove the defines from the RTL. #1127 is a PR to remove WT_DCACHE.

After the modification, you can find below the remaining defines in the CVA6 RTL:
grep -r ifdef |grep .sv|grep core/
core/include/wt_cache_pkg.sv:ifdef PITON_ARIANE
core/include/wt_cache_pkg.sv:ifdef PITON_ARIANE
core/include/ariane_pkg.sv:ifdef PITON_ARIANE
core/include/ariane_pkg.sv:ifdef PITON_ARIANE
core/include/ariane_pkg.sv:ifdef SPIKE_TANDEM
core/include/ariane_pkg.sv:ifdef PITON_ARIANE
core/include/ariane_pkg.sv:ifdef RVFI_MEM
core/csr_regfile.sv:ifdef PITON_ARIANE
core/csr_regfile.sv:ifdef DROMAJO
core/pmp/src/pmp_entry.sv: ifdef FORMAL
core/pmp/src/pmp_entry.sv: ifdef FORMAL
core/pmp/src/pmp.sv: ifdef FORMAL
core/cache_subsystem/wt_cache_subsystem.sv:ifdef PITON_ARIANE
core/cache_subsystem/wt_cache_subsystem.sv:ifdef PITON_ARIANE
core/scoreboard.sv:ifdef RVFI_MEM
core/scoreboard.sv:ifdef RVFI_MEM
core/cva6.sv:ifdef DROMAJO
core/cva6.sv:ifdef FIRESIM_TRACE
core/cva6.sv:ifdef RVFI_TRACE
core/cva6.sv:ifdef PITON_ARIANE
core/cva6.sv:ifdef PITON_ARIANE
core/cva6.sv:ifdef FIRESIM_TRACE
core/cva6.sv:ifdef PITON_ARIANE
core/cva6.sv:ifdef DROMAJO
core/cva6.sv:ifdef DROMAJO
core/cva6.sv:ifdef RVFI_TRACE
core/cva6.sv:ifdef RVFI_MEM
core/ariane.sv:ifdef FIRESIM_TRACE
core/ariane.sv:ifdef RVFI_TRACE
core/ariane.sv:ifdef PITON_ARIANE
core/ariane.sv:ifdef FIRESIME_TRACE
core/ariane.sv:ifdef RVFI_TRACE
core/ariane.sv:ifdef PITON_ARIANE

@Jbalkind, can you have a look to PITON_ARIANE ? I will address the RVFI_TRACE and RVFI_MEM in the meantime.

[JeanRochCoulon]

#1127 has been merged to replace the WT_DCACHE by localparam defined in config pkg

[JeanRochCoulon]

@Jbalkind, quick update. I think the work done on parametrization can help in removing PITON_ARIANE ifdef.
grep -r ifdef |grep .sv|grep core/
core/include/wt_cache_pkg.sv:ifdef PITON_ARIANE
core/include/wt_cache_pkg.sv:ifdef PITON_ARIANE
core/include/ariane_pkg.sv:ifdef PITON_ARIANE
core/include/ariane_pkg.sv:ifdef SPIKE_TANDEM
core/include/ariane_pkg.sv:ifdef PITON_ARIANE
core/csr_regfile.sv:ifdef PITON_ARIANE
core/cache_subsystem/wt_cache_subsystem.sv:ifdef PITON_ARIANE
core/cva6.sv:ifdef PITON_ARIANE

[Jbalkind]

#1321 had a draft approach which could enable replacing one or two more of those by introducing noc_type_e NOCType in cva6_cfg_t

I have a branch which gets rid of the rest but is using an older cva6 https://github.com/Jbalkind/ariane/tree/remove_piton_ariane

Seems like we're getting closer to a solution

[JeanRochCoulon]

Nice! Can you rebase it ?

[Jbalkind]

@cfuguet added at least one macro with the HPDC: https://github.com/openhwgroup/cva6/blob/master/core/cache_subsystem/cva6_hpdcache_subsystem.sv#L267