core-v-verif icon indicating copy to clipboard operation
core-v-verif copied to clipboard

VCS Compile error of ISACOV and suggested work-around

Open MikeOpenHWGroup opened this issue 3 years ago • 9 comments

Issue Type

Compile error under Synopsys VCS (Cadence Xcelium and Metrics Dsim do not have this issue). The offending code in the ISA coverage model is:

  cp_rs1: coverpoint instr.rs1;
  cp_rs2: coverpoint instr.rs2;
  cp_rd: coverpoint instr.rd;

  cross_rd_rs1_rs2: cross cp_rd, cp_rs1, cp_rs2 {
    ignore_bins IGN_OFF = cross_rd_rs1_rs2 with (!reg_crosses_enabled);
  }

The VCS compile-time error message is:

Error-[FCIWCS] Invalid 'with' clause specification ...../uvma_isacov_cov_model.sv, 32 uvma_isacov_pkg, "cross_rd_rs1_rs2"

  The 'with' clause specified on the cross bin does not have any impact and is invalid; it should contain at least one occurence of the constituent coverpoint names.

  Please remove or correct the clause and compile

Steps to Reproduce

Any compile of a core-v-verif UVM environment with SIMULATOR=vcs

Additional context

My understanding of the SV LRM is that the offending code is compliant. The LRM states that a with_covergroup_expression can be any SV expression (See IEEE-1800-2017, clause 19.5, p. 559), so cross_rd_rs1_rs2 with (!reg_crosses_enabled) is legal SV syntax.

Suggested resolution

Asking to Synopsys to fix VCS will take a long time. Perhaps we can modify the code to accomplish the same thing in another way. Does this work:

  cross_rd_rs: cross cp_rd, cp_rs {
    bins CPRD_X_CP_RS = cross_rd_rs iff (reg_crosses_enabled);
  }

MikeOpenHWGroup avatar Nov 09 '21 16:11 MikeOpenHWGroup

Hi @ASintzoff, I forgot to ask, which version of VCS are you using?

MikeOpenHWGroup avatar Nov 09 '21 19:11 MikeOpenHWGroup

@ASintzoff @MikeOpenHWGroup I do not have access to VCS to try this, but this recoding appears to still work in Xcelium:

   cross_rd_rs1_rs2: cross cp_rd, cp_rs1, cp_rs2 {
-    ignore_bins IGN_OFF = cross_rd_rs1_rs2 with (!reg_crosses_enabled);
+    ignore_bins IGN_OFF = binsof(cp_rd) intersect {[0:$]} with (!reg_crosses_enabled);
   }

Please try this and let me know if it both:

  • Suppresses all cross bins when reg_crosses_enabled == 0
  • Enables all cross bins when reg_crosses_enabled == 1

strichmo avatar Nov 09 '21 19:11 strichmo

Hi @ASintzoff, I forgot to ask, which version of VCS are you using?

Q-2020.03-SP2-1

ASintzoff avatar Nov 10 '21 08:11 ASintzoff

@ASintzoff @MikeOpenHWGroup I do not have access to VCS to try this, but this recoding appears to still work in Xcelium:

   cross_rd_rs1_rs2: cross cp_rd, cp_rs1, cp_rs2 {
-    ignore_bins IGN_OFF = cross_rd_rs1_rs2 with (!reg_crosses_enabled);
+    ignore_bins IGN_OFF = binsof(cp_rd) intersect {[0:$]} with (!reg_crosses_enabled);
   }

Please try this and let me know if it both:

* Suppresses all cross bins when reg_crosses_enabled == 0

* Enables all cross bins when reg_crosses_enabled == 1

Unfortunately, the idiom you are suggesting is not working with VCS.

ASintzoff avatar Nov 10 '21 08:11 ASintzoff

Issue Type

Suggested resolution

Asking to Synopsys to fix VCS will take a long time. Perhaps we can modify the code to accomplish the same thing in another way. Does this work:

  cross_rd_rs: cross cp_rd, cp_rs {
    bins CPRD_X_CP_RS = cross_rd_rs iff (reg_crosses_enabled);
  }

This suggested workaround is working with VCS. A pull request will follow.

ASintzoff avatar Nov 10 '21 08:11 ASintzoff

Before creating a pull request, I would like to be complete. Other constructions using with not working with VCS exist in the file.

To summarize all of them, are such workarounds suitable?

   cross_rd_rs1_rs2: cross cp_rd, cp_rs1, cp_rs2 {
-    ignore_bins IGN_OFF = cross_rd_rs1_rs2 with (!reg_crosses_enabled);
+    bins CPRD_X_CP_RS1_RS2 = cross_rd_rs1_rs2 iff (reg_crosses_enabled);
  }

  cp_csr: coverpoint instr.csr {
-    bins CSR[] = {[USTATUS:VLENB]} with (cfg_illegal_csr[item] == 0);
+    bins CSR[] = {[USTATUS:VLENB]} iff (cfg_illegal_csr[instr.csr] == 0); 
  }

  cp_group: coverpoint (instr.group) {
-    illegal_bins ILL_EXT_A = {ALOAD_GROUP, ASTORE_GROUP, AMEM_GROUP} with (!ext_a_enabled);
+    illegal_bins ILL_EXT_A = {ALOAD_GROUP, ASTORE_GROUP, AMEM_GROUP} iff (!ext_a_enabled);
  }

  cp_csr: coverpoint(instr_prev.csr) iff (instr_prev != null) {
-    bins CSR[] = {[USTATUS:VLENB]} with (cfg_illegal_csr[item] == 0);
+    bins CSR[] = {[USTATUS:VLENB]} iff (cfg_illegal_csr[instr_prev.csr] == 0);
  } 

ASintzoff avatar Nov 10 '21 10:11 ASintzoff

@ASintzoff your above change:

cross_rd_rs: cross cp_rd, cp_rs {
    bins CPRD_X_CP_RS = cross_rd_rs iff (reg_crosses_enabled);
}

Doesn't compile unless I change to:

cross_rd_rs: cross cp_rd, cp_rs {
    bins CPRD_X_CP_RS = cross_rd_rs with (reg_crosses_enabled);
}

However even with this change, I do not get the intended effect. When reg_crosses_enabled is 0, I still get all of the cross bins defined via auto bin generation in Xcelium.

The intention is for this cross to be unpopulated by default (as this cross generates > 30000 bins!)

To make progress, and since VCS appears to be the outlier, I would like to propose that we ifdef VCS the alternative encoding for these bins.

cross_rd_rs: cross cp_rd, cp_rs {
`ifdef VCS
    bins CPRD_X_CP_RS = cross_rd_rs iff (reg_crosses_enabled);
`else
    ignore_bins IGN_OFF = cross_rd_rs with (!reg_crosses_enabled);
`endif
}

@MikeOpenHWGroup ifdefs should be a last resort but I do not see an alternative, and the ignore_bins format is heavily used within the ISACOV model. To me it is a hard requirement that we control the bin generation based on ISACOV configuration tightly within the code itself, without relying on simulator refinement, or other tool-specific techniques.

strichmo avatar Nov 10 '21 16:11 strichmo

Agree with @strichmo that ifdefs should be a last resort, but it seems we cannot make progress without them. So I propose that we use them on the cva6/dev branch of core-v-verif for now. This will enable the CVA6 team to make progress integrating ISACOV into the CVA6 environment. I am very motivated to see how much effort it will be to re-use ISACOV on one of the CV32A6 variants. (It should be a small effort, but this has not been done before.)

In the meantime, @ASintzoff, please contact your local Synopsys team and get their feedback on this. You can tell them that a global community of Verification engineers believe that VCS has a bug. :wink:

MikeOpenHWGroup avatar Nov 10 '21 18:11 MikeOpenHWGroup

Agree with @strichmo that ifdefs should be a last resort, but it seems we cannot make progress without them. So I propose that we use them on the cva6/dev branch of core-v-verif for now. This will enable the CVA6 team to make progress integrating ISACOV into the CVA6 environment. I am very motivated to see how much effort it will be to re-use ISACOV on one of the CV32A6 variants. (It should be a small effort, but this has not been done before.)

OK. So, once the CVA6 team will re-use ISACOV, a VCS workaround will be only available on cva6/dev branch of core-v-verif.

In the meantime, @ASintzoff, please contact your local Synopsys team and get their feedback on this. You can tell them that a global community of Verification engineers believe that VCS has a bug.

Contact done and issue opened.

ASintzoff avatar Nov 15 '21 09:11 ASintzoff

Update: I now have access to Synopsys FAEs and they have provided what I think is a practical workaround, see the attached file. This workaround is currently being vetted in pull-request #2006.

FunctionalCoverageInvalidWithClauseSpecification.txt

MikeOpenHWGroup avatar Jun 23 '23 17:06 MikeOpenHWGroup

@MikeOpenHWGroup Do you know if this 2021 issue is resolved?

silabs-robin avatar Dec 11 '23 13:12 silabs-robin

Yes, this issue was resolved and it should have been closed - my bad.

Having said that, the so-called FunctionalCoverageInvalidWithClauseSpecification issue with VCS is still there and I am aware of some new additions to E40S/X functional coverage that re-introduced this code construct. I will deal with these assuming we can get to #2314 resolved one day. :wink:

MikeOpenHWGroup avatar Dec 11 '23 20:12 MikeOpenHWGroup