Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

CV32E40Pv2 verification update week 12 (bis) #2394

Merged
merged 7 commits into from
Mar 22, 2024
Merged

CV32E40Pv2 verification update week 12 (bis) #2394

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
9297f32
Merge pull request #198 from XavierAubert/cv32e40p/dev
XavierAubert Mar 22, 2024
fb200cb
Fix incorrect illeal bin definition
dd-baoshan Mar 22, 2024
f9db87f
Fix minor flaw in idx randomization
dd-baoshan Mar 22, 2024
a71ae14
Update Imperas RISCVISACOV path
dd-baoshan Mar 22, 2024
ecaaf3d
Merge pull request #199 from XavierAubert/cv32e40p/bsm-update_tb_file…
dd-baoshan Mar 22, 2024
27e4b0d
Added Imperas switch for correct compressed instructions coverage.
Mar 22, 2024
844f3d0
Merge pull request #200 from XavierAubert/cv32e40p/dev_dd_pgo
XavierAubert Mar 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions cv32e40p/env/corev-dv/cv32e40p_rand_instr_stream.sv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,11 @@ class cv32e40p_rand_instr_stream extends riscv_rand_instr_stream;
bit idx_search_done = 0;
int rand_cnt = 0;

idx = $urandom_range(0, current_instr_cnt-1);
do begin
idx = $urandom_range(0, current_instr_cnt-1);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not a fan of using system tasks such as $urandom() because it makes writing constraints difficult. Having said that, I know that riscv-dv makes extensive use of these, so I will accept this.

end while (instr_list[idx].atomic);
while (!instr_list[idx].atomic && !idx_search_done) begin
int idx_e = 0;
idx = $urandom_range(0, current_instr_cnt-1);
idx_e = (idx + new_instr_cnt-1);

if (idx_start.size() == 0) begin : SEARCH_IDX_FOR_NEW_INSTR
Expand Down Expand Up @@ -134,6 +135,10 @@ class cv32e40p_rand_instr_stream extends riscv_rand_instr_stream;
break;
end // rand_cnt

do begin
idx = $urandom_range(0, current_instr_cnt-1);
end while (instr_list[idx].atomic);

end // while

if (instr_list[idx].atomic) begin
Expand Down
2 changes: 1 addition & 1 deletion cv32e40p/env/uvme/cov/uvme_cv32e40p_fp_instr_covg.sv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,7 @@ class uvme_cv32e40p_fp_instr_covg extends uvm_component;
// from bhv_logic_3
cp_lsu_apu_contention_wr_rd : coverpoint cntxt.cov_vif.curr_rd_at_wb_regfile_wr_contention {
bins rd[] = {[0:31]} with ( ((item + 1) * (fpu_latency == 1)) != 0 );
illegal_bins rd_addr_32_63 = {[32:63]} with ( ((item + 1) * (fpu_latency == 1)) != 0 );
bins fd[] = {[32:63]} with ( ((item + 1) * (fpu_latency == 1)) != 0 ); // flw uses ls path for load to freq
}

// from bhv_logic_2 (revised)
Expand Down
2 changes: 1 addition & 1 deletion cv32e40p/env/uvme/cov/uvme_cv32e40p_zfinx_instr_covg.sv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -421,7 +421,7 @@ class uvme_cv32e40p_zfinx_instr_covg extends uvm_component;
// from bhv_logic_3
cp_lsu_apu_contention_wr_rd : coverpoint cntxt.cov_vif.curr_rd_at_wb_regfile_wr_contention {
bins rd[] = {[0:31]} with ( (item < 32) & (fpu_latency == 1) );
illegal_bins rd_addr_32_63 = {[32:63]};
illegal_bins rd_addr_32_63 = {[32:63]}; // zfinx does not use freg
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you are declaring these as illegal_bins, does that imply a bug in the RTL, testbench or test-program? without looking at the logfiles, how will we know that one of these illegal bins has been hit?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This coverage file was created by previous owner and I only helped in fixing or update based on coverage analysis.
In my understanding, the illegal bin here mean for rtl issue if hit. It is more appropriate to have this behavior coded as rtl assertion but I did not to edit too much on this coverage model unless the illegal behavior is incorrect. I did mention this illegal_bin to Pascal and he is aware as well.

there are 2 ways to detect if illegal bin is hit. the simulation log (in this case, vsim) will issue runtime error in reporting. We can also observe illegal bin by open the ucdb for coverage analysis. However, we only interested on error reporting and not coverage analysis for illegal bin, therefore log file is the only way we use to detect such error.

Copy link
Member

@MikeOpenHWGroup MikeOpenHWGroup Mar 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. Let's keep this as-is for now. I will use Issue #2395 to suggest that we remove all illegal_bins and replace them with error assertions.

BTW, I really dislike parsing the simulation logs for for errors.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW, I really dislike parsing the simulation logs for for errors.

So do I 😀.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. Let's keep this as-is for now. I will use Issue #2395 to suggest that we remove all illegal_bins and replace them with error assertions.

BTW, I really dislike parsing the simulation logs for for errors.

@MikeOpenHWGroup , I agreed that every person has its own preferences but there are some points we need to think ahead how to address these runtime errors.
so far I know these are the sources of run time errors:

  1. assertion (immediate or property)
    • any assertion will print "Error" message in log file and Erorr count is increase in vsim log file.
    • we can implement the use of uvm_error by adding "else `uvm_error()" in the assertions clause.
  2. illegal bins in cover points
    • any hit will print "Error" message in log file and Erorr count is increase in vsim log file.

there are few things we need to take notes. illegal bins is allow according to systemverilog LRM. Even though we can set rules to prohibit the use of illegal bins in this tb, this will not become a full proof resolution because:-

  1. we cannot prevent third party packages from using it. e.g Imperas coverage model is using illegal bins for illegal instruction checks in its ISACOV.
  2. sometimes designers has tendency to code assertions in rtl files (note: here is referring to assertions that are not bind into rtl) and there is no obligation to prevent them from not using uvm_error because
    • using uvm_error macro requires import of uvm_package and macro included in rtl. (not all testbench is implement with uvm_methodology)
    • designers sometimes does not aware of uvm methodology and and these macro. they usually prefer using $error, $warning, or $fatal in assertions if necessary.

Copy link

@pascalgouedo pascalgouedo Mar 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @dd-baoshan for making the list of errors not managed by uvm_errors.
I could add to this list that Imperas model is not using uvm macros to report Hardware Loops constraints checks (as I understand it). But as they are already managing mismatch with those uvm macros, I guess HLC could do the same.

}

// from bhv_logic_2 (revised)
Expand Down
3 changes: 1 addition & 2 deletions cv32e40p/tb/uvmt/imperas_dv.flist
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,7 @@

+incdir+${IMPERAS_HOME}
+incdir+${IMPERAS_HOME}/ImpProprietary/include/host
+incdir+${IMPERAS_HOME}/ImpProprietary/source/host/CV32E40Pv2_riscvISACOV/source
// +incdir+${IMPERAS_HOME}/ImpProprietary/source/host/riscvISACOV/source
+incdir+${IMPERAS_HOME}/ImpProprietary/source/host/riscvISACOV/source

${TBSRC_HOME}/uvmt/uvmt_cv32e40p_imperas_riscv_coverage_config.svh
-f ${IMPERAS_HOME}/ImpPublic/source/host/rvvi/rvvi.f
Expand Down
1 change: 1 addition & 0 deletions cv32e40p/tests/cfg/default.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ compile_flags:
ovpsim: >
--override cpu/misa_Extensions=0x001104
--override cpu/compress_version=1.0.0-RC5.7
--override cpu/show_c_prefix=T
--override cpu/noinhibit_mask=0xFFFFFFF0
--override cpu/marchid=4
--override cpu/extension_CVE4P/CORE_VERSION=1
Expand Down
1 change: 1 addition & 0 deletions cv32e40p/tests/cfg/no_pulp.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ compile_flags: >
ovpsim: >
--override cpu/misa_Extensions=0x001104
--override cpu/compress_version=1.0.0-RC5.7
--override cpu/show_c_prefix=T
--override cpu/noinhibit_mask=0xFFFFFFF0
--override cpu/marchid=4
--override cpu/extension_CVE4P/CORE_VERSION=1
Expand Down
1 change: 1 addition & 0 deletions cv32e40p/tests/cfg/num_mhpmcounter_29.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ compile_flags:
ovpsim: >
--override cpu/misa_Extensions=0x001104
--override cpu/compress_version=1.0.0-RC5.7
--override cpu/show_c_prefix=T
--override cpu/noinhibit_mask=0000000000
--override cpu/marchid=4
--override cpu/extension_CVE4P/CORE_VERSION=1
Expand Down
1 change: 1 addition & 0 deletions cv32e40p/tests/cfg/pulp.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ compile_flags:
ovpsim: >
--override cpu/misa_Extensions=0x001104
--override cpu/compress_version=1.0.0-RC5.7
--override cpu/show_c_prefix=T
--override cpu/noinhibit_mask=0xFFFFFFF0
--override cpu/marchid=4
--override cpu/mimpid=1
Expand Down
1 change: 1 addition & 0 deletions cv32e40p/tests/cfg/pulp_cluster.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ compile_flags:
ovpsim: >
--override cpu/misa_Extensions=0x001104
--override cpu/compress_version=1.0.0-RC5.7
--override cpu/show_c_prefix=T
--override cpu/noinhibit_mask=0xFFFFFFF0
--override cpu/marchid=4
--override cpu/mimpid=1
Expand Down
1 change: 1 addition & 0 deletions cv32e40p/tests/cfg/pulp_cluster_fpu.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ compile_flags:
ovpsim: >
--override cpu/misa_Extensions=0x001104
--override cpu/compress_version=1.0.0-RC5.7
--override cpu/show_c_prefix=T
--override cpu/noinhibit_mask=0xFFFFFFF0
--override cpu/marchid=4
--override cpu/mimpid=1
Expand Down
1 change: 1 addition & 0 deletions cv32e40p/tests/cfg/pulp_cluster_fpu_1cyclat.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ compile_flags:
ovpsim: >
--override cpu/misa_Extensions=0x001104
--override cpu/compress_version=1.0.0-RC5.7
--override cpu/show_c_prefix=T
--override cpu/noinhibit_mask=0xFFFFFFF0
--override cpu/marchid=4
--override cpu/mimpid=1
Expand Down
1 change: 1 addition & 0 deletions cv32e40p/tests/cfg/pulp_cluster_fpu_2cyclat.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ compile_flags:
ovpsim: >
--override cpu/misa_Extensions=0x001104
--override cpu/compress_version=1.0.0-RC5.7
--override cpu/show_c_prefix=T
--override cpu/noinhibit_mask=0xFFFFFFF0
--override cpu/marchid=4
--override cpu/mimpid=1
Expand Down
1 change: 1 addition & 0 deletions cv32e40p/tests/cfg/pulp_cluster_fpu_zfinx.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ compile_flags:
ovpsim: >
--override cpu/misa_Extensions=0x001104
--override cpu/compress_version=1.0.0-RC5.7
--override cpu/show_c_prefix=T
--override cpu/noinhibit_mask=0xFFFFFFF0
--override cpu/marchid=4
--override cpu/mimpid=1
Expand Down
1 change: 1 addition & 0 deletions cv32e40p/tests/cfg/pulp_cluster_fpu_zfinx_1cyclat.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ compile_flags:
ovpsim: >
--override cpu/misa_Extensions=0x001104
--override cpu/compress_version=1.0.0-RC5.7
--override cpu/show_c_prefix=T
--override cpu/noinhibit_mask=0xFFFFFFF0
--override cpu/marchid=4
--override cpu/mimpid=1
Expand Down
1 change: 1 addition & 0 deletions cv32e40p/tests/cfg/pulp_cluster_fpu_zfinx_2cyclat.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ compile_flags:
ovpsim: >
--override cpu/misa_Extensions=0x001104
--override cpu/compress_version=1.0.0-RC5.7
--override cpu/show_c_prefix=T
--override cpu/noinhibit_mask=0xFFFFFFF0
--override cpu/marchid=4
--override cpu/mimpid=1
Expand Down
1 change: 1 addition & 0 deletions cv32e40p/tests/cfg/pulp_fpu.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ compile_flags:
ovpsim: >
--override cpu/misa_Extensions=0x001104
--override cpu/compress_version=1.0.0-RC5.7
--override cpu/show_c_prefix=T
--override cpu/noinhibit_mask=0xFFFFFFF0
--override cpu/marchid=4
--override cpu/mimpid=1
Expand Down
1 change: 1 addition & 0 deletions cv32e40p/tests/cfg/pulp_fpu_1cyclat.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ compile_flags:
ovpsim: >
--override cpu/misa_Extensions=0x001104
--override cpu/compress_version=1.0.0-RC5.7
--override cpu/show_c_prefix=T
--override cpu/noinhibit_mask=0xFFFFFFF0
--override cpu/marchid=4
--override cpu/mimpid=1
Expand Down
1 change: 1 addition & 0 deletions cv32e40p/tests/cfg/pulp_fpu_2cyclat.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ compile_flags:
ovpsim: >
--override cpu/misa_Extensions=0x001104
--override cpu/compress_version=1.0.0-RC5.7
--override cpu/show_c_prefix=T
--override cpu/noinhibit_mask=0xFFFFFFF0
--override cpu/marchid=4
--override cpu/mimpid=1
Expand Down
1 change: 1 addition & 0 deletions cv32e40p/tests/cfg/pulp_fpu_zfinx.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ compile_flags:
ovpsim: >
--override cpu/misa_Extensions=0x001104
--override cpu/compress_version=1.0.0-RC5.7
--override cpu/show_c_prefix=T
--override cpu/noinhibit_mask=0xFFFFFFF0
--override cpu/marchid=4
--override cpu/mimpid=1
Expand Down
1 change: 1 addition & 0 deletions cv32e40p/tests/cfg/pulp_fpu_zfinx_1cyclat.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ compile_flags:
ovpsim: >
--override cpu/misa_Extensions=0x001104
--override cpu/compress_version=1.0.0-RC5.7
--override cpu/show_c_prefix=T
--override cpu/noinhibit_mask=0xFFFFFFF0
--override cpu/marchid=4
--override cpu/mimpid=1
Expand Down
1 change: 1 addition & 0 deletions cv32e40p/tests/cfg/pulp_fpu_zfinx_2cyclat.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ compile_flags:
ovpsim: >
--override cpu/misa_Extensions=0x001104
--override cpu/compress_version=1.0.0-RC5.7
--override cpu/show_c_prefix=T
--override cpu/noinhibit_mask=0xFFFFFFF0
--override cpu/marchid=4
--override cpu/mimpid=1
Expand Down