diff mbox series

[for-9.1] tcg/ppc: Sync tcg_out_test and constraints

Message ID 20240807040843.7882-1-richard.henderson@linaro.org (mailing list archive)
State New, archived
Headers show
Series [for-9.1] tcg/ppc: Sync tcg_out_test and constraints | expand

Commit Message

Richard Henderson Aug. 7, 2024, 4:08 a.m. UTC 
Ensure the code structure is the same for matching constraints
and emitting code, lest we allow constants that cannot be
trivially tested.

Cc: qemu-stable@nongnu.org
Fixes: ad788aebbab ("tcg/ppc: Support TCG_COND_TST{EQ,NE}")
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2487
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 tcg/ppc/tcg-target.c.inc | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

Comments

Philippe Mathieu-Daudé Aug. 7, 2024, 4:46 p.m. UTC | #1 
On 7/8/24 06:08, Richard Henderson wrote:
> Ensure the code structure is the same for matching constraints
> and emitting code, lest we allow constants that cannot be
> trivially tested.
> 
> Cc: qemu-stable@nongnu.org
> Fixes: ad788aebbab ("tcg/ppc: Support TCG_COND_TST{EQ,NE}")
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2487
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   tcg/ppc/tcg-target.c.inc | 21 ++++++++++-----------
>   1 file changed, 10 insertions(+), 11 deletions(-)
> 
> diff --git a/tcg/ppc/tcg-target.c.inc b/tcg/ppc/tcg-target.c.inc
> index 7f3829beeb..3553a47ba9 100644
> --- a/tcg/ppc/tcg-target.c.inc
> +++ b/tcg/ppc/tcg-target.c.inc
> @@ -325,9 +325,11 @@ static bool tcg_target_const_match(int64_t sval, int ct,
>               if ((uval & ~0xffff) == 0 || (uval & ~0xffff0000ull) == 0) {
>                   return 1;
>               }
> -            if (TCG_TARGET_REG_BITS == 32 || type == TCG_TYPE_I32
> -                ? mask_operand(uval, &mb, &me)
> -                : mask64_operand(uval << clz64(uval), &mb, &me)) {
> +            if (uval == (uint32_t)uval && mask_operand(uval, &mb, &me)) {
> +                return 1;
> +            }
> +            if (TCG_TARGET_REG_BITS == 64 &&
> +                mask64_operand(uval << clz64(uval), &mb, &me)) {
>                   return 1;
>               }
>               return 0;
> @@ -1749,8 +1751,6 @@ static void tcg_out_test(TCGContext *s, TCGReg dest, TCGReg arg1, TCGArg arg2,
>   
>       if (type == TCG_TYPE_I32) {
>           arg2 = (uint32_t)arg2;
> -    } else if (arg2 == (uint32_t)arg2) {
> -        type = TCG_TYPE_I32;
>       }
>   
>       if ((arg2 & ~0xffff) == 0) {
> @@ -1761,12 +1761,11 @@ static void tcg_out_test(TCGContext *s, TCGReg dest, TCGReg arg1, TCGArg arg2,
>           tcg_out32(s, ANDIS | SAI(arg1, dest, arg2 >> 16));
>           return;
>       }
> -    if (TCG_TARGET_REG_BITS == 32 || type == TCG_TYPE_I32) {
> -        if (mask_operand(arg2, &mb, &me)) {
> -            tcg_out_rlw_rc(s, RLWINM, dest, arg1, 0, mb, me, rc);
> -            return;
> -        }
> -    } else {
> +    if (arg2 == (uint32_t)arg2 && mask_operand(arg2, &mb, &me)) {
> +        tcg_out_rlw_rc(s, RLWINM, dest, arg1, 0, mb, me, rc);
> +        return;
> +    }
> +    if (TCG_TARGET_REG_BITS == 64) {
>           int sh = clz64(arg2);
>           if (mask64_operand(arg2 << sh, &mb, &me)) {
>               tcg_out_rld_rc(s, RLDICR, dest, arg1, sh, me, rc);

Preferably having someone from PPC also reviewing this,

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Nicholas Piggin Aug. 9, 2024, 6:11 a.m. UTC | #2 
On Thu Aug 8, 2024 at 2:46 AM AEST, Philippe Mathieu-Daudé wrote:
> On 7/8/24 06:08, Richard Henderson wrote:
> > Ensure the code structure is the same for matching constraints
> > and emitting code, lest we allow constants that cannot be
> > trivially tested.
> > 
> > Cc: qemu-stable@nongnu.org
> > Fixes: ad788aebbab ("tcg/ppc: Support TCG_COND_TST{EQ,NE}")
> > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2487
> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> > ---
> >   tcg/ppc/tcg-target.c.inc | 21 ++++++++++-----------
> >   1 file changed, 10 insertions(+), 11 deletions(-)
> > 
> > diff --git a/tcg/ppc/tcg-target.c.inc b/tcg/ppc/tcg-target.c.inc
> > index 7f3829beeb..3553a47ba9 100644
> > --- a/tcg/ppc/tcg-target.c.inc
> > +++ b/tcg/ppc/tcg-target.c.inc
> > @@ -325,9 +325,11 @@ static bool tcg_target_const_match(int64_t sval, int ct,
> >               if ((uval & ~0xffff) == 0 || (uval & ~0xffff0000ull) == 0) {
> >                   return 1;
> >               }
> > -            if (TCG_TARGET_REG_BITS == 32 || type == TCG_TYPE_I32
> > -                ? mask_operand(uval, &mb, &me)
> > -                : mask64_operand(uval << clz64(uval), &mb, &me)) {
> > +            if (uval == (uint32_t)uval && mask_operand(uval, &mb, &me)) {
> > +                return 1;
> > +            }
> > +            if (TCG_TARGET_REG_BITS == 64 &&
> > +                mask64_operand(uval << clz64(uval), &mb, &me)) {
> >                   return 1;
> >               }
> >               return 0;
> > @@ -1749,8 +1751,6 @@ static void tcg_out_test(TCGContext *s, TCGReg dest, TCGReg arg1, TCGArg arg2,
> >   
> >       if (type == TCG_TYPE_I32) {
> >           arg2 = (uint32_t)arg2;
> > -    } else if (arg2 == (uint32_t)arg2) {
> > -        type = TCG_TYPE_I32;
> >       }
> >   
> >       if ((arg2 & ~0xffff) == 0) {
> > @@ -1761,12 +1761,11 @@ static void tcg_out_test(TCGContext *s, TCGReg dest, TCGReg arg1, TCGArg arg2,
> >           tcg_out32(s, ANDIS | SAI(arg1, dest, arg2 >> 16));
> >           return;
> >       }
> > -    if (TCG_TARGET_REG_BITS == 32 || type == TCG_TYPE_I32) {
> > -        if (mask_operand(arg2, &mb, &me)) {
> > -            tcg_out_rlw_rc(s, RLWINM, dest, arg1, 0, mb, me, rc);
> > -            return;
> > -        }
> > -    } else {
> > +    if (arg2 == (uint32_t)arg2 && mask_operand(arg2, &mb, &me)) {
> > +        tcg_out_rlw_rc(s, RLWINM, dest, arg1, 0, mb, me, rc);
> > +        return;
> > +    }
> > +    if (TCG_TARGET_REG_BITS == 64) {
> >           int sh = clz64(arg2);
> >           if (mask64_operand(arg2 << sh, &mb, &me)) {
> >               tcg_out_rld_rc(s, RLDICR, dest, arg1, sh, me, rc);
>
> Preferably having someone from PPC also reviewing this,
>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

I don't know much of the back end, but I think I follow where the
match was letting through a constant that could not be emitted.
Thanks for the fix and review.

Bigger problem seems to be I'm not testing x86 on ppc so I'll have
to rectify that. Also IIRC we were supposed to be adding a ppc
runner to gitlab CI, I'll have to check what's happening with that,
I think it got stalled on IT security rigmarole :(

Thanks,
Nick
diff mbox series

Patch

diff --git a/tcg/ppc/tcg-target.c.inc b/tcg/ppc/tcg-target.c.inc
index 7f3829beeb..3553a47ba9 100644
--- a/tcg/ppc/tcg-target.c.inc
+++ b/tcg/ppc/tcg-target.c.inc
@@ -325,9 +325,11 @@  static bool tcg_target_const_match(int64_t sval, int ct,
             if ((uval & ~0xffff) == 0 || (uval & ~0xffff0000ull) == 0) {
                 return 1;
             }
-            if (TCG_TARGET_REG_BITS == 32 || type == TCG_TYPE_I32
-                ? mask_operand(uval, &mb, &me)
-                : mask64_operand(uval << clz64(uval), &mb, &me)) {
+            if (uval == (uint32_t)uval && mask_operand(uval, &mb, &me)) {
+                return 1;
+            }
+            if (TCG_TARGET_REG_BITS == 64 &&
+                mask64_operand(uval << clz64(uval), &mb, &me)) {
                 return 1;
             }
             return 0;
@@ -1749,8 +1751,6 @@  static void tcg_out_test(TCGContext *s, TCGReg dest, TCGReg arg1, TCGArg arg2,
 
     if (type == TCG_TYPE_I32) {
         arg2 = (uint32_t)arg2;
-    } else if (arg2 == (uint32_t)arg2) {
-        type = TCG_TYPE_I32;
     }
 
     if ((arg2 & ~0xffff) == 0) {
@@ -1761,12 +1761,11 @@  static void tcg_out_test(TCGContext *s, TCGReg dest, TCGReg arg1, TCGArg arg2,
         tcg_out32(s, ANDIS | SAI(arg1, dest, arg2 >> 16));
         return;
     }
-    if (TCG_TARGET_REG_BITS == 32 || type == TCG_TYPE_I32) {
-        if (mask_operand(arg2, &mb, &me)) {
-            tcg_out_rlw_rc(s, RLWINM, dest, arg1, 0, mb, me, rc);
-            return;
-        }
-    } else {
+    if (arg2 == (uint32_t)arg2 && mask_operand(arg2, &mb, &me)) {
+        tcg_out_rlw_rc(s, RLWINM, dest, arg1, 0, mb, me, rc);
+        return;
+    }
+    if (TCG_TARGET_REG_BITS == 64) {
         int sh = clz64(arg2);
         if (mask64_operand(arg2 << sh, &mb, &me)) {
             tcg_out_rld_rc(s, RLDICR, dest, arg1, sh, me, rc);