qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH] tcg: Restrict qemu_ld2 and qemu_st2 opcodes to 32-bit hosts
@ 2025-08-20 13:49 Philippe Mathieu-Daudé
  2025-08-21 20:27 ` Richard Henderson
  0 siblings, 1 reply; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-08-20 13:49 UTC (permalink / raw)
  To: qemu-devel
  Cc: Richard Henderson, Pierrick Bouvier, Philippe Mathieu-Daudé

qemu_ld2 and qemu_st2 opcodes are band-aid for 32-bit hosts
and can't be reached on 64-bit ones. See in commit 3bedb9d3e28
("tcg: Convert qemu_ld{2} to TCGOutOpLoad{2}") and 86fe5c2597c
("tcg: Convert qemu_st{2} to TCGOutOpLdSt{2}") their constraint
is C_NotImplemented.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 tcg/tcg.c | 27 ++++++++++++++-------------
 1 file changed, 14 insertions(+), 13 deletions(-)

diff --git a/tcg/tcg.c b/tcg/tcg.c
index afac55a203a..ff1a8b71789 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -1221,9 +1221,7 @@ static const TCGOutOp * const all_outop[NB_OPS] = {
     OUTOP(INDEX_op_or, TCGOutOpBinary, outop_or),
     OUTOP(INDEX_op_orc, TCGOutOpBinary, outop_orc),
     OUTOP(INDEX_op_qemu_ld, TCGOutOpQemuLdSt, outop_qemu_ld),
-    OUTOP(INDEX_op_qemu_ld2, TCGOutOpQemuLdSt2, outop_qemu_ld2),
     OUTOP(INDEX_op_qemu_st, TCGOutOpQemuLdSt, outop_qemu_st),
-    OUTOP(INDEX_op_qemu_st2, TCGOutOpQemuLdSt2, outop_qemu_st2),
     OUTOP(INDEX_op_rems, TCGOutOpBinary, outop_rems),
     OUTOP(INDEX_op_remu, TCGOutOpBinary, outop_remu),
     OUTOP(INDEX_op_rotl, TCGOutOpBinary, outop_rotl),
@@ -1248,6 +1246,8 @@ static const TCGOutOp * const all_outop[NB_OPS] = {
 
 #if TCG_TARGET_REG_BITS == 32
     OUTOP(INDEX_op_brcond2_i32, TCGOutOpBrcond2, outop_brcond2),
+    OUTOP(INDEX_op_qemu_ld2, TCGOutOpQemuLdSt2, outop_qemu_ld2),
+    OUTOP(INDEX_op_qemu_st2, TCGOutOpQemuLdSt2, outop_qemu_st2),
     OUTOP(INDEX_op_setcond2_i32, TCGOutOpSetcond2, outop_setcond2),
 #else
     OUTOP(INDEX_op_bswap64, TCGOutOpUnary, outop_bswap64),
@@ -5829,17 +5829,6 @@ static void tcg_reg_alloc_op(TCGContext *s, const TCGOp *op)
         }
         break;
 
-    case INDEX_op_qemu_ld2:
-    case INDEX_op_qemu_st2:
-        {
-            const TCGOutOpQemuLdSt2 *out =
-                container_of(all_outop[op->opc], TCGOutOpQemuLdSt2, base);
-
-            out->out(s, type, new_args[0], new_args[1],
-                     new_args[2], new_args[3]);
-        }
-        break;
-
     case INDEX_op_brcond:
         {
             const TCGOutOpBrcond *out = &outop_brcond;
@@ -5887,6 +5876,16 @@ static void tcg_reg_alloc_op(TCGContext *s, const TCGOp *op)
         break;
 
 #if TCG_TARGET_REG_BITS == 32
+    case INDEX_op_qemu_ld2:
+    case INDEX_op_qemu_st2:
+        {
+            const TCGOutOpQemuLdSt2 *out =
+                container_of(all_outop[op->opc], TCGOutOpQemuLdSt2, base);
+
+            out->out(s, type, new_args[0], new_args[1],
+                     new_args[2], new_args[3]);
+        }
+        break;
     case INDEX_op_brcond2_i32:
         {
             const TCGOutOpBrcond2 *out = &outop_brcond2;
@@ -5912,6 +5911,8 @@ static void tcg_reg_alloc_op(TCGContext *s, const TCGOp *op)
         }
         break;
 #else
+    case INDEX_op_qemu_ld2:
+    case INDEX_op_qemu_st2:
     case INDEX_op_brcond2_i32:
     case INDEX_op_setcond2_i32:
         g_assert_not_reached();
-- 
2.51.0



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [RFC PATCH] tcg: Restrict qemu_ld2 and qemu_st2 opcodes to 32-bit hosts
  2025-08-20 13:49 [RFC PATCH] tcg: Restrict qemu_ld2 and qemu_st2 opcodes to 32-bit hosts Philippe Mathieu-Daudé
@ 2025-08-21 20:27 ` Richard Henderson
  2025-10-09 17:23   ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 7+ messages in thread
From: Richard Henderson @ 2025-08-21 20:27 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel; +Cc: Pierrick Bouvier

On 8/20/25 23:49, Philippe Mathieu-Daudé wrote:
> qemu_ld2 and qemu_st2 opcodes are band-aid for 32-bit hosts
> and can't be reached on 64-bit ones. See in commit 3bedb9d3e28
> ("tcg: Convert qemu_ld{2} to TCGOutOpLoad{2}") and 86fe5c2597c
> ("tcg: Convert qemu_st{2} to TCGOutOpLdSt{2}") their constraint
> is C_NotImplemented.

Not true: ld2/st2 are also used for 128-bit load store.

See: aarch64, x86_64, loongarch64, ppc64, s390x.


r~


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC PATCH] tcg: Restrict qemu_ld2 and qemu_st2 opcodes to 32-bit hosts
  2025-08-21 20:27 ` Richard Henderson
@ 2025-10-09 17:23   ` Philippe Mathieu-Daudé
  2025-10-09 17:38     ` Richard Henderson
  2025-10-09 17:43     ` Philippe Mathieu-Daudé
  0 siblings, 2 replies; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-10-09 17:23 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel; +Cc: Pierrick Bouvier

On 21/8/25 22:27, Richard Henderson wrote:
> On 8/20/25 23:49, Philippe Mathieu-Daudé wrote:
>> qemu_ld2 and qemu_st2 opcodes are band-aid for 32-bit hosts
>> and can't be reached on 64-bit ones. See in commit 3bedb9d3e28
>> ("tcg: Convert qemu_ld{2} to TCGOutOpLoad{2}") and 86fe5c2597c
>> ("tcg: Convert qemu_st{2} to TCGOutOpLdSt{2}") their constraint
>> is C_NotImplemented.
> 
> Not true: ld2/st2 are also used for 128-bit load store.
> 
> See: aarch64, x86_64, loongarch64, ppc64, s390x.

OK. Hmm I guess I need to add a __attribute__((unused)) check like
you did in commit f408df587a0 ("tcg: Convert brcond2_i32 to
TCGOutOpBrcond2") to outop_qemu_ld2/st2 then.


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC PATCH] tcg: Restrict qemu_ld2 and qemu_st2 opcodes to 32-bit hosts
  2025-10-09 17:23   ` Philippe Mathieu-Daudé
@ 2025-10-09 17:38     ` Richard Henderson
  2025-10-09 19:13       ` Philippe Mathieu-Daudé
  2025-10-09 17:43     ` Philippe Mathieu-Daudé
  1 sibling, 1 reply; 7+ messages in thread
From: Richard Henderson @ 2025-10-09 17:38 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel; +Cc: Pierrick Bouvier

On 10/9/25 10:23, Philippe Mathieu-Daudé wrote:
> On 21/8/25 22:27, Richard Henderson wrote:
>> On 8/20/25 23:49, Philippe Mathieu-Daudé wrote:
>>> qemu_ld2 and qemu_st2 opcodes are band-aid for 32-bit hosts
>>> and can't be reached on 64-bit ones. See in commit 3bedb9d3e28
>>> ("tcg: Convert qemu_ld{2} to TCGOutOpLoad{2}") and 86fe5c2597c
>>> ("tcg: Convert qemu_st{2} to TCGOutOpLdSt{2}") their constraint
>>> is C_NotImplemented.
>>
>> Not true: ld2/st2 are also used for 128-bit load store.
>>
>> See: aarch64, x86_64, loongarch64, ppc64, s390x.
> 
> OK. Hmm I guess I need to add a __attribute__((unused)) check like
> you did in commit f408df587a0 ("tcg: Convert brcond2_i32 to
> TCGOutOpBrcond2") to outop_qemu_ld2/st2 then.

Why?  Please expand on the error you're seeing.


r~


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC PATCH] tcg: Restrict qemu_ld2 and qemu_st2 opcodes to 32-bit hosts
  2025-10-09 17:23   ` Philippe Mathieu-Daudé
  2025-10-09 17:38     ` Richard Henderson
@ 2025-10-09 17:43     ` Philippe Mathieu-Daudé
  2025-10-09 19:08       ` Richard Henderson
  1 sibling, 1 reply; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-10-09 17:43 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel; +Cc: Pierrick Bouvier

On 9/10/25 19:23, Philippe Mathieu-Daudé wrote:
> On 21/8/25 22:27, Richard Henderson wrote:
>> On 8/20/25 23:49, Philippe Mathieu-Daudé wrote:
>>> qemu_ld2 and qemu_st2 opcodes are band-aid for 32-bit hosts
>>> and can't be reached on 64-bit ones. See in commit 3bedb9d3e28
>>> ("tcg: Convert qemu_ld{2} to TCGOutOpLoad{2}") and 86fe5c2597c
>>> ("tcg: Convert qemu_st{2} to TCGOutOpLdSt{2}") their constraint
>>> is C_NotImplemented.
>>
>> Not true: ld2/st2 are also used for 128-bit load store.
>>
>> See: aarch64, x86_64, loongarch64, ppc64, s390x.
> 
> OK. Hmm I guess I need to add a __attribute__((unused)) check like
> you did in commit f408df587a0 ("tcg: Convert brcond2_i32 to
> TCGOutOpBrcond2") to outop_qemu_ld2/st2 then.

OK I figured my issue, I shouldn't remove the following in the
MIPS series [*]:

-- >8 --
diff --git a/tcg/mips/tcg-target.c.inc b/tcg/mips/tcg-target.c.inc
index 3f4aca25b13..c519dfbf7d5 100644
--- a/tcg/mips/tcg-target.c.inc
+++ b/tcg/mips/tcg-target.c.inc
@@ -1311,2 +1311,34 @@ static const TCGOutOpQemuLdSt outop_qemu_ld = {

+static void tgen_qemu_ld2(TCGContext *s, TCGType type, TCGReg datalo,
+                          TCGReg datahi, TCGReg addr, MemOpIdx oi)
+{
+    MemOp opc = get_memop(oi);
+    TCGLabelQemuLdst *ldst;
+    HostAddress h;
+
+    tcg_debug_assert(TCG_TARGET_REG_BITS == 32);
+    ldst = prepare_host_addr(s, &h, addr, oi, true);
+
+    if (use_mips32r6_instructions || h.aa.align >= (opc & MO_SIZE)) {
+        tcg_out_qemu_ld_direct(s, datalo, datahi, h.base, opc, type);
+    } else {
+        tcg_out_qemu_ld_unalign(s, datalo, datahi, h.base, opc, type);
+    }
+
+    if (ldst) {
+        ldst->type = type;
+        ldst->datalo_reg = datalo;
+        ldst->datahi_reg = datahi;
+        ldst->raddr = tcg_splitwx_to_rx(s->code_ptr);
+    }
+}
+
+static const TCGOutOpQemuLdSt2 outop_qemu_ld2 = {
+    /* Ensure that the mips32 code is compiled but discarded for mips64. */
+    .base.static_constraint =
+        TCG_TARGET_REG_BITS == 32 ? C_O2_I1(r, r, r) : C_NotImplemented,
+    .out =
+        TCG_TARGET_REG_BITS == 32 ? tgen_qemu_ld2 : NULL,
+};
+
  static void tcg_out_qemu_st_direct(TCGContext *s, TCGReg lo, TCGReg hi,
@@ -1390,2 +1422,34 @@ static const TCGOutOpQemuLdSt outop_qemu_st = {

+static void tgen_qemu_st2(TCGContext *s, TCGType type, TCGReg datalo,
+                          TCGReg datahi, TCGReg addr, MemOpIdx oi)
+{
+    MemOp opc = get_memop(oi);
+    TCGLabelQemuLdst *ldst;
+    HostAddress h;
+
+    tcg_debug_assert(TCG_TARGET_REG_BITS == 32);
+    ldst = prepare_host_addr(s, &h, addr, oi, false);
+
+    if (use_mips32r6_instructions || h.aa.align >= (opc & MO_SIZE)) {
+        tcg_out_qemu_st_direct(s, datalo, datahi, h.base, opc);
+    } else {
+        tcg_out_qemu_st_unalign(s, datalo, datahi, h.base, opc);
+    }
+
+    if (ldst) {
+        ldst->type = type;
+        ldst->datalo_reg = datalo;
+        ldst->datahi_reg = datahi;
+        ldst->raddr = tcg_splitwx_to_rx(s->code_ptr);
+    }
+}
+
+static const TCGOutOpQemuLdSt2 outop_qemu_st2 = {
+    /* Ensure that the mips32 code is compiled but discarded for mips64. */
+    .base.static_constraint =
+        TCG_TARGET_REG_BITS == 32 ? C_O0_I3(rz, rz, r) : C_NotImplemented,
+    .out =
+        TCG_TARGET_REG_BITS == 32 ? tgen_qemu_st2 : NULL,
+};
+
  static void tcg_out_mb(TCGContext *s, unsigned a0)

---

Let's ignore this bogus patch then :)

[*] 
https://lore.kernel.org/qemu-devel/20250820142108.46639-1-philmd@linaro.org/


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [RFC PATCH] tcg: Restrict qemu_ld2 and qemu_st2 opcodes to 32-bit hosts
  2025-10-09 17:43     ` Philippe Mathieu-Daudé
@ 2025-10-09 19:08       ` Richard Henderson
  0 siblings, 0 replies; 7+ messages in thread
From: Richard Henderson @ 2025-10-09 19:08 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel; +Cc: Pierrick Bouvier

On 10/9/25 10:43, Philippe Mathieu-Daudé wrote:
> On 9/10/25 19:23, Philippe Mathieu-Daudé wrote:
>> On 21/8/25 22:27, Richard Henderson wrote:
>>> On 8/20/25 23:49, Philippe Mathieu-Daudé wrote:
>>>> qemu_ld2 and qemu_st2 opcodes are band-aid for 32-bit hosts
>>>> and can't be reached on 64-bit ones. See in commit 3bedb9d3e28
>>>> ("tcg: Convert qemu_ld{2} to TCGOutOpLoad{2}") and 86fe5c2597c
>>>> ("tcg: Convert qemu_st{2} to TCGOutOpLdSt{2}") their constraint
>>>> is C_NotImplemented.
>>>
>>> Not true: ld2/st2 are also used for 128-bit load store.
>>>
>>> See: aarch64, x86_64, loongarch64, ppc64, s390x.
>>
>> OK. Hmm I guess I need to add a __attribute__((unused)) check like
>> you did in commit f408df587a0 ("tcg: Convert brcond2_i32 to
>> TCGOutOpBrcond2") to outop_qemu_ld2/st2 then.
> 
> OK I figured my issue, I shouldn't remove the following in the
> MIPS series [*]:
> 
> -- >8 --
> diff --git a/tcg/mips/tcg-target.c.inc b/tcg/mips/tcg-target.c.inc
> index 3f4aca25b13..c519dfbf7d5 100644
> --- a/tcg/mips/tcg-target.c.inc
> +++ b/tcg/mips/tcg-target.c.inc
> @@ -1311,2 +1311,34 @@ static const TCGOutOpQemuLdSt outop_qemu_ld = {
> 
> +static void tgen_qemu_ld2(TCGContext *s, TCGType type, TCGReg datalo,
> +                          TCGReg datahi, TCGReg addr, MemOpIdx oi)
> +{
> +    MemOp opc = get_memop(oi);
> +    TCGLabelQemuLdst *ldst;
> +    HostAddress h;
> +
> +    tcg_debug_assert(TCG_TARGET_REG_BITS == 32);
> +    ldst = prepare_host_addr(s, &h, addr, oi, true);
> +
> +    if (use_mips32r6_instructions || h.aa.align >= (opc & MO_SIZE)) {
> +        tcg_out_qemu_ld_direct(s, datalo, datahi, h.base, opc, type);
> +    } else {
> +        tcg_out_qemu_ld_unalign(s, datalo, datahi, h.base, opc, type);
> +    }
> +
> +    if (ldst) {
> +        ldst->type = type;
> +        ldst->datalo_reg = datalo;
> +        ldst->datahi_reg = datahi;
> +        ldst->raddr = tcg_splitwx_to_rx(s->code_ptr);
> +    }
> +}
> +
> +static const TCGOutOpQemuLdSt2 outop_qemu_ld2 = {
> +    /* Ensure that the mips32 code is compiled but discarded for mips64. */
> +    .base.static_constraint =
> +        TCG_TARGET_REG_BITS == 32 ? C_O2_I1(r, r, r) : C_NotImplemented,
> +    .out =
> +        TCG_TARGET_REG_BITS == 32 ? tgen_qemu_ld2 : NULL,
> +};

In the mips32 removal series, you should be removing the functions and simplifying this 
structure to

static const TCGOutOpQemuLdSt2 outop_qemu_ld2 = {
     .base.static_constraint = C_NotImplemented,
};

> +static const TCGOutOpQemuLdSt2 outop_qemu_st2 = {

Likewise.


r~


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC PATCH] tcg: Restrict qemu_ld2 and qemu_st2 opcodes to 32-bit hosts
  2025-10-09 17:38     ` Richard Henderson
@ 2025-10-09 19:13       ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-10-09 19:13 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel; +Cc: Pierrick Bouvier

On 9/10/25 19:38, Richard Henderson wrote:
> On 10/9/25 10:23, Philippe Mathieu-Daudé wrote:
>> On 21/8/25 22:27, Richard Henderson wrote:
>>> On 8/20/25 23:49, Philippe Mathieu-Daudé wrote:
>>>> qemu_ld2 and qemu_st2 opcodes are band-aid for 32-bit hosts
>>>> and can't be reached on 64-bit ones. See in commit 3bedb9d3e28
>>>> ("tcg: Convert qemu_ld{2} to TCGOutOpLoad{2}") and 86fe5c2597c
>>>> ("tcg: Convert qemu_st{2} to TCGOutOpLdSt{2}") their constraint
>>>> is C_NotImplemented.
>>>
>>> Not true: ld2/st2 are also used for 128-bit load store.
>>>
>>> See: aarch64, x86_64, loongarch64, ppc64, s390x.
>>
>> OK. Hmm I guess I need to add a __attribute__((unused)) check like
>> you did in commit f408df587a0 ("tcg: Convert brcond2_i32 to
>> TCGOutOpBrcond2") to outop_qemu_ld2/st2 then.
> 
> Why?  Please expand on the error you're seeing.

It was a mistake on my side, no worry:
https://lore.kernel.org/qemu-devel/c75dc27a-5c56-4010-a205-a8296a9ab1e0@linaro.org/



^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2025-10-09 19:14 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-20 13:49 [RFC PATCH] tcg: Restrict qemu_ld2 and qemu_st2 opcodes to 32-bit hosts Philippe Mathieu-Daudé
2025-08-21 20:27 ` Richard Henderson
2025-10-09 17:23   ` Philippe Mathieu-Daudé
2025-10-09 17:38     ` Richard Henderson
2025-10-09 19:13       ` Philippe Mathieu-Daudé
2025-10-09 17:43     ` Philippe Mathieu-Daudé
2025-10-09 19:08       ` Richard Henderson

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).