[BUG] remove ifndef VERILATOR directives

[JeanRochCoulon]

Is there an existing CVA6 bug for this?

• [X] I have searched the existing bug issues

Bug Description

Hello,

In CVA6 core, plenty of ifndef VERILATOR conditions RTL. Are asserts supported by new Verilator 5 version ? If it is the case, we could remove them from RTL.

core/include/instr_tracer_pkg.sv: ifndef VERILATOR core/include/ariane_pkg.sv: ifndef VERILATOR core/csr_regfile.sv: ifndef VERILATOR core/mmu_sv39/tlb.sv: ifndef VERILATOR core/mmu_sv32/cva6_tlb_sv32.sv: ifndef VERILATOR core/load_unit.sv: ifndef VERILATOR core/store_buffer.sv: ifndef VERILATOR core/issue_read_operands.sv: ifndef VERILATOR core/cache_subsystem/cva6_icache.sv: ifndef VERILATOR core/cache_subsystem/wt_dcache_missunit.sv: ifndef VERILATOR core/cache_subsystem/cache_ctrl.sv: ifndef VERILATOR core/cache_subsystem/wt_dcache_wbuffer.sv: ifndef VERILATOR core/cache_subsystem/wt_axi_adapter.sv: ifndef VERILATOR core/cache_subsystem/tag_cmp.sv: ifndef VERILATOR core/cache_subsystem/std_cache_subsystem.sv: ifndef VERILATOR core/cache_subsystem/wt_dcache_mem.sv: ifndef VERILATOR core/cache_subsystem/wt_l15_adapter.sv: ifndef VERILATOR core/cache_subsystem/wt_cache_subsystem.sv: ifndef VERILATOR core/cache_subsystem/miss_handler.sv: ifndef VERILATOR core/cache_subsystem/miss_handler.sv: ifndef VERILATOR core/cache_subsystem/wt_dcache.sv: ifndef VERILATOR core/cache_subsystem/wt_dcache_ctrl.sv: ifndef VERILATOR core/axi_shim.sv: ifndef VERILATOR core/scoreboard.sv: ifndef VERILATOR core/frontend/instr_queue.sv: ifndef VERILATOR core/frontend/frontend.sv: ifndef VERILATOR core/cva6.sv: ifndef VERILATOR core/cva6.sv: ifndef VERILATOR common/local/util/ex_trace_item.svh: ifndef VERILATOR common/local/util/instr_tracer_if.sv: ifndef VERILATOR common/local/util/instr_tracer.sv: ifndef VERILATOR common/local/util/instr_trace_item.svh: ifndef VERILATOR

[zchamski]

Local CI tests confirm that Verilator v5 copes well with assertions: after removing the ifndef VERILATOR occurrences in core/*.sv model builds OK, simulations run w/o failure. Work in progress...

[zchamski]

The uses of if(n)def VERILATOR can be split into several groups:

• trivially unneeded (empty ifndef .... endif), seen a.o. in the cache subsystem;
• guards of features which historically were not properly supported by Verilator (assert statements, wildcard parameter values, etc.)
• guards of instances which depend on such features (the ETH instruction tracer).

Currently (https://github.com/openhwgroup/cva6/commit/5a484fce428a666eeb4e115846541280e1a70e72) there are 48 VERILATOR-guarded regions in core SystemVerilog files:

• 28 inside CVFPU (core/cvfpu/);
• 15 in the cache subsystem (core/cache_subsystem);
• 3 in common/local (definitions used in the ETH instruction tracer);
• 2 in CVA6 code proper to control the inclusion of the ETH instruction tracer instance (cva6.sv, core/include/instr_tracer_pkg.sv).

The first two categories need to be addressed upstream if at all possible, since some asserted properties are quite complex (e.g., contain delays).

@zarubaf and @Moschn: Now that the RVFI+dasm combo is the recommended tracing solution for CVA6, can we drop the explicit tracer altogether?

[zchamski]

@cfuguet @khandelwaltanuj @pascalgouedo @davideschiavone : Which version(s) of Verilator are you using for verification? If you're on the v5 series, maybe some of the ifndef VERILATOR conditional guards can be dropped.

[cfuguet]

Yes, you are right. I use Verilator 5.008 or later, then I can take care of removing those in the HPDcache cache subsystem.