RE: [PATCH] tests/tcg/hexagon: fix underspecifed asm constraints

[Mukilan Thiyagarajan (QUIC)]

Thanks for the review, Taylor!

> Good catch.  However, the "p" argument is also modified, so we need to move 
> it to
> the outputs list and use "+r"(p).  This will also require swapping %1 and %2 
> in the body.

Ah, good point. I didn't account for memw(%2++#4) incrementing register %2.
I've made the suggested changes in v2 of the patch:

https://patchew.org/QEMU/20221229081836.12130-1-quic._5Fmthiyaga@quicinc.com/

Regards,
Mukilan

-----Original Message-----
From: Taylor Simpson <tsimpson@quicinc.com> 
Sent: Wednesday, December 28, 2022 11:07 PM
To: Mukilan Thiyagarajan (QUIC) <quic_mthiyaga@quicinc.com>; 
qemu-devel@nongnu.org
Cc: Brian Cain <bcain@quicinc.com>; Matheus Bernardino (QUIC) 
<quic_mathbern@quicinc.com>
Subject: RE: [PATCH] tests/tcg/hexagon: fix underspecifed asm constraints



> -----Original Message-----
> From: Mukilan Thiyagarajan (QUIC) <quic_mthiyaga@quicinc.com>
> Sent: Wednesday, December 28, 2022 9:37 AM
> To: qemu-devel@nongnu.org; Taylor Simpson <tsimpson@quicinc.com>
> Cc: Brian Cain <bcain@quicinc.com>; Matheus Bernardino (QUIC)
> <quic_mathbern@quicinc.com>; Mukilan Thiyagarajan (QUIC)
> <quic_mthiyaga@quicinc.com>
> Subject: [PATCH] tests/tcg/hexagon: fix underspecifed asm constraints
> 
> There are two test cases where the inline asm doesn't have the correct
> constraints causing them to fail when using certain clang
> versions/optimization levels.
> 
> In mem_noshuf.c, the register r7 is written to but not specified in the 
> clobber
> list.
> 
> In misc.c, the 'result' output needs the early clobber modifier since the rest
> of the inputs are read after assignment to the output register.
> 
> Signed-off-by: Mukilan Thiyagarajan <quic_mthiyaga@quicinc.com>
> ---
>  tests/tcg/hexagon/mem_noshuf.c | 2 +-
>  tests/tcg/hexagon/misc.c       | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/tcg/hexagon/misc.c b/tests/tcg/hexagon/misc.c index
> f0b1947fb3..9b1604d82f 100644
> --- a/tests/tcg/hexagon/misc.c
> +++ b/tests/tcg/hexagon/misc.c
> @@ -189,7 +189,7 @@ static int L2_ploadrifnew_pi(void *p, int pred)
>                 "    p0 = cmp.eq(%1, #1)\n\t"
>                 "    if (!p0.new) %0 = memw(%2++#4)\n\t"
>                 "}\n\t"
> -               : "=r"(result) : "r"(pred), "r"(p)
> +               : "=&r"(result) : "r"(pred), "r"(p)

Good catch.  However, the "p" argument is also modified, so we need to move it 
to the outputs list and use "+r"(p).  This will also require swapping %1 and %2 
in the body.

Otherwise
Reviewed-by: Taylor Simpson <tsimpson@quicinc.com>