core-v-verif
                                
                                 core-v-verif copied to clipboard
                                
                                    core-v-verif copied to clipboard
                            
                            
                            
                        ISACOV: Sequential sampling incorrect
Type
- Functionally incorrect behavior
Steps to Reproduce
Run any test/regression and look at rev32_seq_cg.
Additional context
It seems like the sampling of sequences of instructions is non-functional.
When I look at coverage metrics for a "full" regression, it seems only "A->A" transitions are covered (e.g. "ADDI->ADDI"). (See screenshot below.) This indicates that something about the coverage definition or its sampling is not working as intended. @AyoubJalali are you seeing the same results as I do?
I don't know what is needed to get this working, but the principle is simple so it could potentially be easy.

ps. For "instruction group" coverage, it seems to be working. Except that we need to exclude "UNKNOWN_GROUP".

@AyoubJalali are you seeing the same results as I do?
Yes, exactly same result, i didn't notice that because for me it the last cg to look for now (for me).
Except that we need to exclude "UNKNOWN_GROUP".
Yes because no sample if instr = UNKNOWN
@silabs-robin look at this https://github.com/openhwgroup/core-v-verif/blob/05ba4eb0cf6296bf35f487fbe2ab01e532d0fe26/lib/uvm_agents/uvma_isacov/cov/uvma_isacov_cov_model.sv#L1399
isn't we should create cover group for instr_prev.name instead of instr.name ?? like this
cp_instr_prev_x2: coverpoint(instr_prev.name) iff (instr_prev != null) { `ISACOV_IGN_BINS ignore_bins IGN_X2_OFF = {[0:$]} iff (!seq_instr_x2_enabled); }
isn't we should create cover group for instr_prev.name instead of instr.name ?? like this
That seems to be it, yeah :)
Yes it works for me
Nice. Looking forward to re-measuring our coverage with all of the improvements you have made.
I will close this issue after a related PR gets merged.
Best luck to get 100% :)
I will close this issue after a related PR gets merged.
@silabs-robin can you provide the actual PR number here? Thanks.
I will close this issue after a related PR gets merged.
@silabs-robin can you provide the actual PR number here? Thanks.
There is no PR number yet AFAIK. But, @AyoubJalali, you have a fix for the problem, so you might soon have a PR up? If you @ me in that PR I can link to it here, or if you reference this issue in the PR, then Mike's request will be fulfilled.
No problem for me, I'll reference this issue in the PR