diff mbox series

[v2,02/67] target/arm: Use PLD, PLDW, PLI not NOP for t32

Message ID 20240524232121.284515-3-richard.henderson@linaro.org
State Superseded
Headers show
Series target/arm: Convert a64 advsimd to decodetree (part 1) | expand

Commit Message

Richard Henderson May 24, 2024, 11:20 p.m. UTC 
This fixes a bug in that neither PLI nor PLDW are present in ARMv6T2,
but are introduced with ARMv7 and ARMv7MP respectively.
For clarity, do not use NOP for PLD.

Note that there is no PLDW (literal) -- bit 5 of the first word
is not decoded, and is PLD (literal).  Confirmed on neoverse-n1
host which does *not* trap on the (0) bit in the decode.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/tcg/t32.decode  | 25 ++++++++++++-------------
 target/arm/tcg/translate.c |  4 ++--
 2 files changed, 14 insertions(+), 15 deletions(-)

Comments

Peter Maydell May 28, 2024, 1:13 p.m. UTC | #1 
On Sat, 25 May 2024 at 00:23, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> This fixes a bug in that neither PLI nor PLDW are present in ARMv6T2,
> but are introduced with ARMv7 and ARMv7MP respectively.
> For clarity, do not use NOP for PLD.
>
> Note that there is no PLDW (literal) -- bit 5 of the first word
> is not decoded, and is PLD (literal).  Confirmed on neoverse-n1
> host which does *not* trap on the (0) bit in the decode.

Handling of "(0)" and "(1)" bits in decode is CONSTRAINED UNPREDICTABLE
(see Arm ARM DDI0487K.a F1.7.2): implementations might:
 * UNDEF
 * NOP
 * ignore the bit and execute the insn
 * set any destination registers to UNKNOWN values

Usually (but not always) in QEMU we opt to UNDEF. If I'm
reading the decode lines correctly here in this case we're
opting to ignore the bit because we have both:

+    PLD          1111 1000 -001 1111 1111 ------------        # (literal)
+    PLD          1111 1000 -011 1111 1111 ------------        # (literal)

And this isn't a change in this commit because the NOP lines
we used to have also meant we ignored the bit.

With a tweaked commit message
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM
diff mbox series

Patch

diff --git a/target/arm/tcg/t32.decode b/target/arm/tcg/t32.decode
index f21ad0167a..d327178829 100644
--- a/target/arm/tcg/t32.decode
+++ b/target/arm/tcg/t32.decode
@@ -458,41 +458,41 @@  STR_ri           1111 1000 1100 .... .... ............        @ldst_ri_pos
 # Note that Load, unsigned (literal) overlaps all other load encodings.
 {
   {
-    NOP          1111 1000 -001 1111 1111 ------------        # PLD
+    PLD          1111 1000 -001 1111 1111 ------------        # (literal)
     LDRB_ri      1111 1000 .001 1111 .... ............        @ldst_ri_lit
   }
   {
-    NOP          1111 1000 1001 ---- 1111 ------------        # PLD
+    PLD          1111 1000 1001 ---- 1111 ------------        # (immediate T1)
     LDRB_ri      1111 1000 1001 .... .... ............        @ldst_ri_pos
   }
   LDRB_ri        1111 1000 0001 .... .... 1..1 ........       @ldst_ri_idx
   {
-    NOP          1111 1000 0001 ---- 1111 1100 --------       # PLD
+    PLD          1111 1000 0001 ---- 1111 1100 --------       # (immediate T2)
     LDRB_ri      1111 1000 0001 .... .... 1100 ........       @ldst_ri_neg
   }
   LDRBT_ri       1111 1000 0001 .... .... 1110 ........       @ldst_ri_unp
   {
-    NOP          1111 1000 0001 ---- 1111 000000 -- ----      # PLD
+    PLD          1111 1000 0001 ---- 1111 000000 -- ----      # (register)
     LDRB_rr      1111 1000 0001 .... .... 000000 .. ....      @ldst_rr
   }
 }
 {
   {
-    NOP          1111 1000 -011 1111 1111 ------------        # PLD
+    PLD          1111 1000 -011 1111 1111 ------------        # (literal)
     LDRH_ri      1111 1000 .011 1111 .... ............        @ldst_ri_lit
   }
   {
-    NOP          1111 1000 1011 ---- 1111 ------------        # PLDW
+    PLDW         1111 1000 1011 ---- 1111 ------------        # (immediate T1)
     LDRH_ri      1111 1000 1011 .... .... ............        @ldst_ri_pos
   }
   LDRH_ri        1111 1000 0011 .... .... 1..1 ........       @ldst_ri_idx
   {
-    NOP          1111 1000 0011 ---- 1111 1100 --------       # PLDW
+    PLDW         1111 1000 0011 ---- 1111 1100 --------       # (immediate T2)
     LDRH_ri      1111 1000 0011 .... .... 1100 ........       @ldst_ri_neg
   }
   LDRHT_ri       1111 1000 0011 .... .... 1110 ........       @ldst_ri_unp
   {
-    NOP          1111 1000 0011 ---- 1111 000000 -- ----      # PLDW
+    PLDW         1111 1000 0011 ---- 1111 000000 -- ----      # (register)
     LDRH_rr      1111 1000 0011 .... .... 000000 .. ....      @ldst_rr
   }
 }
@@ -504,24 +504,23 @@  STR_ri           1111 1000 1100 .... .... ............        @ldst_ri_pos
   LDRT_ri        1111 1000 0101 .... .... 1110 ........       @ldst_ri_unp
   LDR_rr         1111 1000 0101 .... .... 000000 .. ....      @ldst_rr
 }
-# NOPs here are PLI.
 {
   {
-    NOP          1111 1001 -001 1111 1111 ------------
+    PLI          1111 1001 -001 1111 1111 ------------        # (literal T3)
     LDRSB_ri     1111 1001 .001 1111 .... ............        @ldst_ri_lit
   }
   {
-    NOP          1111 1001 1001 ---- 1111 ------------
+    PLI          1111 1001 1001 ---- 1111 ------------        # (immediate T1)
     LDRSB_ri     1111 1001 1001 .... .... ............        @ldst_ri_pos
   }
   LDRSB_ri       1111 1001 0001 .... .... 1..1 ........       @ldst_ri_idx
   {
-    NOP          1111 1001 0001 ---- 1111 1100 --------
+    PLI          1111 1001 0001 ---- 1111 1100 --------       # (immediate T2)
     LDRSB_ri     1111 1001 0001 .... .... 1100 ........       @ldst_ri_neg
   }
   LDRSBT_ri      1111 1001 0001 .... .... 1110 ........       @ldst_ri_unp
   {
-    NOP          1111 1001 0001 ---- 1111 000000 -- ----
+    PLI          1111 1001 0001 ---- 1111 000000 -- ----      # (register)
     LDRSB_rr     1111 1001 0001 .... .... 000000 .. ....      @ldst_rr
   }
 }
diff --git a/target/arm/tcg/translate.c b/target/arm/tcg/translate.c
index d605e10f11..187eacffd9 100644
--- a/target/arm/tcg/translate.c
+++ b/target/arm/tcg/translate.c
@@ -8765,12 +8765,12 @@  static bool trans_PLD(DisasContext *s, arg_PLD *a)
     return ENABLE_ARCH_5TE;
 }
 
-static bool trans_PLDW(DisasContext *s, arg_PLD *a)
+static bool trans_PLDW(DisasContext *s, arg_PLDW *a)
 {
     return arm_dc_feature(s, ARM_FEATURE_V7MP);
 }
 
-static bool trans_PLI(DisasContext *s, arg_PLD *a)
+static bool trans_PLI(DisasContext *s, arg_PLI *a)
 {
     return ENABLE_ARCH_7;
 }