diff mbox series

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

Message ID 20231017122702.39311-1-philmd@linaro.org
State New
Headers show
Series [RFC] target/m68k: Use i128 for 128-bit load/store in m68k_copy_line() | expand

Commit Message

Philippe Mathieu-Daudé Oct. 17, 2023, 12:27 p.m. UTC
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(-)

Comments

Richard Henderson Oct. 17, 2023, 1:44 p.m. UTC | #1
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~
Richard Henderson Oct. 17, 2023, 1:52 p.m. UTC | #2
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~
Andreas Schwab Oct. 30, 2023, 1:51 p.m. UTC | #3
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).
diff mbox series

Patch

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)