Bug 545 - Some errors and failures when running tests, with recent nMigen
Summary: Some errors and failures when running tests, with recent nMigen
Status: RESOLVED FIXED
Alias: None
Product: Libre-SOC's first SoC
Classification: Unclassified
Component: Source Code (show other bugs)
Version: unspecified
Hardware: PC Linux
: --- enhancement
Assignee: Cesar Strauss
URL:
Depends on:
Blocks:
 
Reported: 2020-12-12 09:42 GMT by Cesar Strauss
Modified: 2021-05-18 14:22 BST (History)
3 users (show)

See Also:
NLnet milestone: ---
total budget (EUR) for completion of task and all subtasks: 0
budget (EUR) for this task, excluding subtasks' budget: 0
parent task for budget allocation:
child tasks for budget allocation:
The table of payments (in EUR) for this task; TOML format:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Cesar Strauss 2020-12-12 09:42:18 GMT
Lately, as I test nMigen CxxSim, I began to systematically run a greater number of tests than I was used to, as well as keeping nMigen itself updated.

Here is a summary of the results.

Simulator: PySim

Commit IDs:

1) Libre-SOC:
commit 9380181f4c19e845730c8fbbecf18fcbfc9b6f06 (HEAD -> master, origin/master, origin/HEAD)
Author: Cesar Strauss <cestrauss@gmail.com>
Date:   Mon Dec 7 18:44:40 2020 -0300

    Display the instruction type as a vector on cxxsim

2) nmutil:
commit 32a50f48658d5250305a6a4ac8e738c93a44038f (HEAD -> master, origin/master, origin/HEAD)
Author: Cesar Strauss <cestrauss@gmail.com>
Date:   Sun Dec 6 08:47:37 2020 -0300

    Implement the "submodule" attribute

3) pia:
commit 5f7d8c0e32c6aeba8533553b77e6261018b84c06 (HEAD -> master, tag: v0.2.0, origin/master, origin/HEAD)
Author: Jacob Lifshay <programmerjake@gmail.com>
Date:   Wed Oct 28 16:56:57 2020 -0700

    add crate descriptions

4) nmigen-soc
commit 692017c7eaf21ff37302790c4422db6bd08667be (HEAD -> master, origin/master, origin/HEAD)
Author: Luke Kenneth Casson Leighton <lkcl@lkcl.net>
Date:   Thu Oct 1 18:15:54 2020 +0100

    must not delay ack to wb request in SRAM

5) nMigen:
commit 2f8231d961cda20756226a310fdf20279046b801 (HEAD -> cxxsim, origin/cxxsim)
Author: whitequark <whitequark@whitequark.org>
Date:   Mon Dec 7 10:54:06 2020 +0000

    sim.cxxsim: preserve reset value of toplevel inputs.

Test results:

1) Tests with syntax errors:
src/soc/fu/alu/formal/proof_input_stage.py
src/soc/fu/alu/formal/proof_output_stage.py
src/soc/fu/branch/formal/proof_input_stage.py
src/soc/fu/branch/formal/proof_main_stage.py
src/soc/fu/cr/formal/proof_main_stage.py
src/soc/fu/div/formal/proof_main_stage.py
src/soc/fu/logical/formal/proof_input_stage.py
src/soc/fu/logical/formal/proof_main_stage.py
src/soc/fu/mul/formal/proof_main_stage.py
src/soc/fu/shift_rot/formal/proof_main_stage.py
src/soc/regfile/formal/proof_regfile_binary.py
src/soc/decoder/formal/proof_decoder2.py 
src/soc/decoder/test/test_decoder_gas.py
src/soc/experiment/compldst_multi.py 
src/soc/experiment/cscore.py 
src/soc/experiment/imem.py 
src/soc/experiment/lsmem.py 
src/soc/experiment/score6600.py 
src/soc/experiment/score6600_multi.py 
src/soc/experiment/test/async_sim.py

2) Tests that fails assertions:
src/soc/simple/test/test_issuer.py
src/soc/decoder/test/test_power_decoder.py
src/soc/decoder/test/test_decoder_gas.py
src/soc/fu/mul/test/test_pipe_caller.py 
src/soc/fu/trap/test/test_pipe_caller.py
src/soc/fu/compunits/test/test_trap_compunit.py 
src/soc/fu/logical/test/test_pipe_caller.py
src/soc/fu/compunits/formal/proof_fu.py
src/soc/fu/alu/formal/proof_main_stage.py
src/soc/fu/spr/formal/proof_main_stage.py
src/soc/fu/trap/formal/proof_main_stage.py
src/soc/regfile/formal/proof_regfile.py
src/soc/regfile/formal/proof_regfile_array.py
src/soc/decoder/formal/proof_decoder.py 
src/soc/experiment/icache.py 

3) Tests that hangs (enters infinite loops):
src/soc/experiment/l0_cache.py 
src/soc/fu/compunits/test/test_logical_compunit.py 

4) Tests that run without any problems:
src/soc/fu/alu/test/test_pipe_caller.py 
src/soc/fu/branch/test/test_pipe_caller.py 
src/soc/fu/cr/test/test_pipe_caller.py 
src/soc/fu/div/test/test_pipe_caller.py 
src/soc/fu/mmu/test/test_pipe_caller.py 
src/soc/fu/shift_rot/test/test_pipe_caller.py 
src/soc/fu/spr/test/test_pipe_caller.py 
src/soc/fu/compunits/test/test_alu_compunit.py 
src/soc/fu/compunits/test/test_branch_compunit.py 
src/soc/fu/compunits/test/test_cr_compunit.py 
src/soc/fu/compunits/test/test_div_compunit.py 
src/soc/fu/compunits/test/test_ldst_compunit.py 
src/soc/fu/compunits/test/test_shiftrot_compunit.py 
src/soc/fu/compunits/test/test_spr_compunit.py 
src/soc/experiment/alu_fsm.py 
src/soc/experiment/alu_hier.py 
src/soc/experiment/dcache.py 
src/soc/experiment/mmu.py 
src/soc/experiment/test/test_compalu_multi.py 
src/soc/experiment/test/test_l0_cache_buffer2.py 
src/soc/experiment/test/test_mmu_dcache.py 
src/soc/experiment/test/test_mmu_dcache_pi.py 
src/soc/fu/logical/formal/proof_bpermd.py
src/soc/experiment/formal/proof_alu_fsm.py 
src/soc/experiment/formal/proof_datamerger.py
Comment 1 Luke Kenneth Casson Leighton 2020-12-12 10:22:01 GMT
thanks cesar this is a huge help.

(In reply to Cesar Strauss from comment #0)

> Test results:
> 
> 1) Tests with syntax errors:

these really should not be happening.  however they are nonessential.

> 2) Tests that fails assertions:
> src/soc/simple/test/test_issuer.py

if this is just the bpermd test that's fine
> src/soc/fu/mul/test/test_pipe_caller.py

not fine.
 
> src/soc/fu/trap/test/test_pipe_caller.py
> src/soc/fu/compunits/test/test_trap_compunit.py 

these are expected.


> src/soc/fu/logical/test/test_pipe_caller.py

if bpermd that is an ISACaller random fault


> 3) Tests that hangs (enters infinite loops):
> src/soc/experiment/l0_cache.py 

not concerned

> src/soc/fu/compunits/test/test_logical_compunit.py 

probably again because of bpermd

if you re-run the logical tests multiple times they *should* succeed because ISACaller receives random values that fo not trigger its bug.

the multiply one *is* of concern and needs high priority investigation.

can you check first that it works under pysim?
Comment 2 Luke Kenneth Casson Leighton 2020-12-12 16:39:21 GMT
cesar please git pull and try again, madd had been added as a unit test and it does not exist.


commit 26e84dc5d35f80bd4a25e05ea94f7f3e3bd7a4c6 (HEAD -> master, origin/master)
Author: Luke Kenneth Casson Leighton <lkcl@lkcl.net>
Date:   Sat Dec 12 16:37:49 2020 +0000

    skip madd, not implemented
Comment 3 Cesar Strauss 2020-12-12 22:33:47 GMT
(In reply to Luke Kenneth Casson Leighton from comment #2)
> cesar please git pull and try again, madd had been added as a unit test and
> it does not exist.

Indeed, src/soc/fu/mul/test/test_pipe_caller.py now runs without problems.

> can you check first that it works under pysim?

To clarify, before running CXXSim, I ran all these tests under PySim, so that I could detect any differences. So, all the results you see here were actually made with PySim.

>> src/soc/fu/logical/test/test_pipe_caller.py

> if bpermd that is an ISACaller random fault

Yes, it seems to be the case.

>> 3) Tests that hangs (enters infinite loops):
>> src/soc/fu/compunits/test/test_logical_compunit.py 

> probably again because of bpermd

Maybe, I'm not sure. Merits investigating.

>> src/soc/simple/test/test_issuer.py

> if this is just the bpermd test that's fine

Again, I am not sure.

Here is the trace from test_issuer.py:

======================================================================
FAIL: run_all (__main__.TestRunner)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/cstrauss/src/soc/src/soc/simple/test/test_issuer.py", line 311, in run_all
    sim.run()
  File "/home/cstrauss/src/nmigen/nmigen/sim/core.py", line 168, in run
    while self.advance():
  File "/home/cstrauss/src/nmigen/nmigen/sim/core.py", line 159, in advance
    return self._engine.advance()
  File "/home/cstrauss/src/nmigen/nmigen/sim/pysim.py", line 272, in advance
    self._step()
  File "/home/cstrauss/src/nmigen/nmigen/sim/pysim.py", line 261, in _step
    process.run()
  File "/home/cstrauss/src/nmigen/nmigen/sim/_pycoro.py", line 123, in run
    self.coroutine.throw(exn)
  File "/home/cstrauss/src/nmigen/nmigen/sim/_pycoro.py", line 64, in run
    command = self.coroutine.send(response)
  File "/home/cstrauss/src/nmigen/nmigen/sim/core.py", line 93, in wrapper
    yield from process()
  File "/home/cstrauss/src/soc/src/soc/simple/test/test_issuer.py", line 259, in process
    yield from sim.execute_one()
  File "/home/cstrauss/src/soc/src/soc/decoder/isa/caller.py", line 572, in execute_one
    yield from self.call(opname)
  File "/home/cstrauss/src/soc/src/soc/decoder/isa/caller.py", line 723, in call
    results = info.func(self, *inputs)
  File "/home/cstrauss/src/soc/src/soc/decoder/isa/caller.py", line 831, in decorator
    result = func(*args, **kwargs)
  File "/home/cstrauss/src/soc/src/soc/decoder/isa/fixedlogical.py", line 310, in op_bpermd
    perm[i] = RB[index]
  File "/home/cstrauss/src/soc/src/soc/decoder/selectable_int.py", line 275, in __getitem__
    assert key < self.bits, "key %d accessing %d" % (key, self.bits)
AssertionError: key 128 accessing 64

----------------------------------------------------------------------
Ran 7 tests in 798.466s

FAILED (failures=1)
Comment 4 Luke Kenneth Casson Leighton 2020-12-12 22:39:00 GMT
(In reply to Cesar Strauss from comment #3)

>     assert key < self.bits, "key %d accessing %d" % (key, self.bits)
> AssertionError: key 128 accessing 64

yes, that's the one that's a bug in ISACaller, not the hardware.  50% of
tests try to do a key that is too large.  sigh.

so just keep repeating the test until the random input is below the point where the output causes this error, for now.
Comment 5 Luke Kenneth Casson Leighton 2020-12-12 22:40:11 GMT
(In reply to Cesar Strauss from comment #3)
> (In reply to Luke Kenneth Casson Leighton from comment #2)
> > cesar please git pull and try again, madd had been added as a unit test and
> > it does not exist.
> 
> Indeed, src/soc/fu/mul/test/test_pipe_caller.py now runs without problems.

excellent.  it was a unit test that should not have yet been added because we do not have madd implemented in hardware.

so - basically, it's all "good" despite looking "bad" :)
Comment 6 Jacob Lifshay 2020-12-13 03:06:29 GMT
(In reply to Luke Kenneth Casson Leighton from comment #5)
> (In reply to Cesar Strauss from comment #3)
> > (In reply to Luke Kenneth Casson Leighton from comment #2)
> > > cesar please git pull and try again, madd had been added as a unit test and
> > > it does not exist.
> > 
> > Indeed, src/soc/fu/mul/test/test_pipe_caller.py now runs without problems.
> 
> excellent.  it was a unit test that should not have yet been added because
> we do not have madd implemented in hardware.
> 
> so - basically, it's all "good" despite looking "bad" :)

I added madd since we will need it in the future. +1 to setting skip for now (I thought I had already done that... guess I forgot)
Comment 7 Cesar Strauss 2020-12-13 13:03:39 GMT
(In reply to Luke Kenneth Casson Leighton from comment #1)
> > src/soc/fu/logical/test/test_pipe_caller.py
> 
> if bpermd that is an ISACaller random fault 
  
> if you re-run the logical tests multiple times they *should* succeed because
> ISACaller receives random values that do not trigger its bug.

It took me 11 attempts to get a success, but I finally got it.
Comment 8 Luke Kenneth Casson Leighton 2020-12-13 15:32:17 GMT
(In reply to Cesar Strauss from comment #7)
> (In reply to Luke Kenneth Casson Leighton from comment #1)
> > > src/soc/fu/logical/test/test_pipe_caller.py
> > 
> > if bpermd that is an ISACaller random fault 
>   
> > if you re-run the logical tests multiple times they *should* succeed because
> > ISACaller receives random values that do not trigger its bug.
> 
> It took me 11 attempts to get a success, but I finally got it.

sorry :)  that would be the probability of certain bit patterns (bpermd)
staying out of the top range.

one of the intermediary numbers used in the pseudocode is not the right
bitsize, but unfortunately it's created "automatically" (an intermediary).
it'll take some investigation to track down, so i left it for now.
Comment 9 Luke Kenneth Casson Leighton 2021-05-18 13:16:09 BST
(In reply to Cesar Strauss from comment #7)
> (In reply to Luke Kenneth Casson Leighton from comment #1)
> > > src/soc/fu/logical/test/test_pipe_caller.py
> > 
> > if bpermd that is an ISACaller random fault 
>   
> > if you re-run the logical tests multiple times they *should* succeed because
> > ISACaller receives random values that do not trigger its bug.
> 
> It took me 11 attempts to get a success, but I finally got it.

i finally fixed the pseudo-code, it was an 8 bit number that should
be treated as unsigned: when the value read was 128 or greater it was
being considered negative.


index 99d5505..c574911 100644
--- a/openpower/isa/fixedlogical.mdwn
+++ b/openpower/isa/fixedlogical.mdwn
@@ -468,7 +468,7 @@ Pseudo-code:
     perm <- [0] * 8
     for i = 0 to 7
        index <- (RS)[8*i:8*i+7]
-       if index < 64 then
+       if index <u 64 then
             perm[i] <- (RB)[index]
        else
             perm[i] <- 0
Comment 10 Cesar Strauss 2021-05-18 14:22:18 BST
It works!

I'm closing the bug.