[RFC PATCH] target/m68k: Use i128 for 128-bit load/store in m68k_copy_line()

Philippe Mathieu-Daudé posted 1 patch 7 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20231017122702.39311-1-philmd@linaro.org
Maintainers: Laurent Vivier <laurent@vivier.eu>
target/m68k/translate.c | 14 ++++----------
1 file changed, 4 insertions(+), 10 deletions(-)
[RFC PATCH] target/m68k: Use i128 for 128-bit load/store in m68k_copy_line()
Posted by Philippe Mathieu-Daudé 7 months ago
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
Based-on: <20231013175109.124308-1-richard.henderson@linaro.org>
  tcg: Add tcg_gen_{ld,st}_i128

RFC because unsure and untested...
---
 target/m68k/translate.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/target/m68k/translate.c b/target/m68k/translate.c
index 4d0110de95..1e3d155bd9 100644
--- a/target/m68k/translate.c
+++ b/target/m68k/translate.c
@@ -4293,23 +4293,17 @@ DISAS_INSN(chk2)
 
 static void m68k_copy_line(TCGv dst, TCGv src, int index)
 {
+    MemOp mop = MO_128 | MO_TE;
+    TCGv_i128 t = tcg_temp_new_i128();
     TCGv addr;
-    TCGv_i64 t0, t1;
 
     addr = tcg_temp_new();
 
-    t0 = tcg_temp_new_i64();
-    t1 = tcg_temp_new_i64();
-
     tcg_gen_andi_i32(addr, src, ~15);
-    tcg_gen_qemu_ld_i64(t0, addr, index, MO_TEUQ);
-    tcg_gen_addi_i32(addr, addr, 8);
-    tcg_gen_qemu_ld_i64(t1, addr, index, MO_TEUQ);
+    tcg_gen_qemu_ld_i128(t, addr, index, mop);
 
     tcg_gen_andi_i32(addr, dst, ~15);
-    tcg_gen_qemu_st_i64(t0, addr, index, MO_TEUQ);
-    tcg_gen_addi_i32(addr, addr, 8);
-    tcg_gen_qemu_st_i64(t1, addr, index, MO_TEUQ);
+    tcg_gen_st_i128(t, addr, index);
 }
 
 DISAS_INSN(move16_reg)
-- 
2.41.0


Re: [RFC PATCH] target/m68k: Use i128 for 128-bit load/store in m68k_copy_line()
Posted by Richard Henderson 7 months ago
On 10/17/23 05:27, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> Based-on: <20231013175109.124308-1-richard.henderson@linaro.org>
>    tcg: Add tcg_gen_{ld,st}_i128
> 
> RFC because unsure and untested...
> ---
>   target/m68k/translate.c | 14 ++++----------
>   1 file changed, 4 insertions(+), 10 deletions(-)
> 
> diff --git a/target/m68k/translate.c b/target/m68k/translate.c
> index 4d0110de95..1e3d155bd9 100644
> --- a/target/m68k/translate.c
> +++ b/target/m68k/translate.c
> @@ -4293,23 +4293,17 @@ DISAS_INSN(chk2)
>   
>   static void m68k_copy_line(TCGv dst, TCGv src, int index)
>   {
> +    MemOp mop = MO_128 | MO_TE;
> +    TCGv_i128 t = tcg_temp_new_i128();
>       TCGv addr;
> -    TCGv_i64 t0, t1;
>   
>       addr = tcg_temp_new();
>   
> -    t0 = tcg_temp_new_i64();
> -    t1 = tcg_temp_new_i64();
> -
>       tcg_gen_andi_i32(addr, src, ~15);
> -    tcg_gen_qemu_ld_i64(t0, addr, index, MO_TEUQ);
> -    tcg_gen_addi_i32(addr, addr, 8);
> -    tcg_gen_qemu_ld_i64(t1, addr, index, MO_TEUQ);
> +    tcg_gen_qemu_ld_i128(t, addr, index, mop);
>   
>       tcg_gen_andi_i32(addr, dst, ~15);
> -    tcg_gen_qemu_st_i64(t0, addr, index, MO_TEUQ);
> -    tcg_gen_addi_i32(addr, addr, 8);
> -    tcg_gen_qemu_st_i64(t1, addr, index, MO_TEUQ);
> +    tcg_gen_st_i128(t, addr, index);

Aside from the typo, the other thing you need to consider when introducing 16-byte 
operations is the atomicity.  Do you want or need this to be atomic?

For m68k, I strongly suspect that we don't need the entire read or write to be atomic.

The manual says "burst reads and writes" without defining those terms.  I suspect that 
MO_ATOM_NONE is sufficient, but MO_ATOM_IFALIGN_PAIR would preserve the atomicity of the 
current code.


r~


Re: [RFC PATCH] target/m68k: Use i128 for 128-bit load/store in m68k_copy_line()
Posted by Andreas Schwab 6 months, 3 weeks ago
On Okt 17 2023, Richard Henderson wrote:

> The manual says "burst reads and writes" without defining those terms.

Burst transfers are explained in the M68040UM (7.4.2 Line Read Transfer
and 7.4.4 Line Write Transfers).

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
Re: [RFC PATCH] target/m68k: Use i128 for 128-bit load/store in m68k_copy_line()
Posted by Richard Henderson 7 months ago
On 10/17/23 05:27, Philippe Mathieu-Daudé wrote:
> -    tcg_gen_qemu_st_i64(t1, addr, index, MO_TEUQ);
> +    tcg_gen_st_i128(t, addr, index);

Lost the "qemu".


r~