Open Silabs-ArjanB opened 4 years ago
PITONARIANE is already prefixed with PITON as it is related to OpenPiton. That is our prefix. Please do not remove these macros.
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.
@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.
@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
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
Nice! Can you rebase it ?
@cfuguet added at least one macro with the HPDC: https://github.com/openhwgroup/cva6/blob/master/core/cache_subsystem/cva6_hpdcache_subsystem.sv#L267
The CVA6 uses the following defines (used in ifdef/ifndef statements):
Similar to the request made in https://github.com/openhwgroup/cv32e40p/issues/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 https://github.com/openhwgroup/cv32e40p/pull/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.