Re: [PATCH v5 13/14] target/hexagon: import additional tests

[Taylor Simpson <taylor.qemu@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 10996 bytes --]

On Sat, Jun 19, 2021 at 4:49 AM Alessandro Di Federico via <
qemu-devel@nongnu.org> wrote:

> From: Niccolò Izzo <nizzo@rev.ng>
>
> Signed-off-by: Alessandro Di Federico <ale@rev.ng>
> Signed-off-by: Niccolò Izzo <nizzo@rev.ng>
> ---
>


> diff --git a/tests/tcg/hexagon/crt.S b/tests/tcg/hexagon/crt.S
> new file mode 100644
> index 0000000000..2c10577470
> --- /dev/null
> +++ b/tests/tcg/hexagon/crt.S
> @@ -0,0 +1,28 @@
> +#define SYS_exit_group           94
> +
> +    .text
> +    .globl init
> +init:
> +    {
> +        allocframe(r29,#0):raw
> +    }
> +    {
> +        r0=#256
> +    }
> +    {
> +        dealloc_return
> +    }
>

You don't need to alloc/dealloc the frame, just a single packet { r0 =
#256; jumpr r31 }

Then again, why is this function needed at all?


+
> +    .space 240
>

Why is this space needed?

diff --git a/tests/tcg/hexagon/test_andp.S b/tests/tcg/hexagon/test_andp.S
> new file mode 100644
> index 0000000000..3c4aa8b2ae
> --- /dev/null
> +++ b/tests/tcg/hexagon/test_andp.S
> @@ -0,0 +1,23 @@
> +/* Purpose: test a multiple predicate AND combination */
>

This is already tested in misc.c

+
> +    .text
> +    .globl _start
> +
> +_start:
> +    {
> +        r1+=sub(r2,r3)
>

Remove this - r1, r2, and r3 are uninitialized.  Then, you overwrite r1 in
the next packet.


> +        call init
> +    }
> +    {
> +        r0=#0
> +        r1=#1
> +    }
> +    {
> +        p0=cmp.gt(r0,r1)
> +        p0=cmp.gt(r0,r1)
> +        p0=cmp.gt(r1,r0)
> +    }
> +    {
> +        if (!p0) jump:t pass
> +        jump fail
> +    }
>
> diff --git a/tests/tcg/hexagon/test_call.S b/tests/tcg/hexagon/test_call.S
> new file mode 100644
> index 0000000000..53a2450522
> --- /dev/null
> +++ b/tests/tcg/hexagon/test_call.S
> @@ -0,0 +1,63 @@
> +/* Purpose: test function calls and duplex instructions.
> + * The string "Hello there, I'm a test string!" with the first letter
> replaced
> + * with a capital L should be printed out.
> + */
> +
> +    .text
> +    .globl    test
> +test:
> +    {
> +        jumpr r31
> +        memb(r0+#0)=#76
> +    }
> +.Lfunc_end0:
> +.Ltmp0:
> +    .size    test, .Ltmp0-test
> +
> +    .globl    _start
> +_start:
> +    {
> +        call init
> +    }
> +    {
> +        call test
> +        r0=##dummy_buffer
> +        allocframe(#0)
> +    }
> +    {
> +        call write
> +    }
> +    {
> +        jump pass
> +    }
> +    {
> +        r31:30=deallocframe(r30):raw
> +    }
> +.Lfunc_end1:
> +.Ltmp1:
> +    .size    _start, .Ltmp1-_start
> +write:
> +    {
> +        r2=##dummy_buffer
> +    }
> +    { r0=r2; }
>

Assign to r0 directly


> +    {
> +        r2=#256
> +    }
> +    { r1=r2; }
>

Assign to r1 directly

+    { trap0(#7); }
>

This doesn't print anything - the syscall number goes in r6.  Is the intent
to invoke SYS_write?  If so, look at first.S to see how this is done.

+    {
> +        jumpr r31
> +    }
> +.Lfunc_end2:
> +.Ltmp2:
> +    .size    write, .Ltmp2-write
> +
> +    .type    dummy_buffer,@object
> +    .data
> +    .globl    dummy_buffer
> +    .p2align    3
> +dummy_buffer:
> +    .string    "Hello there, I'm a test string!\n"
> +    .space 223
> +    .size    dummy_buffer, 256
>



> diff --git a/tests/tcg/hexagon/test_djump.S
> b/tests/tcg/hexagon/test_djump.S
> new file mode 100644
> index 0000000000..dbad7eb0a1
> --- /dev/null
> +++ b/tests/tcg/hexagon/test_djump.S
> @@ -0,0 +1,24 @@
> +/* Purpose: show dual jumps actually work. This program features a packet
> where
> + * two jumps should (in theory) be performed if !P0. However, we correctly
> + * handle the situation by performing only the first one and ignoring the
> second
> + * one. This can be verified by checking that the CPU dump contains
> 0xDEADBEEF
> + * in R2.
> + */
>

How does 0xDEADBEEF get into r2?

Do we need this test given that every other test relies on this idiom to
check pass/fail?

+
> +    .text
> +    .globl _start
> +
> +_start:
> +    {
> +        call init
> +    }
> +    {
> +        r1 = #255;
> +    }
> +    {
> +        p0 = r1;
> +    }
> +    {
> +        if (p0) jump:t pass
> +        jump fail
> +    }
> diff --git a/tests/tcg/hexagon/test_dotnew.S
> b/tests/tcg/hexagon/test_dotnew.S
> new file mode 100644
> index 0000000000..3897c6bc96
> --- /dev/null
> +++ b/tests/tcg/hexagon/test_dotnew.S
> @@ -0,0 +1,39 @@
> +/* Purpose: test the .new operator while performing memory stores.
> + * In the final CPU dump R0 should contain 3, R1 should contain 2 and R2
> should
> + * contain 1.
> + */
>

CPU dump sounds like something you have locally.  The check-tcg tests
should not rely on this.

+    .text
> +    .globl _start
> +
> +_start:
> +    {
> +        call init
> +    }
> +    {
> +        r0=#1
> +        memw(sp+#0)=r0.new
> +    }
>

You haven't set up the stack, so you shouldn't use sp.  Even if the stack
were set up, you should allocframe first.

+    {
> +        r1=#2
> +        memw(sp+#4)=r1.new
> +    }
> +    {
> +        r2=#3
> +        memw(sp+#8)=r2.new
> +    }
> +    {
> +        r0=memw(sp+#8)
> +    }
> +    {
> +        r1=memw(sp+#4)
> +    }
> +    {
> +        r2=memw(sp+#0)
> +    }
> +    {
> +        r3=mpyi(r1,r2)
> +    }
> +    {
> +        p0 = cmp.eq(r3, #2); if (p0.new) jump:t pass
> +        jump fail
> +    }
> diff --git a/tests/tcg/hexagon/test_dstore.S
> b/tests/tcg/hexagon/test_dstore.S
> new file mode 100644
> index 0000000000..62c4301eb1
> --- /dev/null
> +++ b/tests/tcg/hexagon/test_dstore.S
> @@ -0,0 +1,29 @@
> +/* Purpose: test dual stores correctness.
> + * In this example the values 1 and 2 are both written on the top of the
> stack
> + * in a single packet.
> + * The value is then read back in R3, which should contain only the
> latest value
> + * written (2).
> + */
>

 This is already tested in dual_stores.c

+
> +    .text
> +    .globl _start
> +
> +_start:
> +    {
> +        call init
> +    }
> +    {
> +        r0=#1
> +        r1=#2
> +    }
> +    {
> +        memw(sp+#0)=r0
> +        memw(sp+#0)=r1
> +    }
>

Stack setup ...

+    {
> +        r3=memw(sp+#0)
> +    }
> +    {
> +        p0 = cmp.eq(r3, #2); if (p0.new) jump:t pass
> +        jump fail
> +    }
> diff --git a/tests/tcg/hexagon/test_ext.S b/tests/tcg/hexagon/test_ext.S
> new file mode 100644
> index 0000000000..0f6e21593a
> --- /dev/null
> +++ b/tests/tcg/hexagon/test_ext.S
> @@ -0,0 +1,18 @@
> +/* Purpose: test immediate extender instructions.
> + * In the CPU dump R0 should contain 0xDEADBEEF.
>

The CPU dump comment doesn't apply


> + */
> +
> +    .text
> +    .globl _start
> +
> +_start:
> +    {
> +        call init
> +    }
> +    {
> +        r2=##-559038737
> +    }
> +    {
> +        p0 = cmp.eq(r2, ##-559038737); if (p0.new) jump:t pass
> +        jump fail
> +    }
>


> +++ b/tests/tcg/hexagon/test_hello.S
> @@ -0,0 +1,21 @@
> +/* Purpose: simple hello world program. */
>

This is already tested in first.S

+
> +    .text
> +    .globl _start
> +
> +_start:
> +    {
> +        call init
> +    }
> +    { r0=#4; }
> +    {
> +        r1=##.L.str
> +    }
> +    { trap0(#0); }
> +    {
> +        jump pass
> +    }
> +
> +.L.str:
> +    .string    "Hello world!\n"
> +    .size    .L.str, 14
>


> diff --git a/tests/tcg/hexagon/test_hwloops.S
> b/tests/tcg/hexagon/test_hwloops.S
> new file mode 100644
> index 0000000000..8337083d8e
> --- /dev/null
> +++ b/tests/tcg/hexagon/test_hwloops.S
> @@ -0,0 +1,25 @@
> +/* Purpose: simple C Program to test hardware loops.
> + * It should print numbersfrom 0 to 9.
>

This doesn't actually print anything.


> + */
> +
> +    .text
> +    .globl _start
> +
> +_start:
> +    {
> +        call init
> +    }
> +    {
> +        loop0(.LBB0_1,#10)
> +        r2=#0
> +    }
> +.Ltmp0:
> +.LBB0_1:
> +    {
> +        r2=add(r2,#1)
> +        nop
> +    }:endloop0
> +    {
> +        p0 = cmp.eq(r2, #10); if (p0.new) jump:t pass
> +        jump fail
> +    }
>


> diff --git a/tests/tcg/hexagon/test_jmp.S b/tests/tcg/hexagon/test_jmp.S
> new file mode 100644
> index 0000000000..9bf6ea34e5
> --- /dev/null
> +++ b/tests/tcg/hexagon/test_jmp.S
> @@ -0,0 +1,25 @@
> +/* Purpose: test example, verify the soundness of the add operation */
>

What value does this test have beyond the others in this set?

+
> +    .text
> +    .globl _start
> +
> +_start:
> +    {
> +        call init
> +    }
> +    {
> +        r1=#0
> +        r2=#0
> +    }
> +    {
> +        r3=add(r1,r2)
> +    }
> +    {
> +        p0 = cmp.eq(r3, #0)
> +    }
> +    {
> +        if (p0) jump:t pass
> +    }
> +    {
> +        jump fail
> +    }
>

diff --git a/tests/tcg/hexagon/test_packet.S
b/tests/tcg/hexagon/test_packet.S

> new file mode 100644
> index 0000000000..d26e284be9
> --- /dev/null
> +++ b/tests/tcg/hexagon/test_packet.S
> @@ -0,0 +1,26 @@
> +/* Purpose: test that writes of a register in a packet are performed only
> after
> + * that packet has finished its execution.
> + */
> +
> +    .text
> +    .globl _start
> +
> +_start:
> +    {
> +        call init
> +    }
> +    {
> +        r2=#4
> +        r3=#6
> +    }
> +    {
> +        memw(sp+#0)=r2
>

Stack problem

+    }
> +    {
> +        r3=memw(sp+#0)
> +        r0=add(r2,r3)
> +    }
> +    {
> +        p0 = cmp.eq(r0, #10); if (p0.new) jump:t pass
> +        jump fail
> +    }
> diff --git a/tests/tcg/hexagon/test_reorder.S
> b/tests/tcg/hexagon/test_reorder.S
> new file mode 100644
> index 0000000000..508d5302f9
> --- /dev/null
> +++ b/tests/tcg/hexagon/test_reorder.S
> @@ -0,0 +1,31 @@
> +/* Purpose: demonstrate handling of .new uses appearing before the
> associated
> + * definition.
> + * Here we perform a jump that skips the code resetting R2 from
> 0xDEADBEEF to 0,
> + * only if P0.new is true, but P0 is assigned to 1 (R4) in the next
> instruction
> + * in the packet.
> + * A successful run of the program will show R2 retaining the 0xDEADBEEF
> value
> + * in the CPU dump.
>

CPU dump ...

+ */
> +
> +    .text
> +    .globl _start
> +
> +_start:
> +    {
> +        call init
> +    }
> +    { r2=#-559038737 }
> +    { r4=#1 }
> +    {
> +        if (p0.new) jump:nt skip
> +        p0=r4;
> +    }
> +
> +fallthrough:
> +    { r2=#0 }
> +
> +skip:
> +    {
> +        p0 = cmp.eq(r2, #-559038737); if (p0.new) jump:t pass
> +        jump fail
> +    }
>

Each of these are very small, so I recommend putting them into misc.c or
combine all the assembly into a small number of executables.

[-- Attachment #2: Type: text/html, Size: 16999 bytes --]