Are big tests allowed in binutils?

Are big tests allowed in binutils?

[Dmitry Selyutin]

Hi folks,

I'm going to submit a series of patches which add a couple of new
instructions available as SVP64 extensions (under -mlibresoc switch).
Since I want to be a good citizen, I also want to introduce tests for
each instruction, basically comparing the expected results for dumps
(aka run_dump_test). However, I'd like to cover various possible
cases, so I generated a test to cover many different operands; this
makes tests quite big. For example, the assembly listing for a new
svremap instruction takes 10240 lines, and etalon dump takes a bit
more lines. So, questions:
1. Is it OK at all to have tests that large? Time-wise they're fast,
I'm mostly concerned about code base size and readers' patience.
2. Will you tolerate it if I post these tests in the mailing list?
These will be a major headache for any reader if integrated into the
patches.
3. If it's OK to post these tests into the mailing lists, is it
allowed to post them as separate patches from the ones which add the
instructions?

My preferred option would be to split each patch into two parts:
1. The patch which adds the instruction itself.
2. The patch which adds the test for this instruction.

However, looking at patches in PowerPC, I see that the tests generally
come along with the implementation in scope of one patch. I'm afraid
this might make patches barely readable.
If needed, I can cut the size of tests; time-wise it's fast, 10420
lines of asm take 0,04s of real time, I'm only concerned about
readability.

I'd like to know your opinion on this. Thank you!

-- 
Best regards,
Dmitry Selyutin

Re: Are big tests allowed in binutils?

[Dmitry Selyutin]

On Fri, Jun 17, 2022 at 8:11 AM Dmitry Selyutin <ghostmansd@gmail.com> wrote:
> However, I'd like to cover various possible
> cases, so I generated a test to cover many different operands; this
> makes tests quite big.

I forgot to mention another option. Since I generated these tests
anyway, perhaps I could generate them in binutils instead? I used
Python, but can rewrite it to some other language if necessary. Does
binutils testing suite possess a way to generate tests? Perhaps we
could use it. This would take some time upon $(make check), but not a
critical (it's simply a bunch of nested for loops).
An obvious option would be editing some Makefile to generate the test;
if there's an option for ppc.exp to force such generation, I'd be more
than happy to do that (this'd be a better alternative).

Re: Are big tests allowed in binutils?

[Nick Alcock]

On 17 Jun 2022, Dmitry Selyutin via Binutils outgrape:

> On Fri, Jun 17, 2022 at 8:11 AM Dmitry Selyutin <ghostmansd@gmail.com> wrote:
>> However, I'd like to cover various possible
>> cases, so I generated a test to cover many different operands; this
>> makes tests quite big.
>
> I forgot to mention another option. Since I generated these tests
> anyway, perhaps I could generate them in binutils instead? I used
> Python, but can rewrite it to some other language if necessary. Does
> binutils testing suite possess a way to generate tests? Perhaps we
> could use it. This would take some time upon $(make check), but not a
> critical (it's simply a bunch of nested for loops).
> An obvious option would be editing some Makefile to generate the test;
> if there's an option for ppc.exp to force such generation, I'd be more
> than happy to do that (this'd be a better alternative).

To me that feels more elegant. If the tests were autogenerated, the
generator should be in the source tree anyway, so why not use it to
generate the tests if it's there?

-- 
NULL && (void)

Re: Are big tests allowed in binutils?

[lkcl]

sorry nick i am stuck with a rubbish mailer, rather than top-post i'm repeating your question: why not submit the full source of the autogenerator?

A: because the list of dependencies, whilst entirely FOSS, is absolutely mental and is completely outside of our control, copyright-wise.  it's also 4 levels deep dependencies and also includes *another* repository comprising CSV and markdown files extracted from the Power ISA Specification PDF [1] and microwatt [2]

now, Dmitry did write a small stand-alone program to autogenerate the test cases but he had to write that by hand.

our plan is to *autogenerate the autogenerator* [based on information that ultimately comes directly from the Power ISA Specification].

why?

1) because if mistakes are found they can be tracked back directly to the spec.  [we have found appx ... 15 corrections to the published spec because of this]

2) because (no implied criticism of anyone else that does do manual unit tests) we simply don't have time money or resources to throw brute-force at this so out of sheer forced pragmatism are doing more with less.

i have no view on whether the autogenerated-autogenerator should be submitted or whether the automated unit tests themselves should be submitted but i can say for sure you *really* don't want the full source code of the full software stack we have been developing for 2+ years as a hard critical build dependency of binutils.

l.

[1] https://git.libre-soc.org/?p=openpower-isa.git;a=tree;f=openpower/isa;hb=HEAD
https://git.libre-soc.org/?p=openpower-isa.git;a=tree;f=openpower/isatables;hb=HEAD

[2] https://github.com/antonblanchard/microwatt/blob/master/decode1.vhdl

Re: Are big tests allowed in binutils?

[Nick Alcock]

On 20 Jun 2022, lkcl via Binutils spake thusly:

> sorry nick i am stuck with a rubbish mailer, rather than top-post i'm repeating your question: why not submit the full source of the autogenerator?
>
> A: because the list of dependencies, whilst entirely FOSS, is absolutely mental and is completely outside of our control, copyright-wise.  it's also 4 levels deep dependencies and also includes *another* repository comprising CSV and markdown files extracted from the Power ISA Specification PDF [1] and microwatt [2]
>
> now, Dmitry did write a small stand-alone program to autogenerate the test cases but he had to write that by hand.
>
> our plan is to *autogenerate the autogenerator* [based on information that ultimately comes directly from the Power ISA Specification].

That sounds like a good idea to me.

> 1) because if mistakes are found they can be tracked back directly to
> the spec. [we have found appx ... 15 corrections to the published spec
> because of this]
>
> 2) because (no implied criticism of anyone else that does do manual
> unit tests) we simply don't have time money or resources to throw
> brute-force at this so out of sheer forced pragmatism are doing more
> with less.
> 
> i have no view on whether the autogenerated-autogenerator should be
> submitted or whether the automated unit tests themselves should be
> submitted but i can say for sure you *really* don't want the full
> source code of the full software stack we have been developing for 2+
> years as a hard critical build dependency of binutils.

While it's perfectly acceptable to commit the generated output if
generating the output is annoying (and having a huge stack of deps
certainly sounds like it would be annoying), you've got to commit enough
that others can also modify the testcases the same way you do: the
preferred form for modification in this case is pretty clearly not the
generated testcases but the autogenerators themselves (and any input to
them), so the GPL requires you to provide the generators too. They don't
have to be in the binutils repo, but honestly putting them in there so
people can manually run them if needed seems like a good idea. (Stick
in simple pointers to the deps as well, particularly if just figuring ou
what the deps *are* would otherwise be difficult, as "absolutely mental"
suggests it would be).

-- 
NULL && (void)

Re: Are big tests allowed in binutils?

[lkcl]

On Tue, Jun 21, 2022 at 2:43 PM Nick Alcock <nick.alcock@oracle.com> wrote:

> preferred form for modification in this case is pretty clearly not the
> generated testcases but the autogenerators themselves (and any input to
> them), so the GPL requires you to provide the generators too. They don't
> have to be in the binutils repo, but honestly putting them in there so
> people can manually run them if needed seems like a good idea. (Stick
> in simple pointers to the deps as well, particularly if just figuring ou
> what the deps *are* would otherwise be difficult, as "absolutely mental"
> suggests it would be).

*click*. of course.  you don't commit the source code of llvm to
the binutils repo just because it's a tool that's required, even
though the GPL states clearly that the tool(s) must be *available*.

(i mention llvm deliberately because, like the tools we use to
auto-generate the tests, it's also not GPL-licensed).

describing where those tools are is a good call.

l.

Re: Are big tests allowed in binutils?

[Dmitry Selyutin]

On Tue, Jun 21, 2022 at 8:26 PM lkcl <luke.leighton@gmail.com> wrote:
> describing where those tools are is a good call.

...as long as the tools _are_ available, which is not currently the
case, since we still have to implement a good generator. Or did you
mean the SVP64 source code itself by "tools"?

-- 
Best regards,
Dmitry Selyutin

Re: Are big tests allowed in binutils?

[lkcl]

On Tue, Jun 21, 2022 at 6:30 PM Dmitry Selyutin <ghostmansd@gmail.com> wrote:

> On Tue, Jun 21, 2022 at 8:26 PM lkcl <luke.leighton@gmail.com> wrote:
> > describing where those tools are is a good call.
>
> ...as long as the tools _are_ available, which is not currently the
> case, since we still have to implement a good generator. Or did you
> mean the SVP64 source code itself by "tools"

sv_analysis [0], sv_binutils [1], and the [planned] test auto-generator.
whilst these tools are under the LGPLv3+ [2] and their output, because
they are effectively a compiler tool we *choose* to release the output
of those tools under the GPL [and assign it to the FSF] and one of the
dependencies is not GPL'd (BSD 2-clause) [3]

we have fully-automated install scripts for the installation of all
software dependencies.  up to line 61 of this script [4] is sufficient.
where's a good place to include mention of that, should anyone else
wish to make modifications in future?

l.

[0] https://git.libre-soc.org/?p=openpower-isa.git;a=blob;f=src/openpower/sv/sv_analysis.py;hb=HEAD
[1] https://git.libre-soc.org/?p=openpower-isa.git;a=blob;f=src/openpower/sv/sv_binutils.py;hb=HEAD
[2] https://git.libre-soc.org/?p=openpower-isa.git;a=blob;f=COPYING.LGPLv3;hb=HEAD
[3] https://gitlab.com/nmigen/nmigen/-/blob/master/LICENSE.txt
[4] https://git.libre-soc.org/?p=dev-env-setup.git;a=blob;f=hdl-dev-repos;hb=HEAD

Re: Are big tests allowed in binutils?

[Andreas Schwab]

On Jun 17 2022, Dmitry Selyutin via Binutils wrote:

> For example, the assembly listing for a new
> svremap instruction takes 10240 lines, and etalon dump takes a bit
> more lines. So, questions:
> 1. Is it OK at all to have tests that large? Time-wise they're fast,

The biggest test so far is gas/testsuite/gas/tic54x/all-opcodes.s, with
124668 lines.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: Are big tests allowed in binutils?

[Richard Earnshaw]

On 17/06/2022 06:11, Dmitry Selyutin via Binutils wrote:
> Hi folks,
> 
> I'm going to submit a series of patches which add a couple of new
> instructions available as SVP64 extensions (under -mlibresoc switch).
> Since I want to be a good citizen, I also want to introduce tests for
> each instruction, basically comparing the expected results for dumps
> (aka run_dump_test). However, I'd like to cover various possible
> cases, so I generated a test to cover many different operands; this
> makes tests quite big. For example, the assembly listing for a new
> svremap instruction takes 10240 lines, and etalon dump takes a bit
> more lines. So, questions:
> 1. Is it OK at all to have tests that large? Time-wise they're fast,
> I'm mostly concerned about code base size and readers' patience.
> 2. Will you tolerate it if I post these tests in the mailing list?
> These will be a major headache for any reader if integrated into the
> patches.
> 3. If it's OK to post these tests into the mailing lists, is it
> allowed to post them as separate patches from the ones which add the
> instructions?
> 
> My preferred option would be to split each patch into two parts:
> 1. The patch which adds the instruction itself.
> 2. The patch which adds the test for this instruction.
> 
> However, looking at patches in PowerPC, I see that the tests generally
> come along with the implementation in scope of one patch. I'm afraid
> this might make patches barely readable.
> If needed, I can cut the size of tests; time-wise it's fast, 10420
> lines of asm take 0,04s of real time, I'm only concerned about
> readability.
> 
> I'd like to know your opinion on this. Thank you!
> 

Good testing is about being smart.  Tests get rerun many times and 
although each run is not too expensive, over time it adds up.

Furthermore, most instructions in an architecture often follow standard 
'shapes' in order to make decoding fast for the hardware.  So registers 
numbers are often encoded in consecutive bits at well-known places 
within the instruction.  You can use this knowledge to significantly 
reduce the number of tests that need to be written.

For example, if you have an instruction FOO that takes two 4-bit 
register numbers (a 16-register architecture), and register r0 is all 
bits zero and r15 is all bits one, you might have a test

	FOO r0, r0
	FOO r0, r15
	FOO r15, r0

These three tests would first check that the base opcode for FOO is 
correct, the second that the register bits for the second register are 
all encoded into the right spot and the third that the register bits for 
the first register are placed correctly.  That's three tests rather than 
256 that would be needed to test all register permutations.

For a select number of instructions you might check all register numbers 
are correctly encoded, but it shouldn't be necessary to do that for 
every instruction - the code that inserts the bits is probably common to 
all related instructions and you'd just be repeatedly testing that with 
no added value.

The same is also true for literal values encoded in instructions: only a 
few tests are likely to stress the boundaries of your parsing and 
encoding, so focus on testing those boundaries.

This obviously won't work for every instruction.  Sometimes there are 
encoding restrictions, for example you can't both read and write a 
register in the same instruction.  But most of the time careful choice 
of what you test can significantly reduce the size of the testsuite and 
that in turn can make tracking down a problem much easier when you do 
get a regression.

R.

Re: Are big tests allowed in binutils?

[Dmitry Selyutin]

The tests there are already generated with this assumption in mind
already, they don't check all possible combinations (otherwise we'd
have ended up with ~4GiB per test, eh).
At the same time, they also check the operands together, basically
this is a bunch of nested for loops, where each loop checks several
predefined values for each operand.
I want to write some code which generates a test generator for
binutils; this is the simplest option to avoid binutils depending on
our code (you for sure don't want it).

This, however, is a big task, it requires many changes on our side,
and I don't have time for this right now. It's also blocked somewhat
with another task.
Good news is that I will undoubtedly do it, because we have a plethora
of scenarios which depend on auto-generation.
And yes, we do want to be good citizens, that's why I brought up the
whole auto-generation topic. :-)

-- 
Best regards,
Dmitry Selyutin

Re: Are big tests allowed in binutils?

[Jan Beulich]

On 21.06.2022 18:28, Dmitry Selyutin via Binutils wrote:
> The tests there are already generated with this assumption in mind
> already, they don't check all possible combinations (otherwise we'd
> have ended up with ~4GiB per test, eh).
> At the same time, they also check the operands together, basically
> this is a bunch of nested for loops, where each loop checks several
> predefined values for each operand.

This (in particular "nested loops") doesn't sound like it's matching
what Richard has suggested (which is a pattern I also generally try
to follow). Taking his example and assuming that I properly interpret
"several predefined values", your approach would result in

	FOO r0, r0
	FOO r0, r15
	FOO r15, r0
	FOO r15, r15

i.e. one more test than necessary. Obviously the number of "excess"
tests would grow (significantly) with the number of operands an insn
has (while there would be no difference for single-operand insns).

Jan

> I want to write some code which generates a test generator for
> binutils; this is the simplest option to avoid binutils depending on
> our code (you for sure don't want it).
> 
> This, however, is a big task, it requires many changes on our side,
> and I don't have time for this right now. It's also blocked somewhat
> with another task.
> Good news is that I will undoubtedly do it, because we have a plethora
> of scenarios which depend on auto-generation.
> And yes, we do want to be good citizens, that's why I brought up the
> whole auto-generation topic. :-)
>