diff mbox series

[v2,41/68] target/arm: Convert TT

Message ID 20190819213755.26175-42-richard.henderson@linaro.org
State Superseded
Headers show
Series target/arm: Convert aa32 base isa to decodetree | expand

Commit Message

Richard Henderson Aug. 19, 2019, 9:37 p.m. UTC 
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

---
 target/arm/translate.c | 87 +++++++++++++-----------------------------
 target/arm/t32.decode  |  5 ++-
 2 files changed, 31 insertions(+), 61 deletions(-)

-- 
2.17.1

Comments

Peter Maydell Aug. 25, 2019, 4:33 p.m. UTC | #1 
On Mon, 19 Aug 2019 at 22:38, Richard Henderson
<richard.henderson@linaro.org> wrote:
>

> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

> ---

>  target/arm/translate.c | 87 +++++++++++++-----------------------------

>  target/arm/t32.decode  |  5 ++-

>  2 files changed, 31 insertions(+), 61 deletions(-)


Reviewed-by: Peter Maydell <peter.maydell@linaro.org>


thanks
-- PMM
Peter Maydell Aug. 27, 2019, 11:09 a.m. UTC | #2 
On Mon, 19 Aug 2019 at 22:38, Richard Henderson
<richard.henderson@linaro.org> wrote:
>

> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

> ---

>  target/arm/translate.c | 87 +++++++++++++-----------------------------

>  target/arm/t32.decode  |  5 ++-

>  2 files changed, 31 insertions(+), 61 deletions(-)

>

> diff --git a/target/arm/translate.c b/target/arm/translate.c

> index 9a8864e8ff..d1078ca1ec 100644

> --- a/target/arm/translate.c

> +++ b/target/arm/translate.c

> @@ -8454,6 +8454,30 @@ static bool trans_SG(DisasContext *s, arg_SG *a)

>      return true;

>  }

>

> +static bool trans_TT(DisasContext *s, arg_TT *a)

> +{

> +    TCGv_i32 addr, tmp;

> +

> +    if (!arm_dc_feature(s, ARM_FEATURE_M) ||

> +        !arm_dc_feature(s, ARM_FEATURE_V8)) {

> +        return false;

> +    }

> +    if (a->rd == 13 || a->rd == 15 || a->rn == 15) {

> +        /* We UNDEF for these UNPREDICTABLE cases */

> +        return false;

> +    }

> +    if (a->A && !s->v8m_secure) {

> +        return false;

> +    }

> +

> +    addr = load_reg(s, a->rn);

> +    tmp = tcg_const_i32((a->A << 1) | a->T);

> +    gen_helper_v7m_tt(tmp, cpu_env, addr, tmp);

> +    tcg_temp_free_i32(addr);

> +    store_reg(s, a->rd, tmp);

> +    return true;

> +}


> diff --git a/target/arm/t32.decode b/target/arm/t32.decode

> index ce46650446..bb875f77b0 100644

> --- a/target/arm/t32.decode

> +++ b/target/arm/t32.decode

> @@ -506,7 +506,10 @@ STRD_ri_t32      1110 1001 .110 .... .... .... ........    @ldstd_ri8 w=1 p=1

>  @ldrex_d         .... .... .... rn:4 rt:4 rt2:4 .... .... \

>                   &ldrex imm=0

>

> -STREX            1110 1000 0100 .... .... .... .... ....      @strex_i

> +{

> +  TT             1110 1000 0100 rn:4 1111 rd:4 A:1 T:1 000000

> +  STREX          1110 1000 0100 .... .... .... .... ....      @strex_i

> +}


This patch turns out to have a bug -- there are cases in
trans_TT which are supposed to UNDEF. We return 'false' in
those cases, which means we fall through to trans_STREX(),
which doesn't have any checks in it to make them actually
UNDEF, so we end up generating code for some kind of STREX.
(Interestingly, I wrote the notes in the other email I
sent earlier about returning false vs unallocated_encoding+true
before I found and diagnosed the cause of this bug.)

thanks
-- PMM
diff mbox series

Patch

diff --git a/target/arm/translate.c b/target/arm/translate.c
index 9a8864e8ff..d1078ca1ec 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -8454,6 +8454,30 @@  static bool trans_SG(DisasContext *s, arg_SG *a)
     return true;
 }
 
+static bool trans_TT(DisasContext *s, arg_TT *a)
+{
+    TCGv_i32 addr, tmp;
+
+    if (!arm_dc_feature(s, ARM_FEATURE_M) ||
+        !arm_dc_feature(s, ARM_FEATURE_V8)) {
+        return false;
+    }
+    if (a->rd == 13 || a->rd == 15 || a->rn == 15) {
+        /* We UNDEF for these UNPREDICTABLE cases */
+        return false;
+    }
+    if (a->A && !s->v8m_secure) {
+        return false;
+    }
+
+    addr = load_reg(s, a->rn);
+    tmp = tcg_const_i32((a->A << 1) | a->T);
+    gen_helper_v7m_tt(tmp, cpu_env, addr, tmp);
+    tcg_temp_free_i32(addr);
+    store_reg(s, a->rd, tmp);
+    return true;
+}
+
 /*
  * Load/store register index
  */
@@ -10409,7 +10433,7 @@  static bool thumb_insn_is_16bit(DisasContext *s, uint32_t pc, uint32_t insn)
 /* Translate a 32-bit thumb instruction. */
 static void disas_thumb2_insn(DisasContext *s, uint32_t insn)
 {
-    uint32_t rd, rn, rs;
+    uint32_t rn;
     int op;
 
     /*
@@ -10453,70 +10477,13 @@  static void disas_thumb2_insn(DisasContext *s, uint32_t insn)
     /* fall back to legacy decoder */
 
     rn = (insn >> 16) & 0xf;
-    rs = (insn >> 12) & 0xf;
-    rd = (insn >> 8) & 0xf;
     switch ((insn >> 25) & 0xf) {
     case 0: case 1: case 2: case 3:
         /* 16-bit instructions.  Should never happen.  */
         abort();
     case 4:
-        if (insn & (1 << 22)) {
-            /* 0b1110_100x_x1xx_xxxx_xxxx_xxxx_xxxx_xxxx
-             * - load/store doubleword, load/store exclusive, ldacq/strel,
-             *   table branch, TT.
-             */
-            if (insn & 0x01200000) {
-                /* load/store dual, in decodetree */
-                goto illegal_op;
-            } else if ((insn & (1 << 23)) == 0) {
-                /* 0b1110_1000_010x_xxxx_xxxx_xxxx_xxxx_xxxx
-                 * - load/store exclusive word
-                 * - TT (v8M only)
-                 */
-                if (rs == 15) {
-                    if (!(insn & (1 << 20)) &&
-                        arm_dc_feature(s, ARM_FEATURE_M) &&
-                        arm_dc_feature(s, ARM_FEATURE_V8)) {
-                        /* 0b1110_1000_0100_xxxx_1111_xxxx_xxxx_xxxx
-                         *  - TT (v8M only)
-                         */
-                        bool alt = insn & (1 << 7);
-                        TCGv_i32 addr, op, ttresp;
-
-                        if ((insn & 0x3f) || rd == 13 || rd == 15 || rn == 15) {
-                            /* we UNDEF for these UNPREDICTABLE cases */
-                            goto illegal_op;
-                        }
-
-                        if (alt && !s->v8m_secure) {
-                            goto illegal_op;
-                        }
-
-                        addr = load_reg(s, rn);
-                        op = tcg_const_i32(extract32(insn, 6, 2));
-                        ttresp = tcg_temp_new_i32();
-                        gen_helper_v7m_tt(ttresp, cpu_env, addr, op);
-                        tcg_temp_free_i32(addr);
-                        tcg_temp_free_i32(op);
-                        store_reg(s, rd, ttresp);
-                        break;
-                    }
-                    goto illegal_op;
-                }
-                /* Load/store exclusive, in decodetree */
-                goto illegal_op;
-            } else if ((insn & (7 << 5)) == 0) {
-                /* Table Branch, in decodetree */
-                goto illegal_op;
-            } else {
-                /* Load/store exclusive, load-acq/store-rel, in decodetree */
-                goto illegal_op;
-            }
-        } else {
-            /* Load/store multiple, RFE, SRS, in decodetree */
-            goto illegal_op;
-        }
-        break;
+        /* All in decodetree */
+        goto illegal_op;
     case 5:
         /* All in decodetree */
         goto illegal_op;
diff --git a/target/arm/t32.decode b/target/arm/t32.decode
index ce46650446..bb875f77b0 100644
--- a/target/arm/t32.decode
+++ b/target/arm/t32.decode
@@ -506,7 +506,10 @@  STRD_ri_t32      1110 1001 .110 .... .... .... ........    @ldstd_ri8 w=1 p=1
 @ldrex_d         .... .... .... rn:4 rt:4 rt2:4 .... .... \
                  &ldrex imm=0
 
-STREX            1110 1000 0100 .... .... .... .... ....      @strex_i
+{
+  TT             1110 1000 0100 rn:4 1111 rd:4 A:1 T:1 000000
+  STREX          1110 1000 0100 .... .... .... .... ....      @strex_i
+}
 STREXB           1110 1000 1100 .... .... 1111 0100 ....      @strex_0
 STREXH           1110 1000 1100 .... .... 1111 0101 ....      @strex_0
 STREXD_t32       1110 1000 1100 .... .... .... 0111 ....      @strex_d