[libre-riscv-dev] [Bug 318] fix LDSTCompUnit

[bugzilla-daemon at libre-soc.org]

https://bugs.libre-soc.org/show_bug.cgi?id=318

--- Comment #11 from Luke Kenneth Casson Leighton <lkcl at lkcl.net> ---
(In reply to Cesar Strauss from comment #10)
> I got my development environment running, by consulting
> https://libre-soc.org/HDL_workflow/.
> 
> Then, I ran the testbench at experiment/compldst_multi.py, generated the
> timing diagram, and confirmed in GTKWave that the problem related in comment
> #3 does not occur any longer in the latest git master.
> 
> I started reading the reference material, in order to get a basic knowledge
> of the functionalities and interfaces.
> 
> Then, I followed the nMigen tutorial at
> https://github.com/RobertBaruch/nmigen-tutorial, while reading the code of
> the LD/ST Comp Unit itself, and as a result I got a reasonable understanding
> of both.

excellent.  similar to https://bugs.libre-soc.org/show_bug.cgi?id=316
if you'd like we can find a practical and useful example that's an awful
lot less complicated than this one, by way of getting started.


> I'll now go back to the reference material a little more. Once I get a
> better understanding, I'll return to the code and proceed with reviewing it
> and improving the unit tests. I have formed some thoughts about this already.

me too.  please, when you have a moment, do describe them.  here's a couple:


* we do need a way to test dual ports (two CompLDSTs with
  two ports, 4-4 as well) which write to the same memory

  this will test whether the (prototype) L0CacheBuffer is doing
  its job (which i am not certain that it does.  it handles _one_
  LD or _one_ ST correctly)

* we need some tests to be delayed-response (to each of the rd and wr
  *and* go.ad *and* go.st) and some to be immediate-response

these can be done using multiple "processes".  you notice in the
simulation that a function is passed in: it is possible to pass in
*multiple* such functions.  hmm hmm where's an example of that...

https://git.libre-soc.org/?p=nmutil.git;a=blob;f=src/nmutil/test/test_inout_mux_pipe.py;hb=HEAD

what that does is: set up a pipeline with a 4-input, 4-output "muxer"
at the input and output.  a context (index) is passed down the pipeline,
and used to associate the "result" back with its corresponding output.

so you can see in InputTest, there are *eight* separate and distinct
simultaneously-operating "processes": 4 do sending, 4 do receiving.

they all operate on the same (global) python data structure, which gives
a way for the python unit test to actually check that they're doing the
right thing.

a) that no data is lost
b) that data sent from input N is *received* at output N
c) that the order of any data sent from input N to output N is preserved
   regardless of the interleaving order.

so in a similar fashion, a class could be designed where one function
takes care of rd.req[N] and rd.go[N] acknowledgement (for N=0..2),
and likewise another function responds to ad.req, another to wr.req[0..1]
and so on, where because those "processes" are all members of the same
class, they have access to the full "state" of all other "processes" and
consequently can verify and report that the request-signals are coming
in correctly (or at all), and in the right order.

in addition, under the control of the caller (the unit test), those
individual processes could be told "respond immediately to the signal"
or "response with an arbitrary delay" extremely easily: a simple for-loop
with a "yield" in it - as you can see at line 101 in test_inout_mux_pipe.py
will create that type of behaviour.


beyond that: in a similar way to InputTest, setting up multiple CompLDSTUnits,
and getting them to hammer the same L0CacheBuffer via multiple ports, is also a
very important test, as it is the way to ensure that multiple LD/ST operations
can operate in parallel and not corrupt memory (or be lost).

beyond *that*, at some point we need to also arbitrarily test raising an
"exception" flag, which will result in the "shadow" condition being pulled,
and that should result in the complete cancellation of the operation.


> I'll ask if I need further clarifications, and report my progress.

appreciated.

-- 
You are receiving this mail because:
You are on the CC list for the bug.