Bug 448 - MUL pipeline unit tests
Summary: MUL pipeline unit tests
Status: IN_PROGRESS
Alias: None
Product: Libre-SOC's first SoC
Classification: Unclassified
Component: Source Code (show other bugs)
Version: unspecified
Hardware: PC Linux
: High normal
Assignee: Cole Poirier
URL:
Depends on:
Blocks: 383 323
  Show dependency treegraph
 
Reported: 2020-08-04 22:38 BST by Cole Poirier
Modified: 2020-08-18 13:51 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 Cole Poirier 2020-08-04 22:38:11 BST
MUL pipeline needs unit tests similar to the 5-deep nested loop test Jacob wrote for the DIV pipeline.

https://git.libre-soc.org/?p=soc.git;a=blob;f=src/soc/fu/div/test/test_pipe_caller_long.py;hb=HEAD
Comment 1 Cole Poirier 2020-08-04 22:41:17 BST
Luke, you said I should run through a list of opcodes. Can this list of opcodes be found in the decoder? I started by looking in the 3.1B OPENPOWER spec, but there are lots of other variations such as "multiply low" "multiply high" there that I don't see in Jacob's test. I think that extracting each opcode individually *manually* from the PDF spec is going to be error prone, so I assume there's a different approach you would recommend?
Comment 2 Luke Kenneth Casson Leighton 2020-08-04 22:48:36 BST
(In reply to Cole Poirier from comment #1)
> Luke, you said I should run through a list of opcodes. 

yes.  look in the "long" div test.

> Can this list of opcodes be found in the decoder?

no - in the spec.

> I started by looking in the 3.1B OPENPOWER
> spec, but there are lots of other variations such as "multiply low"
> "multiply high" there that I don't see in Jacob's test.

exactly.

> I think that
> extracting each opcode individually *manually* from the PDF spec is going to
> be error prone, 

then use the text file from the wiki.

> so I assume there's a different approach you would recommend?

no.
Comment 3 Jacob Lifshay 2020-08-04 22:49:55 BST
I may have missed some, but the complete list of instructions built into power-instruction-analyzer is accessible as:

>>> print(power_instruction_analyzer.INSTRS)
['divde', 'divdeo', 'divde.', 'divdeo.', 'divdeu', 'divdeuo', 'divdeu.', 'divdeuo.', 'divd', 'divdo', 'divd.', 'divdo.', 'divdu', 'divduo', 'divdu.', 'divduo.', 'divwe', 'divweo', 'divwe.', 'divweo.', 'divweu', 'divweuo', 'divweu.', 'divweuo.', 'divw', 'divwo', 'divw.', 'divwo.', 'divwu', 'divwuo', 'divwu.', 'divwuo.', 'modsd', 'modud', 'modsw', 'moduw', 'mullw', 'mullwo', 'mullw.', 'mullwo.', 'mulhw', 'mulhw.', 'mulhwu', 'mulhwu.', 'mulld', 'mulldo', 'mulld.', 'mulldo.', 'mulhd', 'mulhd.', 'mulhdu', 'mulhdu.', 'maddhd', 'maddhdu', 'maddld']
Comment 4 Cole Poirier 2020-08-04 23:34:34 BST
(In reply to Luke Kenneth Casson Leighton from comment #2) 
> then use the text file from the wiki.

This is the right approach, the method I was using was looking in the generated fixedarith.py, by looking at the wiki I got the MUL instructions that didn't start with mulxxx i.e. maddxxx.

In order to set the test up to run, jacob created a helper.py file, this doesn't exist in the MUL test dir. Jacob also created a DivPipeKind enum because of the different DIV pipeline implementations. Overall I'm not sure how to properly use the api of the pipelines to write this test. If I commit what I have can you help me with this?
Comment 5 Cole Poirier 2020-08-04 23:35:05 BST
(In reply to Jacob Lifshay from comment #3)
> I may have missed some, but the complete list of instructions built into
> power-instruction-analyzer is accessible as:
> 
> >>> print(power_instruction_analyzer.INSTRS)
> ['divde', 'divdeo', 'divde.', 'divdeo.', 'divdeu', 'divdeuo', 'divdeu.',
> 'divdeuo.', 'divd', 'divdo', 'divd.', 'divdo.', 'divdu', 'divduo', 'divdu.',
> 'divduo.', 'divwe', 'divweo', 'divwe.', 'divweo.', 'divweu', 'divweuo',
> 'divweu.', 'divweuo.', 'divw', 'divwo', 'divw.', 'divwo.', 'divwu',
> 'divwuo', 'divwu.', 'divwuo.', 'modsd', 'modud', 'modsw', 'moduw', 'mullw',
> 'mullwo', 'mullw.', 'mullwo.', 'mulhw', 'mulhw.', 'mulhwu', 'mulhwu.',
> 'mulld', 'mulldo', 'mulld.', 'mulldo.', 'mulhd', 'mulhd.', 'mulhdu',
> 'mulhdu.', 'maddhd', 'maddhdu', 'maddld']

Thanks Jacob, very helpful!
Comment 6 Luke Kenneth Casson Leighton 2020-08-05 00:27:03 BST
(In reply to Cole Poirier from comment #4)
> (In reply to Luke Kenneth Casson Leighton from comment #2) 
> > then use the text file from the wiki.
> 
> This is the right approach, the method I was using was looking in the
> generated fixedarith.py, by looking at the wiki I got the MUL instructions
> that didn't start with mulxxx i.e. maddxxx.
> 
> In order to set the test up to run, jacob created a helper.py file,

irrelevant.

>  this
> doesn't exist in the MUL test dir. Jacob also created a DivPipeKind enum

irrelevant.

> because of the different DIV pipeline implementations. 

irrelevant.

> Overall I'm not sure
> how to properly use the api of the pipelines to write this test. 

irrelevant.

> If I commit
> what I have can you help me with this?

it's real simple.

from this file:

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

at this line number:

https://git.libre-soc.org/?p=soc.git;a=blob;f=src/soc/fu/div/test/test_pipe_caller_long.py;hb=HEAD#l11

copy that function

adapt it to do "mul" instead of "div".

add it to this file:

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

that's all that's needed.

nothing more, nothing less.
Comment 7 Cole Poirier 2020-08-05 01:04:05 BST
(In reply to Luke Kenneth Casson Leighton from comment #6)
> (In reply to Cole Poirier from comment #4)
> > (In reply to Luke Kenneth Casson Leighton from comment #2) 
> > > then use the text file from the wiki.
> > 
> > This is the right approach, the method I was using was looking in the
> > generated fixedarith.py, by looking at the wiki I got the MUL instructions
> > that didn't start with mulxxx i.e. maddxxx.
> > 
> > In order to set the test up to run, jacob created a helper.py file,
> 
> irrelevant.
> 
> >  this
> > doesn't exist in the MUL test dir. Jacob also created a DivPipeKind enum
> 
> irrelevant.
> 
> > because of the different DIV pipeline implementations. 
> 
> irrelevant.
> 
> > Overall I'm not sure
> > how to properly use the api of the pipelines to write this test. 
> 
> irrelevant.
> 
> > If I commit
> > what I have can you help me with this?
> 
> it's real simple.
> 
> from this file:
> 
> https://git.libre-soc.org/?p=soc.git;a=blob;f=src/soc/fu/div/test/
> test_pipe_caller_long.py;hb=HEAD
> 
> at this line number:
> 
> https://git.libre-soc.org/?p=soc.git;a=blob;f=src/soc/fu/div/test/
> test_pipe_caller_long.py;hb=HEAD#l11
> 
> copy that function
> 
> adapt it to do "mul" instead of "div".
> 
> add it to this file:
> 
> https://git.libre-soc.org/?p=soc.git;a=blob;f=src/soc/fu/mul/test/
> test_pipe_caller.py;hb=HEAD
> 
> that's all that's needed.
> 
> nothing more, nothing less.

Thanks! That is absolutely dead simple, very helpful!
Comment 8 Cole Poirier 2020-08-05 01:15:32 BST
Pushed my commit. However, getting a weird cryptic error:

```
{standard input}: Assembler messages:
{standard input}:1: Error: missing operand
Error in program:
maddhd 3, 1, 2

Traceback (most recent call last):
  File "/home/colepoirier/src/soc/src/soc/simulator/program.py", line 86, in _assemble
    sys.exit(1)
SystemExit: 1

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "test_pipe_caller.py", line 330, in <module>
    suite.addTest(TestRunner(MulTestCase().test_data))
  File "/home/colepoirier/src/soc/src/soc/fu/test/common.py", line 84, in __init__
    v(self)
  File "test_pipe_caller.py", line 197, in case_all
    with Program(l, bigendian) as prog:
  File "/home/colepoirier/src/soc/src/soc/simulator/program.py", line 46, in __init__
    self._assemble()
  File "/home/colepoirier/src/soc/src/soc/simulator/program.py", line 87, in _assemble
    self._link(outfile)
  File "/usr/lib/python3.7/tempfile.py", line 639, in __exit__
    self.close()
  File "/usr/lib/python3.7/tempfile.py", line 646, in close
    self._closer.close()
  File "/usr/lib/python3.7/tempfile.py", line 583, in close
    unlink(self.name)
FileNotFoundError: [Errno 2] No such file or directory: '/tmp/tmpe8odpld7.o'
```
Comment 9 Luke Kenneth Casson Leighton 2020-08-05 01:18:42 BST
(In reply to Cole Poirier from comment #8)
> Pushed my commit. However, getting a weird cryptic error:
> 
> ```
> {standard input}: Assembler messages:
> {standard input}:1: Error: missing operand
> Error in program:
> maddhd 3, 1, 2

look at the definition for this operation.  count the number of operands.
Comment 10 Cole Poirier 2020-08-05 01:35:39 BST
(In reply to Luke Kenneth Casson Leighton from comment #9)
> (In reply to Cole Poirier from comment #8)
> > Pushed my commit. However, getting a weird cryptic error:
> > 
> > ```
> > {standard input}: Assembler messages:
> > {standard input}:1: Error: missing operand
> > Error in program:
> > maddhd 3, 1, 2
> 
> look at the definition for this operation.  count the number of operands.

ah that one appears to take 4... Not so simple after all. I don't have an idea as to how to fix this.
Comment 11 Luke Kenneth Casson Leighton 2020-08-05 10:02:25 BST
(In reply to Cole Poirier from comment #10)

> > look at the definition for this operation.  count the number of operands.
> 
> ah that one appears to take 4... Not so simple after all. I don't have an
> idea as to how to fix this.

that's also very simple:

1) leave it out

2) put it in a separate test.
Comment 12 Cole Poirier 2020-08-05 17:35:36 BST
(In reply to Luke Kenneth Casson Leighton from comment #11)
> (In reply to Cole Poirier from comment #10)
> 
> > > look at the definition for this operation.  count the number of operands.
> > 
> > ah that one appears to take 4... Not so simple after all. I don't have an
> > idea as to how to fix this.
> 
> that's also very simple:
> 
> 1) leave it out
> 
> 2) put it in a separate test.

Sure... I'd like to understand how to create this separate unit test. But I'm not sure what the correct api is for adding registers, I looked in the other MUL and DIV tests and they are all two operand, otherwise I would have just used one such test as a template.
Comment 13 Luke Kenneth Casson Leighton 2020-08-05 17:51:32 BST
(In reply to Cole Poirier from comment #12)

> Sure... I'd like to understand how to create this separate unit test. But
> I'm not sure what the correct api is for adding registers, I looked in the
> other MUL and DIV tests and they are all two operand, otherwise I would have
> just used one such test as a template.

leave it.  the fact that we have to discuss it is impeding your progress
in committing a working test for everything else.
Comment 14 Cole Poirier 2020-08-05 18:09:18 BST
(In reply to Luke Kenneth Casson Leighton from comment #13)
> (In reply to Cole Poirier from comment #12)
> 
> > Sure... I'd like to understand how to create this separate unit test. But
> > I'm not sure what the correct api is for adding registers, I looked in the
> > other MUL and DIV tests and they are all two operand, otherwise I would have
> > just used one such test as a template.
> 
> leave it.  the fact that we have to discuss it is impeding your progress
> in committing a working test for everything else.

Makes sense, I commented it out and added a TODO. Unfortunately the test fails like this. Should I commit?

```
['RA']
reading reg 1
[SelectableInt(value=0x0, bits=64)]
concat 128 SelectableInt(value=0x0, bits=128)
abs 0
abs 0
SelectableInt mul 0x0 0x2 64 256
MULS SelectableInt(value=0x0, bits=320) False False
F
======================================================================
FAIL: run_all (__main__.TestRunner)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "test_pipe_caller.py", line 289, in run_all
    sim.run()
  File "/home/colepoirier/src/nmigen/nmigen/sim/pysim.py", line 382, in run
    while self.advance():
  File "/home/colepoirier/src/nmigen/nmigen/sim/pysim.py", line 371, in advance
    self._real_step()
  File "/home/colepoirier/src/nmigen/nmigen/sim/pysim.py", line 347, in _real_step
    process.run()
  File "/home/colepoirier/src/nmigen/nmigen/sim/_pycoro.py", line 120, in run
    self.coroutine.throw(exn)
  File "/home/colepoirier/src/nmigen/nmigen/sim/_pycoro.py", line 61, in run
    command = self.coroutine.send(response)
  File "/home/colepoirier/src/nmigen/nmigen/sim/pysim.py", line 265, in wrapper
    yield from process()
  File "test_pipe_caller.py", line 273, in process
    yield from sim.call(opname)
  File "/home/colepoirier/src/soc/src/soc/decoder/isa/caller.py", line 668, in call
    results = info.func(self, *inputs)
  File "/home/colepoirier/src/soc/src/soc/decoder/isa/caller.py", line 766, in decorator
    result = func(*args, **kwargs)
  File "/home/colepoirier/src/soc/src/soc/decoder/isa/fixedarith.py", line 280, in op_mulli
    prod[0:128] = MULS(RA, EXTS(SI))
  File "/home/colepoirier/src/soc/src/soc/decoder/selectable_int.py", line 311, in __setitem__
    assert value.bits == bits, "%d into %d" % (value.bits, bits)
AssertionError: 320 into 128

----------------------------------------------------------------------
Ran 1 test in 2.526s

FAILED (failures=1)
Comment 15 Luke Kenneth Casson Leighton 2020-08-05 18:13:44 BST
(In reply to Cole Poirier from comment #14)

> Makes sense, I commented it out and added a TODO. Unfortunately the test
> fails like this. Should I commit?

you shouldn't be asking, it should have been done already: you should
be *telling* me it's committed (2 days ago) within under 3 minutes.

2 days of uncommitted work is a major "red flag" because during all
that time absolutely nobody can look at it or comment on it.

the only reason i say this however is because unit tests are *SPECIFICALLY*
isolated from all other tests and are consequently NOT on a critical code
path that could prevent or prohibit anyone else from doing any work.
Comment 16 Cole Poirier 2020-08-05 18:21:22 BST
(In reply to Luke Kenneth Casson Leighton from comment #15)
> (In reply to Cole Poirier from comment #14)
> 
> > Makes sense, I commented it out and added a TODO. Unfortunately the test
> > fails like this. Should I commit?
> 
> you shouldn't be asking, it should have been done already: you should
> be *telling* me it's committed (2 days ago) within under 3 minutes.
> 
> 2 days of uncommitted work is a major "red flag" because during all
> that time absolutely nobody can look at it or comment on it.
> 
> the only reason i say this however is because unit tests are *SPECIFICALLY*
> isolated from all other tests and are consequently NOT on a critical code
> path that could prevent or prohibit anyone else from doing any work.

Thanks for the clarification. I'm still far too afraid of introducing things that break things, so when I get errors I cower ;-)

Also, I committed the initial test where it failed due to madd* insns yesterday. So only a half failure there :)

Committed and pushed.
Comment 17 Luke Kenneth Casson Leighton 2020-08-05 19:48:26 BST
(In reply to Cole Poirier from comment #16)

> Thanks for the clarification. I'm still far too afraid of introducing things
> that break things, so when I get errors I cower ;-)

python unit test infrasttucture is specifically designed to be isolated.


> 
> Also, I committed the initial test where it failed due to madd* insns
> yesterday. So only a half failure there :)
> 
> Committed and pushed.

sorted.  git pull

diff --git a/src/soc/decoder/helpers.py b/src/soc/decoder/helpers.py
index 17534800..870ec4ab 100644
--- a/src/soc/decoder/helpers.py
+++ b/src/soc/decoder/helpers.py
@@ -4,6 +4,7 @@ from nmutil.divmod import trunc_divs, trunc_rems
 from operator import floordiv, mod
 from soc.decoder.selectable_int import selectltu as ltu
 from soc.decoder.selectable_int import selectgtu as gtu
+from soc.decoder.selectable_int import check_extsign
 
 trunc_div = floordiv
 trunc_rem = mod
@@ -37,6 +38,9 @@ def EXTS64(value):
 
 # signed version of MUL
 def MULS(a, b):
+    if isinstance(b, int):
+        b = SelectableInt(b, self.bits)
+    b = check_extsign(self, b)
     a_s = a.value & (1 << (a.bits-1)) != 0
     b_s = b.value & (1 << (b.bits-1)) != 0
     result = abs(a) * abs(b)
Comment 18 Cole Poirier 2020-08-05 20:28:31 BST
(In reply to Luke Kenneth Casson Leighton from comment #17)
> (In reply to Cole Poirier from comment #16)
> 
> > Thanks for the clarification. I'm still far too afraid of introducing things
> > that break things, so when I get errors I cower ;-)
> 
> python unit test infrasttucture is specifically designed to be isolated.

Happy to be challenging myself again by engaging with you on the codebase, it's a pleasant discomfort of learning new things :)
 
> > Also, I committed the initial test where it failed due to madd* insns
> > yesterday. So only a half failure there :)
> > 
> > Committed and pushed.
> 
> sorted.  git pull
> 
> diff --git a/src/soc/decoder/helpers.py b/src/soc/decoder/helpers.py
> index 17534800..870ec4ab 100644
> --- a/src/soc/decoder/helpers.py
> +++ b/src/soc/decoder/helpers.py
> @@ -4,6 +4,7 @@ from nmutil.divmod import trunc_divs, trunc_rems
>  from operator import floordiv, mod
>  from soc.decoder.selectable_int import selectltu as ltu
>  from soc.decoder.selectable_int import selectgtu as gtu
> +from soc.decoder.selectable_int import check_extsign
>  
>  trunc_div = floordiv
>  trunc_rem = mod
> @@ -37,6 +38,9 @@ def EXTS64(value):
>  
>  # signed version of MUL
>  def MULS(a, b):
> +    if isinstance(b, int):
> +        b = SelectableInt(b, self.bits)
> +    b = check_extsign(self, b)
>      a_s = a.value & (1 << (a.bits-1)) != 0
>      b_s = b.value & (1 << (b.bits-1)) != 0
>      result = abs(a) * abs(b)

Thanks Luke! What should I do next?
Comment 19 Luke Kenneth Casson Leighton 2020-08-05 21:31:32 BST
(In reply to Cole Poirier from comment #18)

> Thanks Luke! What should I do next?

https://git.libre-soc.org/?p=soc.git;a=blob;f=src/soc/fu/mul/test/test_pipe_caller.py;h=3274f012794ffa6af1ee581a316805cdb7004ee6;hb=fb07d936ebbe71a4c1c9d67075b65955d8137dfc#l177

extend that list to include at least
0xffffffff
0x7fffffff
0x80000000
0xfffffffe
0xfffffffd

these are in 32 bit close to the boundaries.

also use that exact same list but this time use it for RA but for RB use a *random* value.

also it might be worth looking at riscv isa tests to see what values they use.

we also need that one where close to overflow is calculated by selecting an result in advance, divide by a selected RA (in python that is) and put *that* as RB.
Comment 20 Jacob Lifshay 2020-08-05 21:38:58 BST
(In reply to Luke Kenneth Casson Leighton from comment #19)
> (In reply to Cole Poirier from comment #18)
> 
> > Thanks Luke! What should I do next?
> 
> https://git.libre-soc.org/?p=soc.git;a=blob;f=src/soc/fu/mul/test/test_pipe_caller.py;h=3274f012794ffa6af1ee581a316805cdb7004ee6;hb=fb07d936ebbe71a4c1c9d67075b65955d8137dfc#l177
> 
> extend that list to include at least

the last 4 values in the list in soc.git are close to 32-bit boundaries when truncated to 32-bits.

> 0xffffffff
> 0x7fffffff
> 0x80000000
> 0xfffffffe
> 0xfffffffd

only the last 2 are new when interpreting as 32-bit values.
Comment 21 Jacob Lifshay 2020-08-05 21:40:27 BST
(In reply to Jacob Lifshay from comment #20)
> (In reply to Luke Kenneth Casson Leighton from comment #19)
> > (In reply to Cole Poirier from comment #18)
> > 
> > > Thanks Luke! What should I do next?
> > 
> > https://git.libre-soc.org/?p=soc.git;a=blob;f=src/soc/fu/mul/test/test_pipe_caller.py;h=3274f012794ffa6af1ee581a316805cdb7004ee6;hb=fb07d936ebbe71a4c1c9d67075b65955d8137dfc#l177
> > 
> > extend that list to include at least
> 
> the last 4 values in the list in soc.git are close to 32-bit boundaries when
> truncated to 32-bits.
> 
> > 0xffffffff
> > 0x7fffffff
> > 0x80000000
> > 0xfffffffe
> > 0xfffffffd
> 
> only the last 2 are new when interpreting as 32-bit values.

0xfffffffe is also already in soc.git when truncating to 32-bit values.
Comment 22 Cole Poirier 2020-08-05 21:50:10 BST
(In reply to Jacob Lifshay from comment #21)
> (In reply to Jacob Lifshay from comment #20)
> > (In reply to Luke Kenneth Casson Leighton from comment #19)
> > > (In reply to Cole Poirier from comment #18)
> > > 
> > > > Thanks Luke! What should I do next?
> > > 
> > > https://git.libre-soc.org/?p=soc.git;a=blob;f=src/soc/fu/mul/test/test_pipe_caller.py;h=3274f012794ffa6af1ee581a316805cdb7004ee6;hb=fb07d936ebbe71a4c1c9d67075b65955d8137dfc#l177
> > > 
> > > extend that list to include at least
> > 
> > the last 4 values in the list in soc.git are close to 32-bit boundaries when
> > truncated to 32-bits.
> > 
> > > 0xffffffff
> > > 0x7fffffff
> > > 0x80000000
> > > 0xfffffffe
> > > 0xfffffffd
> > 
> > only the last 2 are new when interpreting as 32-bit values.
> 
> 0xfffffffe is also already in soc.git when truncating to 32-bit values.

So, which ones do I actually need to add?
Comment 23 Cole Poirier 2020-08-05 21:58:51 BST
(In reply to Luke Kenneth Casson Leighton from comment #19)
> also use that exact same list but this time use it for RA but for RB use a
> *random* value.

Done. Pushed.

> also it might be worth looking at riscv isa tests to see what values they
> use.

Will take a look at this while I wait for clarification of the above values to be added.

> we also need that one where close to overflow is calculated by selecting an
> result in advance, divide by a selected RA (in python that is) and put
> *that* as RB.

I'm sorry I don't understand what you're directing me to do here, can you elaborate?
Comment 24 Jacob Lifshay 2020-08-05 22:00:36 BST
(In reply to Cole Poirier from comment #22)
> So, which ones do I actually need to add?

go ahead and add them all, it never hurts to test more cases, even if they are partially redundant.
Comment 25 Cole Poirier 2020-08-05 22:03:46 BST
(In reply to Jacob Lifshay from comment #24)
> (In reply to Cole Poirier from comment #22)
> > So, which ones do I actually need to add?
> 
> go ahead and add them all, it never hurts to test more cases, even if they
> are partially redundant.

Gotcha, done, thanks Jacob :)
Comment 26 Luke Kenneth Casson Leighton 2020-08-05 22:21:11 BST
i just realised, mulli needs to be taken out of this "all" unit test and done separately and specially.


the values need to be in the range that the constant SI can cope with.  that is -32768 to +32767.

however RA should be all those list of values 0x0000... etc
Comment 27 Cole Poirier 2020-08-05 22:23:57 BST
(In reply to Luke Kenneth Casson Leighton from comment #26)
> i just realised, mulli needs to be taken out of this "all" unit test and
> done separately and specially.
> 
> 
> the values need to be in the range that the constant SI can cope with.  that
> is -32768 to +32767.
> 
> however RA should be all those list of values 0x0000... etc

Done, added a TODO for this. In an earlier commit I added a TODO for madd* 3 operand isns test.
Comment 28 Luke Kenneth Casson Leighton 2020-08-05 22:26:13 BST
(In reply to Jacob Lifshay from comment #21)

> 0xfffffffe is also already in soc.git when truncating to 32-bit values.

yeessumm... it is but it isn't quite the same.  *we* know that some of the muls will truncate but it's not quite safe to assume that.

one of the deep joys of corner/offbyone cases
Comment 29 Luke Kenneth Casson Leighton 2020-08-05 22:29:35 BST
(In reply to Cole Poirier from comment #27)

> Done, added a TODO for this.

good idea

> In an earlier commit I added a TODO for madd* 3
> operand isns test.

we have to implement madd first.
Comment 30 Cole Poirier 2020-08-05 22:32:45 BST
(In reply to Luke Kenneth Casson Leighton from comment #29)
> (In reply to Cole Poirier from comment #27)
> 
> > Done, added a TODO for this.
> 
> good idea
> 
> > In an earlier commit I added a TODO for madd* 3
> > operand isns test.
> 
> we have to implement madd first.

Right, do you want me to do that?

Otherwise, which unit test should I write next? 

This one? "we also need that one where close to overflow is calculated by selecting an result in advance, divide by a selected RA (in python that is) and put *that* as RB."

or this one?

"i just realised, mulli needs to be taken out of this "all" unit test and done separately and specially.
the values need to be in the range that the constant SI can cope with.  that is -32768 to +32767.
however RA should be all those list of values 0x0000... etc"

or should I try adding some tests from the riscv unit tests?
Comment 31 Luke Kenneth Casson Leighton 2020-08-05 22:42:47 BST
(In reply to Cole Poirier from comment #23)

> I'm sorry I don't understand what you're directing me to do here, can you
> elaborate?

x = 0x7fffffff # plus some small randomness

ra = random value

rb = x // ra # integer divide

put ra and rb into regs 1 and 2

very simple.

what this is testing is: *when* you multiply 2 numbers that *might* overflow, does it overflow, yes or no.

if you tried with just a random ra and a random rb you would never hit 0x7ffffff not with millions of tests.

so calculate it.
Comment 32 Luke Kenneth Casson Leighton 2020-08-05 22:44:20 BST
(In reply to Cole Poirier from comment #30)

> > we have to implement madd first.
> 
> Right, do you want me to do that?

no, do tests first.  also see if it ia in microwatt.  if so raise separate bugreport.

> Otherwise, which unit test should I write next? 

pick one.
Comment 33 Cole Poirier 2020-08-05 22:56:46 BST
(In reply to Luke Kenneth Casson Leighton from comment #31)
> (In reply to Cole Poirier from comment #23)
> 
> > I'm sorry I don't understand what you're directing me to do here, can you
> > elaborate?
> 
> x = 0x7fffffff # plus some small randomness
> 
> ra = random value
> 
> rb = x // ra # integer divide
> 
> put ra and rb into regs 1 and 2
> 
> very simple.
> 
> what this is testing is: *when* you multiply 2 numbers that *might*
> overflow, does it overflow, yes or no.
> 
> if you tried with just a random ra and a random rb you would never hit
> 0x7ffffff not with millions of tests.
> 
> so calculate it.

Done :)
Comment 34 Cole Poirier 2020-08-05 22:57:58 BST
(In reply to Luke Kenneth Casson Leighton from comment #32)
> (In reply to Cole Poirier from comment #30)
> 
> > > we have to implement madd first.
> > 
> > Right, do you want me to do that?
> 
> no, do tests first.  also see if it ia in microwatt.  if so raise separate
> bugreport.
> 
> > Otherwise, which unit test should I write next? 
> 
> pick one.

Am I able to implement the mulli special case test from just this?

"the values need to be in the range that the constant SI can cope with.  that is -32768 to +32767.
however RA should be all those list of values 0x0000... etc"
Comment 35 Luke Kenneth Casson Leighton 2020-08-05 23:37:44 BST
(In reply to Cole Poirier from comment #33)

> > if you tried with just a random ra and a random rb you would never hit
> > 0x7ffffff not with millions of tests.
> > 
> > so calculate it.
> 
> Done :)

great, will look tomorrow.

(In reply to Cole Poirier from comment #34)

> Am I able to implement the mulli special case test from just this?
> 
> "the values need to be in the range that the constant SI can cope with. 
> that is -32768 to +32767.
> however RA should be all those list of values 0x0000... etc"

yes.  however also add -32768, -32767, -2, -1, 0, 1, 2, 32766, 32767 as well as a few random values to the SI list.
Comment 36 Cole Poirier 2020-08-05 23:39:19 BST
(In reply to Luke Kenneth Casson Leighton from comment #35)
> (In reply to Cole Poirier from comment #33)
> > Done :)
> 
> great, will look tomorrow.

Cool. 


> (In reply to Cole Poirier from comment #34)
> 
> > Am I able to implement the mulli special case test from just this?
> > 
> > "the values need to be in the range that the constant SI can cope with. 
> > that is -32768 to +32767.
> > however RA should be all those list of values 0x0000... etc"
> 
> yes.  however also add -32768, -32767, -2, -1, 0, 1, 2, 32766, 32767 as well
> as a few random values to the SI list.

Helpful, will do.
Comment 37 Luke Kenneth Casson Leighton 2020-08-06 12:36:41 BST
https://git.libre-soc.org/?p=soc.git;a=blob;f=src/soc/fu/mul/test/test_pipe_caller.py;hb=HEAD#l274

so... ra and rb are set once and then the exact same values issued to each instruction?  that doesn't sound useful, does it?

in addition: rb is in the range 0 to 2^64 while result is in the range 0 to 2^31.

what is the probability that a small random number divided by a massively large random number will be anything other than zero?

programming is about walking through the code and modelling its behaviour in your head.

if you can't model it, you run it and explore the results.

did you run this test and inspect the results?

you would have seen every single instruction had rb=0, which is not the intent, is it?
Comment 38 Cole Poirier 2020-08-06 18:29:34 BST
(In reply to Luke Kenneth Casson Leighton from comment #37)
> https://git.libre-soc.org/?p=soc.git;a=blob;f=src/soc/fu/mul/test/
> test_pipe_caller.py;hb=HEAD#l274
> 
> so... ra and rb are set once and then the exact same values issued to each
> instruction?  that doesn't sound useful, does it?
> 
> in addition: rb is in the range 0 to 2^64 while result is in the range 0 to
> 2^31.
> 
> what is the probability that a small random number divided by a massively
> large random number will be anything other than zero?

Very, very helpful, I now much better understand how to write this test. Just committed a new version, please provide further feedback :)


> programming is about walking through the code and modelling its behaviour in
> your head.
> 
> if you can't model it, you run it and explore the results.
> 
> did you run this test and inspect the results?
> 
> you would have seen every single instruction had rb=0, which is not the
> intent, is it?

Agreed, I just didn't understand it sufficiently to be able to do this, but these comments bring me closer to understanding. This was my intention. Commit what I understood I should do, and wait for better understanding to come through your corrections.
Comment 39 Luke Kenneth Casson Leighton 2020-08-06 18:43:21 BST
(In reply to Cole Poirier from comment #38)

>  Just committed a new version, please provide further feedback :)

i would be able to do so if you had done "git push"...

> > you would have seen every single instruction had rb=0, which is not the
> > intent, is it?
> 
> Agreed, I just didn't understand it sufficiently to be able to do this, but
> these comments bring me closer to understanding. This was my intention.
> Commit what I understood I should do, and wait for better understanding to
> come through your corrections.

now all we need to do is have _you_ think through the implications of what you're doing rather than asking me to do it!

there are two absolutely crucial aspects to team programming:

1) communication - constant feedback and discussion
2) modelling and debugging: feedback on the *code*.

the first of these you're doing extremely well.  the latter you've not yet got the idea that *you* need to think through, from words/thoughts and abstract design concepts, translating that into step-by-step code.

the expectation is therefore in this case that *each* instruction will need testing with a series of random values targetting the overflow range.  that therefore implies a *two* level nested for-loop:

outer loop: instruction
inner loop: 20 (or so)
execution: *NOW* we create random values targetting close to the overflow range.
Comment 40 Cole Poirier 2020-08-06 18:52:57 BST
(In reply to Luke Kenneth Casson Leighton from comment #39)
> (In reply to Cole Poirier from comment #38)
> 
> >  Just committed a new version, please provide further feedback :)
> 
> i would be able to do so if you had done "git push"...
> 
> > > you would have seen every single instruction had rb=0, which is not the
> > > intent, is it?
> > 
> > Agreed, I just didn't understand it sufficiently to be able to do this, but
> > these comments bring me closer to understanding. This was my intention.
> > Commit what I understood I should do, and wait for better understanding to
> > come through your corrections.
> 
> now all we need to do is have _you_ think through the implications of what
> you're doing rather than asking me to do it!

That's what's happening now, I really wasn't capable of doing this prior. The next test I *will* be able to think through it my self as now I actually understand what's going on :)

> there are two absolutely crucial aspects to team programming:
> 
> 1) communication - constant feedback and discussion
> 2) modelling and debugging: feedback on the *code*.
> 
> the first of these you're doing extremely well.  the latter you've not yet
> got the idea that *you* need to think through, from words/thoughts and
> abstract design concepts, translating that into step-by-step code.

Got it now, looked through the results of the test this time, and they seem to be doing what they're supposed to do.

> the expectation is therefore in this case that *each* instruction will need
> testing with a series of random values targetting the overflow range.  that
> therefore implies a *two* level nested for-loop:
> 
> outer loop: instruction
> inner loop: 20 (or so)
> execution: *NOW* we create random values targetting close to the overflow
> range.

Indeed, thank you, I am *really* feeling like I have a new fundamental understanding of how the tests work!
Comment 41 Luke Kenneth Casson Leighton 2020-08-06 18:59:35 BST
        test_values = [-32768, -32767, -32766, -2, -1, 0, 1, 2, 32766, 32767, random.randint(-1 << 15, (1 << 15)-1), random.randint(-1 << 15, (
            1 << 15) - 1), random.randint(-1 << 15, (1 << 15)-1), random.randint(-1 << 15, (1 << 15)-1), random.randint(-1 << 15, (1 << 15)-1)]



80 chars per line please.  limit the editor window to exactly 80 chars and
there will not be a problem.

also before committing do "git diff" - in an 80 char xterm - and again
you will immediately spot which lines "wrap" because along the left hand
side there will be spaces or characters instead of "+" or "-".
Comment 42 Cole Poirier 2020-08-06 19:03:17 BST
(In reply to Luke Kenneth Casson Leighton from comment #41)
>         test_values = [-32768, -32767, -32766, -2, -1, 0, 1, 2, 32766,
> 32767, random.randint(-1 << 15, (1 << 15)-1), random.randint(-1 << 15, (
>             1 << 15) - 1), random.randint(-1 << 15, (1 << 15)-1),
> random.randint(-1 << 15, (1 << 15)-1), random.randint(-1 << 15, (1 << 15)-1)]
> 
> 
> 
> 80 chars per line please.  limit the editor window to exactly 80 chars and
> there will not be a problem.
> 
> also before committing do "git diff" - in an 80 char xterm - and again
> you will immediately spot which lines "wrap" because along the left hand
> side there will be spaces or characters instead of "+" or "-".

Yes I'm having trouble with this, I set my max line length to 80 chars, and ran autopep8 on it but I end up having the above atrocious formatting? Isn't autopep8 supposed to enforce 80 chars? I *can* manually review *every* time in an 80 char wide xterm, but would rather have my tools work properly. Please help.
Comment 43 Luke Kenneth Casson Leighton 2020-08-06 19:07:00 BST
(In reply to Cole Poirier from comment #42)

> Yes I'm having trouble with this, I set my max line length to 80 chars, and
> ran autopep8 on it but I end up having the above atrocious formatting?

then you should have spotted that in the "git diff" prior to commit and fixed it!

> Isn't autopep8 supposed to enforce 80 chars?

only if you set -a -a -a.

>  I *can* manually review *every* time in an 80 char wide xterm, 

yes.  do this, please, every time.  it's what is expected.  you cannot just "arbitrarily commit without thought and proper review".

in particular: tools often do "damage".  it's therefore critically important
to do a full review.

every single time.
Comment 44 Cole Poirier 2020-08-06 19:12:43 BST
(In reply to Luke Kenneth Casson Leighton from comment #43)
> (In reply to Cole Poirier from comment #42)
> 
> > Yes I'm having trouble with this, I set my max line length to 80 chars, and
> > ran autopep8 on it but I end up having the above atrocious formatting?
> 
> then you should have spotted that in the "git diff" prior to commit and
> fixed it!

Yes I *should* have, will do so going forwards.

> > Isn't autopep8 supposed to enforce 80 chars?
> 
> only if you set -a -a -a.

That perplexes me, I would think it should be the default. Oh well.

> >  I *can* manually review *every* time in an 80 char wide xterm, 
> 
> yes.  do this, please, every time.  it's what is expected.  you cannot just
> "arbitrarily commit without thought and proper review".
> 
> in particular: tools often do "damage".  it's therefore critically important
> to do a full review.
> 
> every single time.

Will do :)
Comment 45 Luke Kenneth Casson Leighton 2020-08-06 22:10:01 BST
https://git.libre-soc.org/?p=soc.git;a=blob;f=src/soc/fu/mul/test/test_pipe_caller.py;hb=HEAD#l290

blegh.  that's a mess, Cole.

instead, do a for loop appending one random value to a static list, each time.
Comment 46 Luke Kenneth Casson Leighton 2020-08-06 22:12:41 BST
  l = ["mulli 3, 1, 2"]

that is "please multiply register 1 by the constant 2, putting the result in register 3"

this is not what we want.
Comment 47 Cole Poirier 2020-08-06 23:54:36 BST
(In reply to Luke Kenneth Casson Leighton from comment #46)
>   l = ["mulli 3, 1, 2"]
> 
> that is "please multiply register 1 by the constant 2, putting the result in
> register 3"
> 
> this is not what we want.

Sorry that's what's in all the other tests, what is it that we actually want?
Comment 48 Cole Poirier 2020-08-06 23:58:33 BST
(In reply to Luke Kenneth Casson Leighton from comment #45)
> https://git.libre-soc.org/?p=soc.git;a=blob;f=src/soc/fu/mul/test/
> test_pipe_caller.py;hb=HEAD#l290
> 
> blegh.  that's a mess, Cole.
> 
> instead, do a for loop appending one random value to a static list, each
> time.

Like this?

def case_mulli(self):

    test_values = [-32768, -32767, -32766, -2, -1, 0, 1, 2, 32766, 32767]

    for i in range(20):
        test_values.append(random.randint(-1 << 15, (1 << 15)-1))
                   
    l = ["mulli 3, 1, 2"]
    for ra in test_values:
        for rb in test_values:
            initial_regs = [0] * 32
            initial_regs[1] = ra
            initial_regs[2] = rb
            # use "with" so as to close the files used
            with Program(l, bigendian) as prog:
                self.add_case(prog, initial_regs)
Comment 49 Luke Kenneth Casson Leighton 2020-08-07 00:26:19 BST
(In reply to Cole Poirier from comment #47)

> Sorry that's what's in all the other tests,

all the ones that take *registers*

> what is it that we actually want?

look in the 3.0B manual how mulli works (or the pseudocode in fixedarith.mdwn)

then also look in alu test or anywhere that "addi" is used.
Comment 50 Cole Poirier 2020-08-07 00:43:32 BST
(In reply to Luke Kenneth Casson Leighton from comment #49)
> (In reply to Cole Poirier from comment #47)
> 
> > Sorry that's what's in all the other tests,
> 
> all the ones that take *registers*


Ahhh right, it's an immediate op

> > what is it that we actually want?
> 
> look in the 3.0B manual how mulli works (or the pseudocode in
> fixedarith.mdwn)
> 
> then also look in alu test or anywhere that "addi" is used.

Very helpful, thanks.
Comment 51 Cole Poirier 2020-08-07 01:22:06 BST
I updated (and pushed) mulli to what I thought would be the way to do it but I know I'm still doing it wrong... because the tests only have register 'ra' and it's always zero. I don't know how to resolve this from looking at addi's use in the other unit tests.
Comment 52 Luke Kenneth Casson Leighton 2020-08-07 01:33:26 BST
(In reply to Cole Poirier from comment #51)
> I updated (and pushed) mulli to what I thought would be the way to do it but
> I know I'm still doing it wrong... because the tests only have register 'ra'
> and it's always zero. 

you're looking at the output and finding discrepancies from expected behaviour.  good.

> I don't know how to resolve this from looking at
> addi's use in the other unit tests.

 choice = random.choice(test_values)
 l = [f"mulli 1, 0, {choice}"]

this says, "multiply register 0 by the immediate X and put the result in register 1, doesn't it?

 initial_regs = [0] * 32
 initial_regs[1] = random.randint(-1 << 15, (1 << 15) - 1)
 initial_regs[2] = random.randint(-1 << 15, (1 << 15) - 1)

here, register 1 and 2 have been set to a random value.

what is register 0 set to by the previous line?

so why are you surprised to find that exactly what you have asked to be done is in fact being done?
Comment 53 Cole Poirier 2020-08-07 01:38:43 BST
(In reply to Luke Kenneth Casson Leighton from comment #52)
> (In reply to Cole Poirier from comment #51)
> > I updated (and pushed) mulli to what I thought would be the way to do it but
> > I know I'm still doing it wrong... because the tests only have register 'ra'
> > and it's always zero. 
> 
> you're looking at the output and finding discrepancies from expected
> behaviour.  good.

Learning.

> > I don't know how to resolve this from looking at
> > addi's use in the other unit tests.
> 
>  choice = random.choice(test_values)
>  l = [f"mulli 1, 0, {choice}"]
> 
> this says, "multiply register 0 by the immediate X and put the result in
> register 1, doesn't it?

Apparently that's what it says. Very helpful, I was struggling with this.

Going to go through point by point to confirm my understanding, below.

> 
>  initial_regs = [0] * 32

So this sets r0 through r31 to 0?

Registers are 64 bits?

>  initial_regs[1] = random.randint(-1 << 15, (1 << 15) - 1)
>  initial_regs[2] = random.randint(-1 << 15, (1 << 15) - 1)
> 
> here, register 1 and 2 have been set to a random value.

Right.

> what is register 0 set to by the previous line?

It is set to 0 because that's what all registers are initialized to.

> so why are you surprised to find that exactly what you have asked to be done
> is in fact being done?

Because I don't understand how to read it fluently yet! But I'm learning quite a lot just today.

Thanks so much Luke, I'm beginning to understand.
Comment 54 Luke Kenneth Casson Leighton 2020-08-07 01:43:51 BST
(In reply to Cole Poirier from comment #53)
> 
> Going to go through point by point to confirm my understanding, below.

this is programming.  one line at a time.  
> > 
> >  initial_regs = [0] * 32
> 
> So this sets r0 through r31 to 0?

it initialises an array which, when handed to ISACaller, does that, yes 
 
> Registers are 64 bits?

yyup.
 

> > what is register 0 set to by the previous line?
> 
> It is set to 0 because that's what all registers are initialized to.

and you asked for register 0, which was set to zero, to be RA.
Comment 55 Cole Poirier 2020-08-07 01:54:30 BST
(In reply to Luke Kenneth Casson Leighton from comment #54)
> (In reply to Cole Poirier from comment #53)

> this is programming.  one line at a time.  

Indeed.

> > >  initial_regs = [0] * 32
> > 
> > So this sets r0 through r31 to 0?
> 
> it initialises an array which, when handed to ISACaller, does that, yes 

Ah ISACaller, will be getting to know this module well.

> > Registers are 64 bits?
> 
> yyup.

I was 99% sure, but had a niggling doubt.

> > > what is register 0 set to by the previous line?
> > 
> > It is set to 0 because that's what all registers are initialized to.

Sorry, all registers are initialized to by the line "initial_regs = [0] * 32", well eventually as I now understand this is an array that specifies the initial values for ISACaller to set.

> and you asked for register 0, which was set to zero, to be RA.

Right, because:

mulli RT,RA,SI

and I asked for "mulli 1 0 X"

therefore:

RT = r1
RA = r0
X  = immediate (random.choice from test_values)
Comment 56 Cole Poirier 2020-08-07 02:28:27 BST
Updated and pushed, I think it actually works from examining the test output!
Comment 57 Luke Kenneth Casson Leighton 2020-08-07 07:37:17 BST
(In reply to Cole Poirier from comment #56)
> Updated and pushed, I think it actually works from examining the test output!

        test_values = [-32768, -32767, -32766, -2, -1, 0, 1, 2, 32766, 32767] 

so, this is now some fixed values (only), where we want fixed values
*and* random values appended.

        for i in range(40):
            choice = random.choice(test_values)
            l = [f"mulli 0, 1, {choice}"]

this is now "multiply immediate-choice randomly selected by register 1
and put the result in register 0"

we don't want "a random choice" we want a for-loop of *all* options from
test_values

            initial_regs = [0] * 32
            initial_regs[1] = random.randint(-1 << 15, (1 << 15) - 1)

this - a 64-bit register - is limited to the range -32768 to +32767
we want register 1 to be the full range 0 to 2^64-1
Comment 58 Cole Poirier 2020-08-07 18:00:19 BST
(In reply to Luke Kenneth Casson Leighton from comment #57)
> (In reply to Cole Poirier from comment #56)
> > Updated and pushed, I think it actually works from examining the test output!
> 
>         test_values = [-32768, -32767, -32766, -2, -1, 0, 1, 2, 32766,
> 32767] 
> 
> so, this is now some fixed values (only), where we want fixed values
> *and* random values appended.
> 
>         for i in range(40):
>             choice = random.choice(test_values)
>             l = [f"mulli 0, 1, {choice}"]
> 
> this is now "multiply immediate-choice randomly selected by register 1
> and put the result in register 0"
> 
> we don't want "a random choice" we want a for-loop of *all* options from
> test_values

Right, that makes more sense. I'm sorry but I wasn't able to figure out the manner in which you want me to append these random values? Is it simple a line before the "for val in test_values" loop that goes like this?

```
for i in range(20):
    test_values.append(random.randint(-1 << 15, (1 << 15) - 1))
```
 
>             initial_regs = [0] * 32
>             initial_regs[1] = random.randint(-1 << 15, (1 << 15) - 1)
> 
> this - a 64-bit register - is limited to the range -32768 to +32767
> we want register 1 to be the full range 0 to 2^64-1

Thank you, I was under the misapprehension that it needed to be in the 2^16-1 range in order to ever hit the edge cases over million of runs, thanks for helping me clear up this misunderstanding.
Comment 59 Luke Kenneth Casson Leighton 2020-08-08 12:27:28 BST
(In reply to Cole Poirier from comment #58)
> (In reply to Luke Kenneth Casson Leighton from comment #57)
> > (In reply to Cole Poirier from comment #56)
> > > Updated and pushed, I think it actually works from examining the test output!
> > 
> >         test_values = [-32768, -32767, -32766, -2, -1, 0, 1, 2, 32766,
> > 32767] 
> > 
> > so, this is now some fixed values (only), where we want fixed values
> > *and* random values appended.
> > 
> >         for i in range(40):
> >             choice = random.choice(test_values)
> >             l = [f"mulli 0, 1, {choice}"]
> > 
> > this is now "multiply immediate-choice randomly selected by register 1
> > and put the result in register 0"
> > 
> > we don't want "a random choice" we want a for-loop of *all* options from
> > test_values
> 
> Right, that makes more sense. I'm sorry but I wasn't able to figure out the
> manner in which you want me to append these random values? Is it simple a
> line before the "for val in test_values" loop that goes like this?
> 
> ```
> for i in range(20):
>     test_values.append(random.randint(-1 << 15, (1 << 15) - 1))
> ```

yep that's it. the idea is to tidy up the morass of dozens of hand-added random values, not change the actual functionality.

 
> >             initial_regs = [0] * 32
> >             initial_regs[1] = random.randint(-1 << 15, (1 << 15) - 1)
> > 
> > this - a 64-bit register - is limited to the range -32768 to +32767
> > we want register 1 to be the full range 0 to 2^64-1
> 
> Thank you, I was under the misapprehension that it needed to be in the
> 2^16-1 range in order to ever hit the edge cases over million of runs,

shrinking the range definitely won't achieve that.

> thanks for helping me clear up this misunderstanding.

i see where you're heading, good point, yes we can refine it, by combining similar things to the "all" test.

however it should be the 0x0 0x1 0xffff.. etc etc list again expanded by some random values this time in the range 0 to 2^64-1

so the inner loop should set up the list same as in the "all" test named test_values_ra, append a few random values, then do "for ra in test_values_ra"