* [PATCH for-9.1] tcg/ppc: Sync tcg_out_test and constraints
@ 2024-08-07 4:08 Richard Henderson
2024-08-07 16:46 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 3+ messages in thread
From: Richard Henderson @ 2024-08-07 4:08 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-stable
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);
--
2.43.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH for-9.1] tcg/ppc: Sync tcg_out_test and constraints
2024-08-07 4:08 [PATCH for-9.1] tcg/ppc: Sync tcg_out_test and constraints Richard Henderson
@ 2024-08-07 16:46 ` Philippe Mathieu-Daudé
2024-08-09 6:11 ` Nicholas Piggin
0 siblings, 1 reply; 3+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-08-07 16:46 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: qemu-stable, qemu-ppc
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>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH for-9.1] tcg/ppc: Sync tcg_out_test and constraints
2024-08-07 16:46 ` Philippe Mathieu-Daudé
@ 2024-08-09 6:11 ` Nicholas Piggin
0 siblings, 0 replies; 3+ messages in thread
From: Nicholas Piggin @ 2024-08-09 6:11 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, Richard Henderson, qemu-devel
Cc: qemu-stable, qemu-ppc
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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-08-09 6:12 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-08-07 4:08 [PATCH for-9.1] tcg/ppc: Sync tcg_out_test and constraints Richard Henderson
2024-08-07 16:46 ` Philippe Mathieu-Daudé
2024-08-09 6:11 ` Nicholas Piggin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).