[PATCH 0/4] target/arm: fix some simd writes vs sve

[PATCH 0/4] target/arm: fix some simd writes vs sve

[Richard Henderson]

The launchpad bug only mentions EXT, but I found three more
via inspection.  I really should extend RISU so that we can
do AdvSIMD testing with SVE enabled...


r~


Richard Henderson (4):
  target/arm: Flush high bits of sve register after AdvSIMD EXT
  target/arm: Flush high bits of sve register after AdvSIMD TBL/TBX
  target/arm: Flush high bits of sve register after AdvSIMD ZIP/UZP/TRN
  target/arm: Flush high bits of sve register after AdvSIMD INS

 target/arm/translate-a64.c | 9 +++++++++
 1 file changed, 9 insertions(+)

-- 
2.20.1

[PATCH 1/4] target/arm: Flush high bits of sve register after AdvSIMD EXT

[Richard Henderson]

Writes to AdvSIMD registers flush the bits above 128.

Buglink: https://bugs.launchpad.net/bugs/1863247
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/translate-a64.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index 7c26c3bfeb..620a429067 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -6895,6 +6895,7 @@ static void disas_simd_ext(DisasContext *s, uint32_t insn)
     tcg_temp_free_i64(tcg_resl);
     write_vec_element(s, tcg_resh, rd, 1, MO_64);
     tcg_temp_free_i64(tcg_resh);
+    clear_vec_high(s, true, rd);
 }
 
 /* TBL/TBX
-- 
2.20.1

[PATCH 2/4] target/arm: Flush high bits of sve register after AdvSIMD TBL/TBX

[Richard Henderson]

Writes to AdvSIMD registers flush the bits above 128.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/translate-a64.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index 620a429067..096a854aed 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -6964,6 +6964,7 @@ static void disas_simd_tb(DisasContext *s, uint32_t insn)
     tcg_temp_free_i64(tcg_resl);
     write_vec_element(s, tcg_resh, rd, 1, MO_64);
     tcg_temp_free_i64(tcg_resh);
+    clear_vec_high(s, true, rd);
 }
 
 /* ZIP/UZP/TRN
-- 
2.20.1

[PATCH 3/4] target/arm: Flush high bits of sve register after AdvSIMD ZIP/UZP/TRN

[Richard Henderson]

Writes to AdvSIMD registers flush the bits above 128.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/translate-a64.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index 096a854aed..b83d09dbcd 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -7054,6 +7054,7 @@ static void disas_simd_zip_trn(DisasContext *s, uint32_t insn)
     tcg_temp_free_i64(tcg_resl);
     write_vec_element(s, tcg_resh, rd, 1, MO_64);
     tcg_temp_free_i64(tcg_resh);
+    clear_vec_high(s, true, rd);
 }
 
 /*
-- 
2.20.1

[PATCH 4/4] target/arm: Flush high bits of sve register after AdvSIMD INS

[Richard Henderson]

Writes to AdvSIMD registers flush the bits above 128.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/translate-a64.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index b83d09dbcd..bd68588a71 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -7412,6 +7412,9 @@ static void handle_simd_inse(DisasContext *s, int rd, int rn,
     write_vec_element(s, tmp, rd, dst_index, size);
 
     tcg_temp_free_i64(tmp);
+
+    /* INS is considered a 128-bit write for SVE. */
+    clear_vec_high(s, true, rd);
 }
 
 
@@ -7441,6 +7444,9 @@ static void handle_simd_insg(DisasContext *s, int rd, int rn, int imm5)
 
     idx = extract32(imm5, 1 + size, 4 - size);
     write_vec_element(s, cpu_reg(s, rn), rd, idx, size);
+
+    /* INS is considered a 128-bit write for SVE. */
+    clear_vec_high(s, true, rd);
 }
 
 /*
-- 
2.20.1

Re: [PATCH 0/4] target/arm: fix some simd writes vs sve

[Peter Maydell]

On Fri, 14 Feb 2020 at 19:46, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> The launchpad bug only mentions EXT, but I found three more
> via inspection.  I really should extend RISU so that we can
> do AdvSIMD testing with SVE enabled...

Applied to target-arm.next, thanks.

I did a quick pass through for other functions that seem to call
write_vec_element() but not clear_vec_high() -- do any of these
need fixes?
- handle_vec_simd_wshli()
- handle_3rd_widening()
- handle_3rd_wide()
- handle_pmull_64()
- handle_simd_3same_pair(), "size == 3" branch
- handle_2misc_widening()
- handle_rev(), "size != 0" branch
- handle_2misc_pairwise()
- handle_shll()
- disas_simd_indexed() final else branch (the one with the second
  of the two identical comments about "The simplest way to handle
  the 16x16 indexed ops")
- disas_crypto_three_reg_sha512()
- disas_crypto_four_reg()
- disas_crypto_xar()

Probably false positives in that list, I just did a quick eyeball.

We could probably test this with risu by having the startup code
fill in the top parts of the SVE regs with non-zero and then
making sure they're included in the comparison after insns.
You'd need to test each insn in a separate risu run, though,
otherwise the first insn in the block clears the Z regs and we
don't get to tell if the insn after it does...

thanks
-- PMM