Missing coverage in the div-01.s testcase

[ssecatch-w]

The div-01 testcase has 32 initial operations where the registers are scrambled for rd, rs1, and rs2.
There are an additional 557 operations that use x12 for rd, x10 for rs1, and x11 for rs2.
I believe that this setup does not fully meet coverage, which also implies that the functional coverage for this operation needs to be updated.

These two operations appear in the first 32 register scrambling choices:
div zero,zero,s10
div a7,tp,zero

while rd == rs1 == x0 is a necessary bin, there should also be
rd == x0 != rs1 to ensure that divides never write to x0
and possibly
rs1 == x0 != rd to ensure that divides will accept a dividend register x0 and calculate it properly

Note that there are already several testcases for a dividend value of 0, just not one for a dividend register of x0 that doesn't "write" to x0:
inst_10:
// rs1==x7, rs2==x13, rd==x20, rs1_val == 0, rs2_val == -9
// opcode: div ; op1:x7; op2:x13; dest:x20; op1val:0x0; op2val:-0x9
TEST_RR_OP(div, x20, x7, x13, 0x0, 0x0, -0x9, x2, 40, x8)

And as far as I can tell, that first one is the only testcase that validates that rs1 == rd, which given that they are both x0, does not really cover this case.
inst_0:
// rs1 == rd != rs2, rs1==x0, rs2==x26, rd==x0, rs1_val > 0 and rs2_val > 0, rs1_val != rs2_val, rs2_val == 262144
// opcode: div ; op1:x0; op2:x26; dest:x0; op1val:0x0; op2val:0x40000
TEST_RR_OP(div, x0, x0, x26, 0, 0x0, 0x40000, x2, 0, x8)

I believe the coverage needs to be update to be something more like:
rs1 == rd != rs2; rd != x0
rs1 == rd != rs2; rd == x0
rs1 == x0 != rd
rd == x0 != rs1

[neelgala]

this requires updates to specific nodes in the dataset.cgf so that the same scenarios can be applied across other instructions as well.

[alitariq4589]

@pawks I dont think there is yet any functionality in riscv-ctg to check direct/numeric values to rd register in rfmt_op_comb, we can just check it against rs1 and rs2 and no functionality to even add rd_val at all in rfmt_val_comb. I think there should be an added functionality for this... or is there any other way to check direct numeric value against it ?

[pawks]

CTG is a directed test generator which uses the coverpoints as constraints. The coverpoints are always defined over the source registers. You can always decompose a rd based coverpoint to its source operands.

[alitariq4589]

@pawks There needs to be support for checking whether or not a register is x0 or not in riscv-ctg for this issue to be solved. Currently there is only support for checking register names across each other only, like rs1== rd != rs2. And what we need is rs1 == rd != rs2 and rd != x0 and also rs1 == rd != rs2 and rd == x0.

[pawks]

@pawks There needs to be support for checking whether or not a register is x0 or not in riscv-ctg for this issue to be solved. Currently there is only support for checking register names across each other only, like rs1== rd != rs2. And what we need is rs1 == rd != rs2 and rd != x0 and also rs1 == rd != rs2 and rd == x0.

I believe the kind of coverpoints you are mentioning are already supported. Did you face any errors or issues when you tried to run the tool with these coverpoints?

[alitariq4589]

Yes. There is support for checking the values of registers (like whats stored in them), but i have not yet found any support for checking the register names (like whether or not the register is x0 or x1). I believe op_comb is the label which should have this support in dataset.cgf.

[alitariq4589]

This issue is resolved in PR #368 by generating new tests for div-01.S of riscv-arch-test and the related repo of riscv-ctg which has changes in dataset.cgf is PR#68.

[ssecatch-w]

I don't think it has been resolved.
As far as I can tell, the new code still has the case of rd == rs == x0
and rd != rs; rs == x0
but not
rd != rs; rd == x0

[YazanHussnain-10x]

Can you tell me which source register you are referring to?

[ssecatch-w]

Both of them.

There should be separate cases for rs1 = x0 (the dividend) and rs2 = x0 (the divisor), as a divide of 0 has defined behavior in the specification, and of divide by 0 is well understood to be 0:
Condition, Dividend, Divisor, DIVU[W], REMU[W], DIV[W], REM[W]
Division by zero, x, 0, 2L − 1, x, −1, x

[alitariq4589]

@ssecatchseagate in response to your (following) first comment, the conditions were updated in the dataset.cgf as were exactly told by you.

I believe the coverage needs to be update to be something more like:
rs1 == rd != rs2; rd != x0
rs1 == rd != rs2; rd == x0
rs1 == x0 != rd
rd == x0 != rs1

But after seeing your most recent comment, I see that you wanted to do more like following:

rs1 == x0 and rd != x0
rs1 == x0 and rd ==x0
rs2 == x0 and rd != x0
rs2 == x0 and rd == x0

This covers the case when dividend is zero and also covers when divisor is zero and these conditions are checked in both the cases of rd (when rd == x0 and when rd != x0); so covering the condition which you just described. I think the dataset.cgf should have these nodes instead of what were added in the first place in the PR.

Please add up if I am missing something.

[ssecatch-w]

Correct, that is probably more complete.
(Additionally, by my reading, all three registers could also be x0)

However, I didn't see those cases in the div-01.S pointed to in the PR. Maybe I'm missing it in the code, can you identify it for me. This is all I could see locate from https://github.com/riscv-non-isa/riscv-arch-test/blob/f56b5649b2c7adbbf06c5e3d8f065d300e7459bb/riscv-test-suite/rv32i_m/M/src/div-01.S
However, the file is large, and perhaps I was not searching for the correct text string.

rs1 == x0 & rd == x0 from before:
inst_0:
// rs1 == rd != rs2 and rd == 'x0', rs1==x0, rs2==x31, rd==x0, rs1_val != rs2_val, rs1_val > 0 and rs2_val < 0, rs1_val==46341 and rs2_val==-46339
// opcode: div ; op1:x0; op2:x31; dest:x0; op1val:0x0; op2val:-0xb503
TEST_RR_OP(div, x0, x0, x31, 0, 0x0, -0xb503, x1, 0*XLEN/8, x2)

rs2 == x0, rs1 and rd some other value. This is new, and replaces an older ==x0 check that was in the first 10 instruction sequences in the repo.
inst_59:
// rs2==x0, rs1_val == -1048577,
// opcode: div ; op1:x31; op2:x0; dest:x31; op1val:-0x100001; op2val:0x0
TEST_RR_OP(div, x31, x31, x0, 0xFFFFFFFF, -0x100001, 0x0, x8, 7*XLEN/8, x9)

[alitariq4589]

@ssecatchseagate You were right, all the coverpoints are not getting used to generate the tests. Usually, it is expected from the riscv-ctg to generate all the tests depending upon the coverpoints in op_comb node of the given coverpoint (or at least that is what I understand). Here the coverpoint used in op_comb is rfmt_op_comb which is defined in dataset.cgf and which needed your mentioned coverpoints to be added.

@pawks I have added the coverpoints inside rfmt_op_comb in dataset.cgf in following manner (as was also discussed in our last meeting):

    'rs1 == rd != rs2 and rd != ''x0''': 0
    'rs1 == rd != rs2 and rd == ''x0''': 0
    'rs1 == ''x0'' and rd != rs1': 0
    'rd == ''x0'' and rs1 != rd': 0

Above coverpoints are related directly to the production of tests of this issue.

I have also tried adding them as following:

    "rs1 == rd != rs2 and rd != 'x0'": 0
    "rs1 == rd != rs2 and rd == 'x0'": 0
    "rs1 == 'x0' and rd != rs1": 0
    "rd == 'x0' and rs1 != rd": 0

Both of the above are an example of using escape characters in yaml script and have the same behavior.

Now, there are two problems which are associated with this type of cgf format:

1. Sometimes one out of four and sometimes two out of four above register-combination coverpoints are not used while generating the test. Is there a way to enforce the riscv-ctg tool to use all of the four coverpoints when generating the assembly tests ?

2. Sometimes, the riscv_ctg throws following error:

Command Used:

riscv_ctg -v debug -d /home/user1/10xE/compliance/custom/issue\#306_div-01.S -r -cf /home/user1/10xE/compliance/riscv-ctg/sample_cgfs/dataset.cgf -cf /home/user1/10xE/compliance/riscv-ctg/sample_cgfs/rv32im.cgf -bi rv32i -p4

Error Log:

    INFO | ****** RISC-V Compliance Test Generator 0.11.0 *******
    INFO | Copyright (c) 2020, InCore Semiconductors Pvt. Ltd.
    INFO | All Rights Reserved.
    INFO | Copying env folder to Output directory.
 WARNING | Deprecated node used: 'opcode'. Use 'mnemonics' instead
 WARNING | Deprecated node used: 'opcode'. Use 'mnemonics' instead
 WARNING | Deprecated node used: 'opcode'. Use 'mnemonics' instead
 WARNING | Deprecated node used: 'opcode'. Use 'mnemonics' instead
 WARNING | Deprecated node used: 'opcode'. Use 'mnemonics' instead
 WARNING | Deprecated node used: 'opcode'. Use 'mnemonics' instead
 WARNING | Deprecated node used: 'opcode'. Use 'mnemonics' instead
 WARNING | Deprecated node used: 'opcode'. Use 'mnemonics' instead
 WARNING | Neither mnemonics nor csr_comb node not found in covergroup: datasets
    INFO | Generating Test for :div-div
   DEBUG | div : Generating OpComb
    INFO | Generating Test for :divu-divu
   DEBUG | divu : Generating OpComb
    INFO | Generating Test for :mul-mul
   DEBUG | mul : Generating OpComb
    INFO | Generating Test for :mulh-mulh
   DEBUG | mulh : Generating OpComb
    INFO | Generating Test for :mulhsu-mulhsu
   DEBUG | mulhsu : Generating OpComb
    INFO | Generating Test for :mulhu-mulhu
   DEBUG | mulhu : Generating OpComb
    INFO | Generating Test for :rem-rem
   DEBUG | rem : Generating OpComb
    INFO | Generating Test for :remu-remu
   DEBUG | remu : Generating OpComb
multiprocessing.pool.RemoteTraceback: 
"""
Traceback (most recent call last):
  File "/usr/lib/python3.10/multiprocessing/pool.py", line 125, in worker
    result = (True, func(*args, **kwds))
  File "/usr/lib/python3.10/multiprocessing/pool.py", line 51, in starmapstar
    return list(itertools.starmap(args[0], args[1]))
  File "/home/user1/.local/lib/python3.10/site-packages/riscv_ctg/ctg.py", line 79, in create_test
    gen_test(op_node, opcode)
  File "/home/user1/.local/lib/python3.10/site-packages/riscv_ctg/ctg.py", line 46, in gen_test
    op_comb = gen.opcomb(node)
  File "/home/user1/.local/lib/python3.10/site-packages/riscv_ctg/generator.py", line 344, in opcomb
    cond_str += ", ".join([var+"=="+solution[var] for var in cond_vars]+[op_comb[i] for i in sat_set])
  File "/home/user1/.local/lib/python3.10/site-packages/riscv_ctg/generator.py", line 344, in <listcomp>
    cond_str += ", ".join([var+"=="+solution[var] for var in cond_vars]+[op_comb[i] for i in sat_set])
TypeError: 'set' object is not subscriptable
"""

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/home/user1/.local/bin/riscv_ctg", line 8, in <module>
    sys.exit(cli())
  File "/home/user1/.local/lib/python3.10/site-packages/click/core.py", line 1157, in __call__
    return self.main(*args, **kwargs)
  File "/home/user1/.local/lib/python3.10/site-packages/click/core.py", line 1078, in main
    rv = self.invoke(ctx)
  File "/home/user1/.local/lib/python3.10/site-packages/click/core.py", line 1434, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/home/user1/.local/lib/python3.10/site-packages/click/core.py", line 783, in invoke
    return __callback(*args, **kwargs)
  File "/home/user1/.local/lib/python3.10/site-packages/riscv_ctg/main.py", line 29, in cli
    ctg(verbose, out_dir, randomize ,xlen, int(flen), cgf,procs,base_isa,inst)
  File "/home/user1/.local/lib/python3.10/site-packages/riscv_ctg/ctg.py", line 129, in ctg
    results = pool.starmap(create_test, [(usage_str, node,label,base_isa,max_inst, op_template,
  File "/usr/lib/python3.10/multiprocessing/pool.py", line 375, in starmap
    return self._map_async(func, iterable, starmapstar, chunksize).get()
  File "/usr/lib/python3.10/multiprocessing/pool.py", line 774, in get
    raise self._value
TypeError: 'set' object is not subscriptable

Now if I disable multiprocessing by replacing the line 129 and 130 of ctg.py for better debugging the error with following lines:

for label,node in cgf.items():
    results = create_test(usage_str, node,label,base_isa,max_inst, op_template,randomize, out_dir, xlen, flen)

the error log becomes as follows:

    INFO | ****** RISC-V Compliance Test Generator 0.11.0 *******
    INFO | Copyright (c) 2020, InCore Semiconductors Pvt. Ltd.
    INFO | All Rights Reserved.
    INFO | Copying env folder to Output directory.
 WARNING | Deprecated node used: 'opcode'. Use 'mnemonics' instead
 WARNING | Neither mnemonics nor csr_comb node not found in covergroup: datasets
    INFO | Generating Test for :div-div
   DEBUG | div : Generating OpComb
Traceback (most recent call last):
  File "/home/user1/.local/bin/riscv_ctg", line 8, in <module>
    sys.exit(cli())
  File "/home/user1/.local/lib/python3.10/site-packages/click/core.py", line 1157, in __call__
    return self.main(*args, **kwargs)
  File "/home/user1/.local/lib/python3.10/site-packages/click/core.py", line 1078, in main
    rv = self.invoke(ctx)
  File "/home/user1/.local/lib/python3.10/site-packages/click/core.py", line 1434, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/home/user1/.local/lib/python3.10/site-packages/click/core.py", line 783, in invoke
    return __callback(*args, **kwargs)
  File "/home/user1/.local/lib/python3.10/site-packages/riscv_ctg/main.py", line 29, in cli
    ctg(verbose, out_dir, randomize ,xlen, int(flen), cgf,procs,base_isa,inst)
  File "/home/user1/.local/lib/python3.10/site-packages/riscv_ctg/ctg.py", line 132, in ctg
    results = create_test(usage_str, node,label,base_isa,max_inst, op_template,randomize, out_dir, xlen, flen)
  File "/home/user1/.local/lib/python3.10/site-packages/riscv_ctg/ctg.py", line 79, in create_test
    gen_test(op_node, opcode)
  File "/home/user1/.local/lib/python3.10/site-packages/riscv_ctg/ctg.py", line 46, in gen_test
    op_comb = gen.opcomb(node)
  File "/home/user1/.local/lib/python3.10/site-packages/riscv_ctg/generator.py", line 345, in opcomb
    cond_str += ", ".join([var+"=="+solution[var] for var in cond_vars]+[op_comb[i] for i in sat_set])
  File "/home/user1/.local/lib/python3.10/site-packages/riscv_ctg/generator.py", line 345, in <listcomp>
    cond_str += ", ".join([var+"=="+solution[var] for var in cond_vars]+[op_comb[i] for i in sat_set])
TypeError: 'set' object is not subscriptable

And on checking the type of op_comb, it turns out that it is a set. A set in python cannot be index, but in line 345 in generator.py, you will see op_comb[i] which is indexing the set elements. So is this problem a bug in the riscv-ctg tool ? But interesting thing is, this error does not appear every time the riscv_ctg command is used and it started coming after adding the coverpoints I have mentioned above. Also one more thing to note is that, the frequency of getting this error increases when using more threads (like -p4) while generating the tests (or so I have observed).

[MuhammadHammad001]

@ssecatchseagate. @neelgala, @allenjbaum, @alitariq4589, @UmerShahidengr
The reason for this comment is to mention that I have generated the tests for the coverpoints mentioned in this issue by @ssecatchseagate in the PR#385 of riscv-arch-test using the solution proposed in PR#79 of riscv-ctg. Now, coverage is also 100%. The reason for missing coverage has been explained in the later PR. I would be thankful if you please review both of these PRs and let me know if any change is required.