core-v-verif
core-v-verif copied to clipboard
OBI_MEMORY_AGENT: conditional Driver and Sequencer instantiation
Type
Linter issues: the Driver and Sequencer are unconditionally created in uvma_obi_memory_agent_c::create_components()
Steps to Reproduce
Below are a set of links to the specific linter issues:
- 'sequencer' of type 'uvma_obi_memory_sqr_c' is not created in a proper conditional code
- 'driver' of type 'uvma_obi_memory_sqr_c' is not created in a proper conditional code
Additional context
This is a set of linter issues identified by the Verissimo SystemVerilog Testbench Linter. Please reach out to me directly if you have questions about the tool.
I disagree with the lint rule. While it may make sense to only create the components needed by the agent cfg, it ends up being a nightmare from a TLM connection standpoint. It forces users of the agent to check its own cfg before attempting to make any TLM connections:
my_agent_cfg_c cfg ;
my_agent_c my_agent;
uvm_analysis_fifo#(my_trn_c) drv_fifo;
uvm_analysis_fifo#(my_trn_c) mon_fifo;
...
my_agent = my_agent_c::type_id::create("my_agent", this);
drv_fifo = new("drv_fifo");
mon_fifo = new("mon_fifo");
...
// Conditionally connect some things
if (cfg.enabled) begin
my_agent.mon_ap.connect(mon_fifo.analysis_export);
if (cfg.is_active) begin
my_agent.drv_ap.connect(drv_fifo.analysis_export);
end
end
// or just don't care and connect everything
my_agent.mon_ap.connect(mon_fifo.analysis_export);
my_agent.drv_ap.connect(drv_fifo.analysis_export);
As a user, I don't want to have to care about the agent's internals. I want to configure it, create it and connect it. I don't want to minesweep for null references at runtime.
Therefore I think the lint rule should be ignored in favor of a relaxed build_phase() and connect_phase and have these conditions in the run_phase() alone.
I'm not sure I follow @datum-dpoulin. If the environment is using an agent as a monitor only, the environment "knows" that and configures the agent accordingly (by setting cfg.is_active to UVM_PASSIVE). So there is no reason for the environment to connect to the agent's sequencer port and no reason for the monitor agent to create a sequencer or a driver.
I agree with the lint rule. It is standard practice to only generate UVM hierarchy at run-time based on the dynamic configurations. This does have a downstream effect on connectivity and TLM but in general those issues should pop up as run-time null handle failures that are relatively easy to diagnose and fix.
There's a different reason to keep the sequencer in the mix, but that won't come up until we start using the advanced UVM approach of implementing monitoring loops in sequences.
Ok, I'll let go for now! :)
Do you want me to fix this in cv32e40p/dev?
Thanks @datum-dpoulin.
Do you want me to fix this in cv32e40p/dev?
Yes please. It will eventually propogated to the master and other core's branches.
Fix in PR #943