Use of $urandom()

[MikeOpenHWGroup]

In core-v-verif there are 8 files that use some variation of $urandom(). You are in the "assignees" list of this issue because git believes you authored one or more of these.

The Verissimo linter flags these as violation SVTB.29.1.3.1, which is a lint error because class members randomized with these calls cannot be constrained. I also worry that invocations of $urandom() may impair random stability (that is, the ability to reproduce results from the same seed).

Possible resolutions:

1. Accept the use of $urandom() and create a project-wide waiver for SVTB.29.1.3.1.
2. Enforce the use of a configuration or context class of random members to implement the same functionality.
3. Waive, or not, on a case-by-case basis.

Example fixes

From uvma_obi_memory_drv.sv. Replace this:

UVMA_OBI_MEMORY_DRV_IDLE_RANDOM: begin
   `uvm_info("OBI_MEMORY_DRV", $sformatf("Invalid drv_random: %0d", cfg.drv_idle), UVM_NONE)
   slv_mp.drv_slv_cb.rdata <= $urandom();
   slv_mp.drv_slv_cb.err   <= $urandom();
   slv_mp.drv_slv_cb.ruser <= $urandom();
   slv_mp.drv_slv_cb.rid   <= $urandom();
end

...with this:

UVMA_OBI_MEMORY_DRV_IDLE_RANDOM: begin
   `uvm_info("OBI_MEMORY_DRV", $sformatf("Invalid drv_random: %0d", cfg.drv_idle), UVM_NONE)
   if (!obi_mem_drv_random_cntxt.randomize())
      `uvm_error("OBI_MEMORY_DRV", "Cannot randomize obi_mem_drv_random_cntxt")
   slv_mp.drv_slv_cb.rdata <= obi_mem_drv_random_cntxt.rdata;
   slv_mp.drv_slv_cb.err   <= obi_mem_drv_random_cntxt.err;
   slv_mp.drv_slv_cb.ruser <= obi_mem_drv_random_cntxt.ruser;
   slv_mp.drv_slv_cb.rid   <= obi_mem_drv_random_cntxt.rid;
end

Steps to Reproduce

Lint checks are run every six hours - the link above will show all violations of this rule.

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.

[datum-dpoulin]

This is the exception to the rule. We have a cfg parameter that is random constrained: cfg.drv_idle_mode. One of the options is to drive random data on idle.

I think it's a waive, on a case-per-case basis

[MikeOpenHWGroup]

Can you give a bit more context on this one @datum-dpoulin?

[datum-dpoulin]

"... lint error because class members randomized with these calls cannot be constrained"

cfg.drv_idle_mode is being constrained to create this random behavior. That's why I'd say that this is the exception to the linter rule.

"I also worry that invocations of $urandom() may impair random stability (that is, the ability to reproduce results from the same seed)."

Why would $urandom() be different than other randomization functions?

FYI the new OBI agent architecture doesn't have problems like this because the driver gets a new sequence item from the sequencer on every clock. Just puts the contents onto the wires.

[camitroaie]

I believe you are right @datum-dpoulin, it looks like an exception safe to waive. You do have the drv_idle_mode knob that already controls the behavior... to some extent... in the worst case "a control freak" could use factories to override uvma_obi_memory_drv_c with a child class that overrides uvma_obi_memory_drv_c::drv_mstr_idle(). Of course it would have been nicer to only have to override a drv_mstr_idle_random() containing just the UVMA_OBI_MEMORY_DRV_IDLE_RANDOM case item code that sets $urandom() values, but it could be over-engineering, maybe a comment with the solution is enough.

+You could use some inline code waivers here: https://dvteclipse.com/documentation/svlinter/Inline_Lint_Waivers.html

I don't think random stability is an issue.

[MikeOpenHWGroup]

OK, I'll roll over on this one. Please add an inline waiver as shown above.

So, that covers the example in the original issue report. There are still 7 other files with SVTB.29.1.3.1 lint errors.

@strichmo, I think you have already addressed the ones attributed to you. @datum-dpoulin and @silabs-oysteink, please have a look at yours.

[strichmo]

I did not address this before my departure but here's my 2 cents:

1. As long as $urandom or $urandom_range is used we should be staying with the SystemVerilog LRM seeding and should maintain random stability.
2. At least with Xcelium, the new constraint solver has a higher "fixed cost" for doing quick randomizations. For example on some internal UVCs we were initially using objects to fully randomize "per-cycle" delay values. However in profiling this created a very noticeable slowdown in simulation by invoking the constraint solver much more often. We achieved multiples of speedup by replacing these simpler constraints with $urandom_range(). I followed a similar thinking when writing some of the OBI UVC.

[MikeOpenHWGroup]

OK, it appears that a consensus is developing that in these instances it is safe to waive SVTB.29.1.3.1, and in some cases desirable from a performance standpoint. I would like to add guidance on this in our newly minted coding style guidelines. The Verissimo rule is:

Do not use $urandom, $urandom_range $random, $dist_uniform, $dist_normal, $dist_exponential,
$dist_poisson, $dist_chi_square, $dist_t, $dist_erlang because members randomized with these
calls cannot be constrained.

How about we add this to our coding stlye guidelines?

Use of $urandom, $urandom_range $random, $dist_uniform, $dist_normal, $dist_exponential,
$dist_poisson, $dist_chi_square, $dist_t, $dist_erlang should be avoided because members randomized
with these calls cannot be constrained.

This rule may be waived if the randomization of a member using one of these system methods is controlled
by a member that is constrainable.  For example:

    if (cfg.drive_random_rdata) begin
       slv_mp.drv_slv_cb.rdata <= $urandom();
    end

Note that the use of the verbs "should" and "may" is consistent with the guidelines.