RE: [PATCH] Remove test_vshuff from hvx_misc tests
> -Original Message- > From: Brian Cain > Sent: Tuesday, May 9, 2023 3:01 PM > To: Taylor Simpson ; Marco Liebel (QUIC) > ; qemu-devel@nongnu.org > Cc: Matheus Bernardino (QUIC) > Subject: RE: [PATCH] Remove test_vshuff from hvx_misc tests > > > > > -Original Message- > > From: Taylor Simpson > > Sent: Tuesday, May 9, 2023 2:28 PM > > To: Marco Liebel (QUIC) ; qemu- > > de...@nongnu.org > > Cc: Brian Cain ; Matheus Bernardino (QUIC) > > > > Subject: RE: [PATCH] Remove test_vshuff from hvx_misc tests > > > > > > > > > -Original Message- > > > From: Marco Liebel (QUIC) > > > Sent: Tuesday, May 9, 2023 1:43 PM > > > To: qemu-devel@nongnu.org > > > Cc: Taylor Simpson ; Brian Cain > > > ; Matheus Bernardino (QUIC) > > > ; Marco Liebel (QUIC) > > > > > > Subject: [PATCH] Remove test_vshuff from hvx_misc tests > > > > > > test_vshuff checks that the vshuff instruction works correctly when > > > both vector registers are the same. Using vshuff in this way is > > > undefined and will be rejected by the compiler in a future version of the > toolchain. > > > > > > Signed-off-by: Marco Liebel > > > --- > > > tests/tcg/hexagon/hvx_misc.c | 45 > > > > > > 1 file changed, 45 deletions(-) > > > > Let's not remove the test completely. Just change it to use different > registers. > > I'm fine either way. But IIRC we added this test particularly in order to > verify > the potentially ambiguous behavior of the same operand here. It may be > well tested otherwise. I confirmed the hvx_histogram test executes this instruction, so Reviewed-by: Taylor Simpson
RE: [PATCH] Remove test_vshuff from hvx_misc tests
> -Original Message- > From: Taylor Simpson > Sent: Tuesday, May 9, 2023 2:28 PM > To: Marco Liebel (QUIC) ; qemu- > de...@nongnu.org > Cc: Brian Cain ; Matheus Bernardino (QUIC) > > Subject: RE: [PATCH] Remove test_vshuff from hvx_misc tests > > > > > -Original Message- > > From: Marco Liebel (QUIC) > > Sent: Tuesday, May 9, 2023 1:43 PM > > To: qemu-devel@nongnu.org > > Cc: Taylor Simpson ; Brian Cain > > ; Matheus Bernardino (QUIC) > > ; Marco Liebel (QUIC) > > > > Subject: [PATCH] Remove test_vshuff from hvx_misc tests > > > > test_vshuff checks that the vshuff instruction works correctly when both > > vector registers are the same. Using vshuff in this way is undefined and > > will > > be rejected by the compiler in a future version of the toolchain. > > > > Signed-off-by: Marco Liebel > > --- > > tests/tcg/hexagon/hvx_misc.c | 45 > > 1 file changed, 45 deletions(-) > > Let's not remove the test completely. Just change it to use different > registers. I'm fine either way. But IIRC we added this test particularly in order to verify the potentially ambiguous behavior of the same operand here. It may be well tested otherwise.
RE: [PATCH] Remove test_vshuff from hvx_misc tests
> -Original Message-
> From: Marco Liebel (QUIC)
> Sent: Tuesday, May 9, 2023 1:43 PM
> To: qemu-devel@nongnu.org
> Cc: Taylor Simpson ; Brian Cain ;
> Matheus Bernardino (QUIC) ; Marco Liebel
> (QUIC)
> Subject: [PATCH] Remove test_vshuff from hvx_misc tests
>
> test_vshuff checks that the vshuff instruction works correctly when
> both vector registers are the same. Using vshuff in this way is
> undefined and will be rejected by the compiler in a future version of
> the toolchain.
>
> Signed-off-by: Marco Liebel
> ---
> tests/tcg/hexagon/hvx_misc.c | 45
> 1 file changed, 45 deletions(-)
>
> diff --git a/tests/tcg/hexagon/hvx_misc.c b/tests/tcg/hexagon/hvx_misc.c
> index d0e64e035f..bc4e76d7f0 100644
> --- a/tests/tcg/hexagon/hvx_misc.c
> +++ b/tests/tcg/hexagon/hvx_misc.c
> @@ -342,49 +342,6 @@ static void test_vsubuwsat_dv(void)
> check_output_w(__LINE__, 2);
> }
>
> -static void test_vshuff(void)
> -{
> -/* Test that vshuff works when the two operands are the same register */
> -const uint32_t splat = 0x089be55c;
> -const uint32_t shuff = 0x454fa926;
> -MMVector v0, v1;
> -
> -memset(expect, 0x12, sizeof(MMVector));
> -memset(output, 0x34, sizeof(MMVector));
> -
> -asm volatile("v25 = vsplat(%0)\n\t"
> - "vshuff(v25, v25, %1)\n\t"
> - "vmem(%2 + #0) = v25\n\t"
> - : /* no outputs */
> - : "r"(splat), "r"(shuff), "r"(output)
> - : "v25", "memory");
> -
> -/*
> - * The semantics of Hexagon are the operands are pass-by-value, so create
> - * two copies of the vsplat result.
> - */
> -for (int i = 0; i < MAX_VEC_SIZE_BYTES / 4; i++) {
> -v0.uw[i] = splat;
> -v1.uw[i] = splat;
> -}
> -/* Do the vshuff operation */
> -for (int offset = 1; offset < MAX_VEC_SIZE_BYTES; offset <<= 1) {
> -if (shuff & offset) {
> -for (int k = 0; k < MAX_VEC_SIZE_BYTES; k++) {
> -if (!(k & offset)) {
> -uint8_t tmp = v0.ub[k];
> -v0.ub[k] = v1.ub[k + offset];
> -v1.ub[k + offset] = tmp;
> -}
> -}
> -}
> -}
> -/* Put the result in the expect buffer for verification */
> -expect[0] = v1;
> -
> -check_output_b(__LINE__, 1);
> -}
> -
> static void test_load_tmp_predicated(void)
> {
> void *p0 = buffer0;
> @@ -489,8 +446,6 @@ int main()
> test_vadduwsat();
> test_vsubuwsat_dv();
>
> -test_vshuff();
> -
> test_load_tmp_predicated();
> test_load_cur_predicated();
>
> --
> 2.25.1
Reviewed-by: Brian Cain
RE: [PATCH] Remove test_vshuff from hvx_misc tests
> -Original Message- > From: Marco Liebel (QUIC) > Sent: Tuesday, May 9, 2023 1:43 PM > To: qemu-devel@nongnu.org > Cc: Taylor Simpson ; Brian Cain > ; Matheus Bernardino (QUIC) > ; Marco Liebel (QUIC) > > Subject: [PATCH] Remove test_vshuff from hvx_misc tests > > test_vshuff checks that the vshuff instruction works correctly when both > vector registers are the same. Using vshuff in this way is undefined and will > be rejected by the compiler in a future version of the toolchain. > > Signed-off-by: Marco Liebel > --- > tests/tcg/hexagon/hvx_misc.c | 45 > 1 file changed, 45 deletions(-) Let's not remove the test completely. Just change it to use different registers. Thanks, Taylor
