[Qemu-devel] [PATCH 0/6] target/rx: Improvements to disassembly

[Qemu-devel] [PATCH 0/6] target/rx: Improvements to disassembly

[Richard Henderson]

Here's a sample of the new output, taken from u-boot.bin:

IN:
0xfff8000a:  fb 12 00 01 00 00          mov.l   #0x00000100, r1
0xfff80010:  fb 32 f0 13 00 00          mov.l   #0x000013f0, r3
0xfff80016:  43 13                      sub     r1, r3
0xfff80018:  fb 22 00 ea f9 ff          mov.l   #-398848, r2
0xfff8001e:  7f 8f                      smovf
0xfff80020:  ef 01                      mov.l   r0, r1
0xfff80022:  05 1e 32 00                bsr.a   fff83240

IN:
0xfff83240:  72 11 5c fb                add     #-1188, r1
0xfff83244:  75 21 f0                   and     #-16, r1
0xfff83247:  02                         rts

Obviously there are still a few inconsistencies in the
format strings used for the immediates, but the format
is readable and it is easy to look at the opcode to see
how our decode compares to the manual.


r~


Richard Henderson (6):
  target/rx: Disassemble rx_index_addr into a string
  target/rx: Replace operand with prt_ldmi in disassembler
  target/rx: Use prt_ldmi for XCHG_mr disassembly
  target/rx: Emit all disassembly in one prt()
  target/rx: Collect all bytes during disassembly
  target/rx: Dump bytes for each insn during disassembly

 target/rx/disas.c | 366 +++++++++++++++++++++-------------------------
 1 file changed, 166 insertions(+), 200 deletions(-)

-- 
2.17.1

[Qemu-devel] [PATCH 1/6] target/rx: Disassemble rx_index_addr into a string

[Richard Henderson]

We were eliding all zero indexes.  It is only ld==0 that does
not have an index in the instruction.  This also allows us to
avoid breaking the final print into multiple pieces.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/rx/disas.c | 154 +++++++++++++++++-----------------------------
 1 file changed, 55 insertions(+), 99 deletions(-)

diff --git a/target/rx/disas.c b/target/rx/disas.c
index 8cada4825d..64342537ee 100644
--- a/target/rx/disas.c
+++ b/target/rx/disas.c
@@ -107,49 +107,42 @@ static const char psw[] = {
     'i', 'u', 0, 0, 0, 0, 0, 0,
 };
 
-static uint32_t rx_index_addr(int ld, int size, DisasContext *ctx)
+static void rx_index_addr(DisasContext *ctx, char out[8], int ld, int mi)
 {
-    bfd_byte buf[2];
+    uint32_t addr = ctx->addr;
+    uint8_t buf[2];
+    uint16_t dsp;
+
     switch (ld) {
     case 0:
-        return 0;
+        /* No index; return empty string.  */
+        out[0] = '\0';
+        return;
     case 1:
-        ctx->dis->read_memory_func(ctx->addr, buf, 1, ctx->dis);
         ctx->addr += 1;
-        return ((uint8_t)buf[0]) << size;
+        ctx->dis->read_memory_func(addr, buf, 1, ctx->dis);
+        dsp = buf[0];
+        break;
     case 2:
-        ctx->dis->read_memory_func(ctx->addr, buf, 2, ctx->dis);
         ctx->addr += 2;
-        return lduw_le_p(buf) << size;
+        ctx->dis->read_memory_func(addr, buf, 2, ctx->dis);
+        dsp = lduw_le_p(buf);
+        break;
+    default:
+        g_assert_not_reached();
     }
-    g_assert_not_reached();
+
+    sprintf(out, "%u", dsp << (mi < 3 ? mi : 4 - mi));
 }
 
 static void operand(DisasContext *ctx, int ld, int mi, int rs, int rd)
 {
-    int dsp;
     static const char sizes[][4] = {".b", ".w", ".l", ".uw", ".ub"};
+    char dsp[8];
+
     if (ld < 3) {
-        switch (mi) {
-        case 4:
-            /* dsp[rs].ub */
-            dsp = rx_index_addr(ld, RX_MEMORY_BYTE, ctx);
-            break;
-        case 3:
-            /* dsp[rs].uw */
-            dsp = rx_index_addr(ld, RX_MEMORY_WORD, ctx);
-            break;
-        default:
-            /* dsp[rs].b */
-            /* dsp[rs].w */
-            /* dsp[rs].l */
-            dsp = rx_index_addr(ld, mi, ctx);
-            break;
-        }
-        if (dsp > 0) {
-            prt("%d", dsp);
-        }
-        prt("[r%d]%s", rs, sizes[mi]);
+        rx_index_addr(ctx, dsp, ld, mi);
+        prt("%s[r%d]%s", dsp, rs, sizes[mi]);
     } else {
         prt("r%d", rs);
     }
@@ -235,7 +228,7 @@ static bool trans_MOV_ra(DisasContext *ctx, arg_MOV_ra *a)
 /* mov.[bwl] rs,rd */
 static bool trans_MOV_mm(DisasContext *ctx, arg_MOV_mm *a)
 {
-    int dsp;
+    char dspd[8], dsps[8];
 
     prt("mov.%c\t", size[a->sz]);
     if (a->lds == 3 && a->ldd == 3) {
@@ -244,29 +237,15 @@ static bool trans_MOV_mm(DisasContext *ctx, arg_MOV_mm *a)
         return true;
     }
     if (a->lds == 3) {
-        prt("r%d, ", a->rd);
-        dsp = rx_index_addr(a->ldd, a->sz, ctx);
-        if (dsp > 0) {
-            prt("%d", dsp);
-        }
-        prt("[r%d]", a->rs);
+        rx_index_addr(ctx, dspd, a->ldd, a->sz);
+        prt("r%d, %s[r%d]", a->rs, dspd, a->rd);
     } else if (a->ldd == 3) {
-        dsp = rx_index_addr(a->lds, a->sz, ctx);
-        if (dsp > 0) {
-            prt("%d", dsp);
-        }
-        prt("[r%d], r%d", a->rs, a->rd);
+        rx_index_addr(ctx, dsps, a->lds, a->sz);
+        prt("%s[r%d], r%d", dsps, a->rs, a->rd);
     } else {
-        dsp = rx_index_addr(a->lds, a->sz, ctx);
-        if (dsp > 0) {
-            prt("%d", dsp);
-        }
-        prt("[r%d], ", a->rs);
-        dsp = rx_index_addr(a->ldd, a->sz, ctx);
-        if (dsp > 0) {
-            prt("%d", dsp);
-        }
-        prt("[r%d]", a->rd);
+        rx_index_addr(ctx, dsps, a->lds, a->sz);
+        rx_index_addr(ctx, dspd, a->ldd, a->sz);
+        prt("%s[r%d], %s[r%d]", dsps, a->rs, dspd, a->rd);
     }
     return true;
 }
@@ -357,12 +336,10 @@ static bool trans_PUSH_r(DisasContext *ctx, arg_PUSH_r *a)
 /* push dsp[rs] */
 static bool trans_PUSH_m(DisasContext *ctx, arg_PUSH_m *a)
 {
-    prt("push\t");
-    int dsp = rx_index_addr(a->ld, a->sz, ctx);
-    if (dsp > 0) {
-        prt("%d", dsp);
-    }
-    prt("[r%d]", a->rs);
+    char dsp[8];
+
+    rx_index_addr(ctx, dsp, a->ld, a->sz);
+    prt("push\t%s[r%d]", dsp, a->rs);
     return true;
 }
 
@@ -389,17 +366,13 @@ static bool trans_XCHG_rr(DisasContext *ctx, arg_XCHG_rr *a)
 /* xchg dsp[rs].<mi>,rd */
 static bool trans_XCHG_mr(DisasContext *ctx, arg_XCHG_mr *a)
 {
-    int dsp;
     static const char msize[][4] = {
         "b", "w", "l", "ub", "uw",
     };
+    char dsp[8];
 
-    prt("xchg\t");
-    dsp = rx_index_addr(a->ld, a->mi, ctx);
-    if (dsp > 0) {
-        prt("%d", dsp);
-    }
-    prt("[r%d].%s, r%d", a->rs, msize[a->mi], a->rd);
+    rx_index_addr(ctx, dsp, a->ld, a->mi);
+    prt("xchg\t%s[r%d].%s, r%d", dsp, a->rs, msize[a->mi], a->rd);
     return true;
 }
 
@@ -552,13 +525,10 @@ static bool trans_ADC_rr(DisasContext *ctx, arg_ADC_rr *a)
 /* adc dsp[rs], rd */
 static bool trans_ADC_mr(DisasContext *ctx, arg_ADC_mr *a)
 {
-    int dsp;
-    prt("adc\t");
-    dsp = rx_index_addr(a->ld, 2, ctx);
-    if (dsp > 0) {
-        prt("%d", dsp);
-    }
-    prt("[r%d], r%d", a->rs, a->rd);
+    char dsp[8];
+
+    rx_index_addr(ctx, dsp, a->ld, 2);
+    prt("adc\t%s[r%d], r%d", dsp, a->rs, a->rd);
     return true;
 }
 
@@ -1217,25 +1187,17 @@ static bool trans_ITOF(DisasContext *ctx, arg_ITOF *a)
 
 #define BOP_IM(name, reg)                                       \
     do {                                                        \
-        int dsp;                                                \
-        prt("b%s\t#%d, ", #name, a->imm);                       \
-        dsp = rx_index_addr(a->ld, RX_MEMORY_BYTE, ctx);        \
-        if (dsp > 0) {                                          \
-            prt("%d", dsp);                                     \
-        }                                                       \
-        prt("[r%d]", reg);                                      \
+        char dsp[8];                                            \
+        rx_index_addr(ctx, dsp, a->ld, RX_MEMORY_BYTE);         \
+        prt("b%s\t#%d, %s[r%d]", #name, a->imm, dsp, reg);      \
         return true;                                            \
     } while (0)
 
 #define BOP_RM(name)                                            \
     do {                                                        \
-        int dsp;                                                \
-        prt("b%s\tr%d, ", #name, a->rd);                        \
-        dsp = rx_index_addr(a->ld, RX_MEMORY_BYTE, ctx);        \
-        if (dsp > 0) {                                          \
-            prt("%d", dsp);                                     \
-        }                                                       \
-        prt("[r%d]", a->rs);                                    \
+        char dsp[8];                                            \
+        rx_index_addr(ctx, dsp, a->ld, RX_MEMORY_BYTE);         \
+        prt("b%s\tr%d, %s[r%d]", #name, a->rd, dsp, a->rs);     \
         return true;                                            \
     } while (0)
 
@@ -1346,12 +1308,10 @@ static bool trans_BNOT_ir(DisasContext *ctx, arg_BNOT_ir *a)
 /* bmcond #imm, dsp[rd] */
 static bool trans_BMCnd_im(DisasContext *ctx, arg_BMCnd_im *a)
 {
-    int dsp = rx_index_addr(a->ld, RX_MEMORY_BYTE, ctx);
-    prt("bm%s\t#%d, ", cond[a->cd], a->imm);
-    if (dsp > 0) {
-        prt("%d", dsp);
-    }
-    prt("[%d]", a->rd);
+    char dsp[8];
+
+    rx_index_addr(ctx, dsp, a->ld, RX_MEMORY_BYTE);
+    prt("bm%s\t#%d, %s[r%d]", cond[a->cd], a->imm, dsp, a->rd);
     return true;
 }
 
@@ -1443,16 +1403,12 @@ static bool trans_WAIT(DisasContext *ctx, arg_WAIT *a)
 /* sccnd.[bwl] dsp:[rd] */
 static bool trans_SCCnd(DisasContext *ctx, arg_SCCnd *a)
 {
-    int dsp;
-    prt("sc%s.%c\t", cond[a->cd], size[a->sz]);
     if (a->ld < 3) {
-        dsp = rx_index_addr(a->sz, a->ld, ctx);
-        if (dsp > 0) {
-            prt("%d", dsp);
-        }
-        prt("[r%d]", a->rd);
+        char dsp[8];
+        rx_index_addr(ctx, dsp, a->sz, a->ld);
+        prt("sc%s.%c\t%s[r%d]", cond[a->cd], size[a->sz], dsp, a->rd);
     } else {
-        prt("r%d", a->rd);
+        prt("sc%s.%c\tr%d", cond[a->cd], size[a->sz], a->rd);
     }
     return true;
 }
-- 
2.17.1

[Qemu-devel] [PATCH 2/6] target/rx: Replace operand with prt_ldmi in disassembler

[Richard Henderson]

This has consistency with prt_ri().  It loads all data before
beginning output.  It uses exactly one call to prt() to emit
the full instruction.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/rx/disas.c | 77 +++++++++++++++++------------------------------
 1 file changed, 27 insertions(+), 50 deletions(-)

diff --git a/target/rx/disas.c b/target/rx/disas.c
index 64342537ee..515b365528 100644
--- a/target/rx/disas.c
+++ b/target/rx/disas.c
@@ -135,18 +135,18 @@ static void rx_index_addr(DisasContext *ctx, char out[8], int ld, int mi)
     sprintf(out, "%u", dsp << (mi < 3 ? mi : 4 - mi));
 }
 
-static void operand(DisasContext *ctx, int ld, int mi, int rs, int rd)
+static void prt_ldmi(DisasContext *ctx, const char *insn,
+                     int ld, int mi, int rs, int rd)
 {
     static const char sizes[][4] = {".b", ".w", ".l", ".uw", ".ub"};
     char dsp[8];
 
     if (ld < 3) {
         rx_index_addr(ctx, dsp, ld, mi);
-        prt("%s[r%d]%s", dsp, rs, sizes[mi]);
+        prt("%s\t%s[r%d]%s, r%d", insn, dsp, rs, sizes[mi], rd);
     } else {
-        prt("r%d", rs);
+        prt("%s\tr%d, r%d", insn, rs, rd);
     }
-    prt(", r%d", rd);
 }
 
 static void prt_ir(DisasContext *ctx, const char *insn, int imm, int rd)
@@ -416,8 +416,7 @@ static bool trans_AND_ir(DisasContext *ctx, arg_AND_ir *a)
 /* and rs,rd */
 static bool trans_AND_mr(DisasContext *ctx, arg_AND_mr *a)
 {
-    prt("and\t");
-    operand(ctx, a->ld, a->mi, a->rs, a->rd);
+    prt_ldmi(ctx, "and", a->ld, a->mi, a->rs, a->rd);
     return true;
 }
 
@@ -440,8 +439,7 @@ static bool trans_OR_ir(DisasContext *ctx, arg_OR_ir *a)
 /* or rs,rd */
 static bool trans_OR_mr(DisasContext *ctx, arg_OR_mr *a)
 {
-    prt("or\t");
-    operand(ctx, a->ld, a->mi, a->rs, a->rd);
+    prt_ldmi(ctx, "or", a->ld, a->mi, a->rs, a->rd);
     return true;
 }
 
@@ -463,8 +461,7 @@ static bool trans_XOR_ir(DisasContext *ctx, arg_XOR_ir *a)
 /* xor rs,rd */
 static bool trans_XOR_mr(DisasContext *ctx, arg_XOR_mr *a)
 {
-    prt("xor\t");
-    operand(ctx, a->ld, a->mi, a->rs, a->rd);
+    prt_ldmi(ctx, "xor", a->ld, a->mi, a->rs, a->rd);
     return true;
 }
 
@@ -479,8 +476,7 @@ static bool trans_TST_ir(DisasContext *ctx, arg_TST_ir *a)
 /* tst rs, rd */
 static bool trans_TST_mr(DisasContext *ctx, arg_TST_mr *a)
 {
-    prt("tst\t");
-    operand(ctx, a->ld, a->mi, a->rs, a->rd);
+    prt_ldmi(ctx, "tst", a->ld, a->mi, a->rs, a->rd);
     return true;
 }
 
@@ -548,8 +544,7 @@ static bool trans_ADD_irr(DisasContext *ctx, arg_ADD_irr *a)
 /* add dsp[rs], rd */
 static bool trans_ADD_mr(DisasContext *ctx, arg_ADD_mr *a)
 {
-    prt("add\t");
-    operand(ctx, a->ld, a->mi, a->rs, a->rd);
+    prt_ldmi(ctx, "add", a->ld, a->mi, a->rs, a->rd);
     return true;
 }
 
@@ -573,8 +568,7 @@ static bool trans_CMP_ir(DisasContext *ctx, arg_CMP_ir *a)
 /* cmp dsp[rs], rs2 */
 static bool trans_CMP_mr(DisasContext *ctx, arg_CMP_mr *a)
 {
-    prt("cmp\t");
-    operand(ctx, a->ld, a->mi, a->rs, a->rd);
+    prt_ldmi(ctx, "cmp", a->ld, a->mi, a->rs, a->rd);
     return true;
 }
 
@@ -589,8 +583,7 @@ static bool trans_SUB_ir(DisasContext *ctx, arg_SUB_ir *a)
 /* sub dsp[rs], rd */
 static bool trans_SUB_mr(DisasContext *ctx, arg_SUB_mr *a)
 {
-    prt("sub\t");
-    operand(ctx, a->ld, a->mi, a->rs, a->rd);
+    prt_ldmi(ctx, "sub", a->ld, a->mi, a->rs, a->rd);
     return true;
 }
 
@@ -611,8 +604,7 @@ static bool trans_SBB_rr(DisasContext *ctx, arg_SBB_rr *a)
 /* sbb dsp[rs], rd */
 static bool trans_SBB_mr(DisasContext *ctx, arg_SBB_mr *a)
 {
-    prt("sbb\t");
-    operand(ctx, a->ld, RX_IM_LONG, a->rs, a->rd);
+    prt_ldmi(ctx, "sbb", a->ld, RX_IM_LONG, a->rs, a->rd);
     return true;
 }
 
@@ -640,8 +632,7 @@ static bool trans_MAX_ir(DisasContext *ctx, arg_MAX_ir *a)
 /* max dsp[rs], rd */
 static bool trans_MAX_mr(DisasContext *ctx, arg_MAX_mr *a)
 {
-    prt("max\t");
-    operand(ctx, a->ld, a->mi, a->rs, a->rd);
+    prt_ldmi(ctx, "max", a->ld, a->mi, a->rs, a->rd);
     return true;
 }
 
@@ -656,8 +647,7 @@ static bool trans_MIN_ir(DisasContext *ctx, arg_MIN_ir *a)
 /* min dsp[rs], rd */
 static bool trans_MIN_mr(DisasContext *ctx, arg_MIN_mr *a)
 {
-    prt("max\t");
-    operand(ctx, a->ld, a->mi, a->rs, a->rd);
+    prt_ldmi(ctx, "min", a->ld, a->mi, a->rs, a->rd);
     return true;
 }
 
@@ -673,8 +663,7 @@ static bool trans_MUL_ir(DisasContext *ctx, arg_MUL_ir *a)
 /* mul dsp[rs], rd */
 static bool trans_MUL_mr(DisasContext *ctx, arg_MUL_mr *a)
 {
-    prt("mul\t");
-    operand(ctx, a->ld, a->mi, a->rs, a->rd);
+    prt_ldmi(ctx, "mul", a->ld, a->mi, a->rs, a->rd);
     return true;
 }
 
@@ -696,8 +685,7 @@ static bool trans_EMUL_ir(DisasContext *ctx, arg_EMUL_ir *a)
 /* emul dsp[rs], rd */
 static bool trans_EMUL_mr(DisasContext *ctx, arg_EMUL_mr *a)
 {
-    prt("emul\t");
-    operand(ctx, a->ld, a->mi, a->rs, a->rd);
+    prt_ldmi(ctx, "emul", a->ld, a->mi, a->rs, a->rd);
     return true;
 }
 
@@ -712,8 +700,7 @@ static bool trans_EMULU_ir(DisasContext *ctx, arg_EMULU_ir *a)
 /* emulu dsp[rs], rd */
 static bool trans_EMULU_mr(DisasContext *ctx, arg_EMULU_mr *a)
 {
-    prt("emulu\t");
-    operand(ctx, a->ld, a->mi, a->rs, a->rd);
+    prt_ldmi(ctx, "emulu", a->ld, a->mi, a->rs, a->rd);
     return true;
 }
 
@@ -728,8 +715,7 @@ static bool trans_DIV_ir(DisasContext *ctx, arg_DIV_ir *a)
 /* div dsp[rs], rd */
 static bool trans_DIV_mr(DisasContext *ctx, arg_DIV_mr *a)
 {
-    prt("div\t");
-    operand(ctx, a->ld, a->mi, a->rs, a->rd);
+    prt_ldmi(ctx, "div", a->ld, a->mi, a->rs, a->rd);
     return true;
 }
 
@@ -744,8 +730,7 @@ static bool trans_DIVU_ir(DisasContext *ctx, arg_DIVU_ir *a)
 /* divu dsp[rs], rd */
 static bool trans_DIVU_mr(DisasContext *ctx, arg_DIVU_mr *a)
 {
-    prt("divu\t");
-    operand(ctx, a->ld, a->mi, a->rs, a->rd);
+    prt_ldmi(ctx, "divu", a->ld, a->mi, a->rs, a->rd);
     return true;
 }
 
@@ -1089,8 +1074,7 @@ static bool trans_FADD_ir(DisasContext *ctx, arg_FADD_ir *a)
 /* fadd rs, rd */
 static bool trans_FADD_mr(DisasContext *ctx, arg_FADD_mr *a)
 {
-    prt("fadd\t");
-    operand(ctx, a->ld, RX_IM_LONG, a->rs, a->rd);
+    prt_ldmi(ctx, "fadd", a->ld, RX_IM_LONG, a->rs, a->rd);
     return true;
 }
 
@@ -1105,8 +1089,7 @@ static bool trans_FCMP_ir(DisasContext *ctx, arg_FCMP_ir *a)
 /* fcmp rs, rd */
 static bool trans_FCMP_mr(DisasContext *ctx, arg_FCMP_mr *a)
 {
-    prt("fcmp\t");
-    operand(ctx, a->ld, RX_IM_LONG, a->rs, a->rd);
+    prt_ldmi(ctx, "fcmp", a->ld, RX_IM_LONG, a->rs, a->rd);
     return true;
 }
 
@@ -1121,8 +1104,7 @@ static bool trans_FSUB_ir(DisasContext *ctx, arg_FSUB_ir *a)
 /* fsub rs, rd */
 static bool trans_FSUB_mr(DisasContext *ctx, arg_FSUB_mr *a)
 {
-    prt("fsub\t");
-    operand(ctx, a->ld, RX_IM_LONG, a->rs, a->rd);
+    prt_ldmi(ctx, "fsub", a->ld, RX_IM_LONG, a->rs, a->rd);
     return true;
 }
 
@@ -1130,8 +1112,7 @@ static bool trans_FSUB_mr(DisasContext *ctx, arg_FSUB_mr *a)
 /* ftoi rs, rd */
 static bool trans_FTOI(DisasContext *ctx, arg_FTOI *a)
 {
-    prt("ftoi\t");
-    operand(ctx, a->ld, RX_IM_LONG, a->rs, a->rd);
+    prt_ldmi(ctx, "ftoi", a->ld, RX_IM_LONG, a->rs, a->rd);
     return true;
 }
 
@@ -1146,8 +1127,7 @@ static bool trans_FMUL_ir(DisasContext *ctx, arg_FMUL_ir *a)
 /* fmul rs, rd */
 static bool trans_FMUL_mr(DisasContext *ctx, arg_FMUL_mr *a)
 {
-    prt("fmul\t");
-    operand(ctx, a->ld, RX_IM_LONG, a->rs, a->rd);
+    prt_ldmi(ctx, "fmul", a->ld, RX_IM_LONG, a->rs, a->rd);
     return true;
 }
 
@@ -1162,8 +1142,7 @@ static bool trans_FDIV_ir(DisasContext *ctx, arg_FDIV_ir *a)
 /* fdiv rs, rd */
 static bool trans_FDIV_mr(DisasContext *ctx, arg_FDIV_mr *a)
 {
-    prt("fdiv\t");
-    operand(ctx, a->ld, RX_IM_LONG, a->rs, a->rd);
+    prt_ldmi(ctx, "fdiv", a->ld, RX_IM_LONG, a->rs, a->rd);
     return true;
 }
 
@@ -1171,8 +1150,7 @@ static bool trans_FDIV_mr(DisasContext *ctx, arg_FDIV_mr *a)
 /* round rs, rd */
 static bool trans_ROUND(DisasContext *ctx, arg_ROUND *a)
 {
-    prt("round\t");
-    operand(ctx, a->ld, RX_IM_LONG, a->rs, a->rd);
+    prt_ldmi(ctx, "round", a->ld, RX_IM_LONG, a->rs, a->rd);
     return true;
 }
 
@@ -1180,8 +1158,7 @@ static bool trans_ROUND(DisasContext *ctx, arg_ROUND *a)
 /* itof dsp[rs], rd */
 static bool trans_ITOF(DisasContext *ctx, arg_ITOF *a)
 {
-    prt("itof\t");
-    operand(ctx, a->ld, RX_IM_LONG, a->rs, a->rd);
+    prt_ldmi(ctx, "itof", a->ld, RX_IM_LONG, a->rs, a->rd);
     return true;
 }
 
-- 
2.17.1

[Qemu-devel] [PATCH 3/6] target/rx: Use prt_ldmi for XCHG_mr disassembly

[Richard Henderson]

Note that the ld == 3 case handled by prt_ldmi is decoded as
XCHG_rr and cannot appear here.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/rx/disas.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/target/rx/disas.c b/target/rx/disas.c
index 515b365528..db10385fd0 100644
--- a/target/rx/disas.c
+++ b/target/rx/disas.c
@@ -366,13 +366,7 @@ static bool trans_XCHG_rr(DisasContext *ctx, arg_XCHG_rr *a)
 /* xchg dsp[rs].<mi>,rd */
 static bool trans_XCHG_mr(DisasContext *ctx, arg_XCHG_mr *a)
 {
-    static const char msize[][4] = {
-        "b", "w", "l", "ub", "uw",
-    };
-    char dsp[8];
-
-    rx_index_addr(ctx, dsp, a->ld, a->mi);
-    prt("xchg\t%s[r%d].%s, r%d", dsp, a->rs, msize[a->mi], a->rd);
+    prt_ldmi(ctx, "xchg", a->ld, a->mi, a->rs, a->rd);
     return true;
 }
 
-- 
2.17.1

[Qemu-devel] [PATCH 4/6] target/rx: Emit all disassembly in one prt()

[Richard Henderson]

Many of the multi-part prints have been eliminated by previous
patches.  Eliminate the rest of them.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/rx/disas.c | 75 ++++++++++++++++++++++++-----------------------
 1 file changed, 39 insertions(+), 36 deletions(-)

diff --git a/target/rx/disas.c b/target/rx/disas.c
index db10385fd0..ebc1a44249 100644
--- a/target/rx/disas.c
+++ b/target/rx/disas.c
@@ -228,24 +228,21 @@ static bool trans_MOV_ra(DisasContext *ctx, arg_MOV_ra *a)
 /* mov.[bwl] rs,rd */
 static bool trans_MOV_mm(DisasContext *ctx, arg_MOV_mm *a)
 {
-    char dspd[8], dsps[8];
+    char dspd[8], dsps[8], szc = size[a->sz];
 
-    prt("mov.%c\t", size[a->sz]);
     if (a->lds == 3 && a->ldd == 3) {
         /* mov.[bwl] rs,rd */
-        prt("r%d, r%d", a->rs, a->rd);
-        return true;
-    }
-    if (a->lds == 3) {
+        prt("mov.%c\tr%d, r%d", szc, a->rs, a->rd);
+    } else if (a->lds == 3) {
         rx_index_addr(ctx, dspd, a->ldd, a->sz);
-        prt("r%d, %s[r%d]", a->rs, dspd, a->rd);
+        prt("mov.%c\tr%d, %s[r%d]", szc, a->rs, dspd, a->rd);
     } else if (a->ldd == 3) {
         rx_index_addr(ctx, dsps, a->lds, a->sz);
-        prt("%s[r%d], r%d", dsps, a->rs, a->rd);
+        prt("mov.%c\t%s[r%d], r%d", szc, dsps, a->rs, a->rd);
     } else {
         rx_index_addr(ctx, dsps, a->lds, a->sz);
         rx_index_addr(ctx, dspd, a->ldd, a->sz);
-        prt("%s[r%d], %s[r%d]", dsps, a->rs, dspd, a->rd);
+        prt("mov.%c\t%s[r%d], %s[r%d]", szc, dsps, a->rs, dspd, a->rd);
     }
     return true;
 }
@@ -254,8 +251,11 @@ static bool trans_MOV_mm(DisasContext *ctx, arg_MOV_mm *a)
 /* mov.[bwl] rs,[-rd] */
 static bool trans_MOV_rp(DisasContext *ctx, arg_MOV_rp *a)
 {
-    prt("mov.%c\tr%d, ", size[a->sz], a->rs);
-    prt((a->ad == 0) ? "[r%d+]" : "[-r%d]", a->rd);
+    if (a->ad) {
+        prt("mov.%c\tr%d, [-r%d]", size[a->sz], a->rs, a->rd);
+    } else {
+        prt("mov.%c\tr%d, [r%d+]", size[a->sz], a->rs, a->rd);
+    }
     return true;
 }
 
@@ -263,9 +263,11 @@ static bool trans_MOV_rp(DisasContext *ctx, arg_MOV_rp *a)
 /* mov.[bwl] [-rd],rs */
 static bool trans_MOV_pr(DisasContext *ctx, arg_MOV_pr *a)
 {
-    prt("mov.%c\t", size[a->sz]);
-    prt((a->ad == 0) ? "[r%d+]" : "[-r%d]", a->rd);
-    prt(", r%d", a->rs);
+    if (a->ad) {
+        prt("mov.%c\t[-r%d], r%d", size[a->sz], a->rd, a->rs);
+    } else {
+        prt("mov.%c\t[r%d+], r%d", size[a->sz], a->rd, a->rs);
+    }
     return true;
 }
 
@@ -299,9 +301,11 @@ static bool trans_MOVU_ar(DisasContext *ctx, arg_MOVU_ar *a)
 /* movu.[bw] [-rs],rd */
 static bool trans_MOVU_pr(DisasContext *ctx, arg_MOVU_pr *a)
 {
-    prt("movu.%c\t", size[a->sz]);
-    prt((a->ad == 0) ? "[r%d+]" : "[-r%d]", a->rd);
-    prt(", r%d", a->rs);
+    if (a->ad) {
+        prt("movu.%c\t[-r%d], r%d", size[a->sz], a->rd, a->rs);
+    } else {
+        prt("movu.%c\t[r%d+], r%d", size[a->sz], a->rd, a->rs);
+    }
     return true;
 }
 
@@ -478,11 +482,11 @@ static bool trans_TST_mr(DisasContext *ctx, arg_TST_mr *a)
 /* not rs, rd */
 static bool trans_NOT_rr(DisasContext *ctx, arg_NOT_rr *a)
 {
-    prt("not\t");
     if (a->rs != a->rd) {
-        prt("r%d, ", a->rs);
+        prt("not\tr%d, r%d", a->rs, a->rd);
+    } else {
+        prt("not\tr%d", a->rs);
     }
-    prt("r%d", a->rd);
     return true;
 }
 
@@ -490,11 +494,11 @@ static bool trans_NOT_rr(DisasContext *ctx, arg_NOT_rr *a)
 /* neg rs, rd */
 static bool trans_NEG_rr(DisasContext *ctx, arg_NEG_rr *a)
 {
-    prt("neg\t");
     if (a->rs != a->rd) {
-        prt("r%d, ", a->rs);
+        prt("neg\tr%d, r%d", a->rs, a->rd);
+    } else {
+        prt("neg\tr%d", a->rs);
     }
-    prt("r%d", a->rd);
     return true;
 }
 
@@ -606,11 +610,10 @@ static bool trans_SBB_mr(DisasContext *ctx, arg_SBB_mr *a)
 /* abs rs, rd */
 static bool trans_ABS_rr(DisasContext *ctx, arg_ABS_rr *a)
 {
-    prt("abs\t");
-    if (a->rs == a->rd) {
-        prt("r%d", a->rd);
+    if (a->rs != a->rd) {
+        prt("abs\tr%d, r%d", a->rs, a->rd);
     } else {
-        prt("r%d, r%d", a->rs, a->rd);
+        prt("abs\tr%d", a->rs);
     }
     return true;
 }
@@ -733,11 +736,11 @@ static bool trans_DIVU_mr(DisasContext *ctx, arg_DIVU_mr *a)
 /* shll #imm:5, rs, rd */
 static bool trans_SHLL_irr(DisasContext *ctx, arg_SHLL_irr *a)
 {
-    prt("shll\t#%d, ", a->imm);
     if (a->rs2 != a->rd) {
-        prt("r%d, ", a->rs2);
+        prt("shll\t#%d, r%d, r%d", a->imm, a->rs2, a->rd);
+    } else {
+        prt("shll\t#%d, r%d", a->imm, a->rd);
     }
-    prt("r%d", a->rd);
     return true;
 }
 
@@ -752,11 +755,11 @@ static bool trans_SHLL_rr(DisasContext *ctx, arg_SHLL_rr *a)
 /* shar #imm:5, rs, rd */
 static bool trans_SHAR_irr(DisasContext *ctx, arg_SHAR_irr *a)
 {
-    prt("shar\t#%d,", a->imm);
     if (a->rs2 != a->rd) {
-        prt("r%d, ", a->rs2);
+        prt("shar\t#%d, r%d, r%d", a->imm, a->rs2, a->rd);
+    } else {
+        prt("shar\t#%d, r%d", a->imm, a->rd);
     }
-    prt("r%d", a->rd);
     return true;
 }
 
@@ -771,11 +774,11 @@ static bool trans_SHAR_rr(DisasContext *ctx, arg_SHAR_rr *a)
 /* shlr #imm:5, rs, rd */
 static bool trans_SHLR_irr(DisasContext *ctx, arg_SHLR_irr *a)
 {
-    prt("shlr\t#%d, ", a->imm);
     if (a->rs2 != a->rd) {
-        prt("r%d, ", a->rs2);
+        prt("shlr\t#%d, r%d, r%d", a->imm, a->rs2, a->rd);
+    } else {
+        prt("shlr\t#%d, r%d", a->imm, a->rd);
     }
-    prt("r%d", a->rd);
     return true;
 }
 
-- 
2.17.1

[Qemu-devel] [PATCH 5/6] target/rx: Collect all bytes during disassembly

[Richard Henderson]

Collected, to be used in the next patch.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/rx/disas.c | 62 ++++++++++++++++++++++++++++++++---------------
 1 file changed, 42 insertions(+), 20 deletions(-)

diff --git a/target/rx/disas.c b/target/rx/disas.c
index ebc1a44249..5a32a87534 100644
--- a/target/rx/disas.c
+++ b/target/rx/disas.c
@@ -25,43 +25,59 @@ typedef struct DisasContext {
     disassemble_info *dis;
     uint32_t addr;
     uint32_t pc;
+    uint8_t len;
+    uint8_t bytes[8];
 } DisasContext;
 
 
 static uint32_t decode_load_bytes(DisasContext *ctx, uint32_t insn,
-                           int i, int n)
+                                  int i, int n)
 {
-    bfd_byte buf;
+    uint32_t addr = ctx->addr;
+
+    g_assert(ctx->len == i);
+    g_assert(n <= ARRAY_SIZE(ctx->bytes));
+
     while (++i <= n) {
-        ctx->dis->read_memory_func(ctx->addr++, &buf, 1, ctx->dis);
-        insn |= buf << (32 - i * 8);
+        ctx->dis->read_memory_func(addr++, &ctx->bytes[i - 1], 1, ctx->dis);
+        insn |= ctx->bytes[i - 1] << (32 - i * 8);
     }
+    ctx->addr = addr;
+    ctx->len = n;
+
     return insn;
 }
 
 static int32_t li(DisasContext *ctx, int sz)
 {
-    int32_t addr;
-    bfd_byte buf[4];
-    addr = ctx->addr;
+    uint32_t addr = ctx->addr;
+    uintptr_t len = ctx->len;
 
     switch (sz) {
     case 1:
+        g_assert(len + 1 <= ARRAY_SIZE(ctx->bytes));
         ctx->addr += 1;
-        ctx->dis->read_memory_func(addr, buf, 1, ctx->dis);
-        return (int8_t)buf[0];
+        ctx->len += 1;
+        ctx->dis->read_memory_func(addr, ctx->bytes + len, 1, ctx->dis);
+        return (int8_t)ctx->bytes[len];
     case 2:
+        g_assert(len + 2 <= ARRAY_SIZE(ctx->bytes));
         ctx->addr += 2;
-        ctx->dis->read_memory_func(addr, buf, 2, ctx->dis);
-        return ldsw_le_p(buf);
+        ctx->len += 2;
+        ctx->dis->read_memory_func(addr, ctx->bytes + len, 2, ctx->dis);
+        return ldsw_le_p(ctx->bytes + len);
     case 3:
+        g_assert(len + 3 <= ARRAY_SIZE(ctx->bytes));
         ctx->addr += 3;
-        ctx->dis->read_memory_func(addr, buf, 3, ctx->dis);
-        return (int8_t)buf[2] << 16 | lduw_le_p(buf);
+        ctx->len += 3;
+        ctx->dis->read_memory_func(addr, ctx->bytes + len, 3, ctx->dis);
+        return (int8_t)ctx->bytes[len + 2] << 16 | lduw_le_p(ctx->bytes + len);
     case 0:
+        g_assert(len + 4 <= ARRAY_SIZE(ctx->bytes));
         ctx->addr += 4;
-        ctx->dis->read_memory_func(addr, buf, 4, ctx->dis);
-        return ldl_le_p(buf);
+        ctx->len += 4;
+        ctx->dis->read_memory_func(addr, ctx->bytes + len, 4, ctx->dis);
+        return ldl_le_p(ctx->bytes + len);
     default:
         g_assert_not_reached();
     }
@@ -110,7 +126,7 @@ static const char psw[] = {
 static void rx_index_addr(DisasContext *ctx, char out[8], int ld, int mi)
 {
     uint32_t addr = ctx->addr;
-    uint8_t buf[2];
+    uintptr_t len = ctx->len;
     uint16_t dsp;
 
     switch (ld) {
@@ -119,14 +135,18 @@ static void rx_index_addr(DisasContext *ctx, char out[8], int ld, int mi)
         out[0] = '\0';
         return;
     case 1:
+        g_assert(len + 1 <= ARRAY_SIZE(ctx->bytes));
         ctx->addr += 1;
-        ctx->dis->read_memory_func(addr, buf, 1, ctx->dis);
-        dsp = buf[0];
+        ctx->len += 1;
+        ctx->dis->read_memory_func(addr, ctx->bytes + len, 1, ctx->dis);
+        dsp = ctx->bytes[len];
         break;
     case 2:
+        g_assert(len + 2 <= ARRAY_SIZE(ctx->bytes));
         ctx->addr += 2;
-        ctx->dis->read_memory_func(addr, buf, 2, ctx->dis);
-        dsp = lduw_le_p(buf);
+        ctx->len += 2;
+        ctx->dis->read_memory_func(addr, ctx->bytes + len, 2, ctx->dis);
+        dsp = lduw_le_p(ctx->bytes + len);
         break;
     default:
         g_assert_not_reached();
@@ -1392,8 +1412,10 @@ int print_insn_rx(bfd_vma addr, disassemble_info *dis)
     DisasContext ctx;
     uint32_t insn;
     int i;
+
     ctx.dis = dis;
     ctx.pc = ctx.addr = addr;
+    ctx.len = 0;
 
     insn = decode_load(&ctx);
     if (!decode(&ctx, insn)) {
-- 
2.17.1

[Qemu-devel] [PATCH 6/6] target/rx: Dump bytes for each insn during disassembly

[Richard Henderson]

There are so many different forms of each RX instruction
that it will be very useful to be able to look at the bytes
to see on which path a bug may lie.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/rx/disas.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/target/rx/disas.c b/target/rx/disas.c
index 5a32a87534..d73b53db44 100644
--- a/target/rx/disas.c
+++ b/target/rx/disas.c
@@ -102,7 +102,21 @@ static int bdsp_s(DisasContext *ctx, int d)
 /* Include the auto-generated decoder.  */
 #include "decode.inc.c"
 
-#define prt(...) (ctx->dis->fprintf_func)((ctx->dis->stream), __VA_ARGS__)
+static void dump_bytes(DisasContext *ctx)
+{
+    int i, len = ctx->len;
+
+    for (i = 0; i < len; ++i) {
+        ctx->dis->fprintf_func(ctx->dis->stream, "%02x ", ctx->bytes[i]);
+    }
+    ctx->dis->fprintf_func(ctx->dis->stream, "%*c", (8 - i) * 3, '\t');
+}
+
+#define prt(...) \
+    do {                                                        \
+        dump_bytes(ctx);                                        \
+        ctx->dis->fprintf_func(ctx->dis->stream, __VA_ARGS__);  \
+    } while (0)
 
 #define RX_MEMORY_BYTE 0
 #define RX_MEMORY_WORD 1
-- 
2.17.1

Re: [Qemu-devel] [PATCH 1/6] target/rx: Disassemble rx_index_addr into a string

[Yoshinori Sato]

On Fri, 24 May 2019 00:07:58 +0900,
Richard Henderson wrote:
> 
> We were eliding all zero indexes.  It is only ld==0 that does
> not have an index in the instruction.  This also allows us to
> avoid breaking the final print into multiple pieces.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Yoshinori Sato <ysato@users.sourceforge.jp>

> ---
>  target/rx/disas.c | 154 +++++++++++++++++-----------------------------
>  1 file changed, 55 insertions(+), 99 deletions(-)
> 
> diff --git a/target/rx/disas.c b/target/rx/disas.c
> index 8cada4825d..64342537ee 100644
> --- a/target/rx/disas.c
> +++ b/target/rx/disas.c
> @@ -107,49 +107,42 @@ static const char psw[] = {
>      'i', 'u', 0, 0, 0, 0, 0, 0,
>  };
>  
> -static uint32_t rx_index_addr(int ld, int size, DisasContext *ctx)
> +static void rx_index_addr(DisasContext *ctx, char out[8], int ld, int mi)
>  {
> -    bfd_byte buf[2];
> +    uint32_t addr = ctx->addr;
> +    uint8_t buf[2];
> +    uint16_t dsp;
> +
>      switch (ld) {
>      case 0:
> -        return 0;
> +        /* No index; return empty string.  */
> +        out[0] = '\0';
> +        return;
>      case 1:
> -        ctx->dis->read_memory_func(ctx->addr, buf, 1, ctx->dis);
>          ctx->addr += 1;
> -        return ((uint8_t)buf[0]) << size;
> +        ctx->dis->read_memory_func(addr, buf, 1, ctx->dis);
> +        dsp = buf[0];
> +        break;
>      case 2:
> -        ctx->dis->read_memory_func(ctx->addr, buf, 2, ctx->dis);
>          ctx->addr += 2;
> -        return lduw_le_p(buf) << size;
> +        ctx->dis->read_memory_func(addr, buf, 2, ctx->dis);
> +        dsp = lduw_le_p(buf);
> +        break;
> +    default:
> +        g_assert_not_reached();
>      }
> -    g_assert_not_reached();
> +
> +    sprintf(out, "%u", dsp << (mi < 3 ? mi : 4 - mi));
>  }
>  
>  static void operand(DisasContext *ctx, int ld, int mi, int rs, int rd)
>  {
> -    int dsp;
>      static const char sizes[][4] = {".b", ".w", ".l", ".uw", ".ub"};
> +    char dsp[8];
> +
>      if (ld < 3) {
> -        switch (mi) {
> -        case 4:
> -            /* dsp[rs].ub */
> -            dsp = rx_index_addr(ld, RX_MEMORY_BYTE, ctx);
> -            break;
> -        case 3:
> -            /* dsp[rs].uw */
> -            dsp = rx_index_addr(ld, RX_MEMORY_WORD, ctx);
> -            break;
> -        default:
> -            /* dsp[rs].b */
> -            /* dsp[rs].w */
> -            /* dsp[rs].l */
> -            dsp = rx_index_addr(ld, mi, ctx);
> -            break;
> -        }
> -        if (dsp > 0) {
> -            prt("%d", dsp);
> -        }
> -        prt("[r%d]%s", rs, sizes[mi]);
> +        rx_index_addr(ctx, dsp, ld, mi);
> +        prt("%s[r%d]%s", dsp, rs, sizes[mi]);
>      } else {
>          prt("r%d", rs);
>      }
> @@ -235,7 +228,7 @@ static bool trans_MOV_ra(DisasContext *ctx, arg_MOV_ra *a)
>  /* mov.[bwl] rs,rd */
>  static bool trans_MOV_mm(DisasContext *ctx, arg_MOV_mm *a)
>  {
> -    int dsp;
> +    char dspd[8], dsps[8];
>  
>      prt("mov.%c\t", size[a->sz]);
>      if (a->lds == 3 && a->ldd == 3) {
> @@ -244,29 +237,15 @@ static bool trans_MOV_mm(DisasContext *ctx, arg_MOV_mm *a)
>          return true;
>      }
>      if (a->lds == 3) {
> -        prt("r%d, ", a->rd);
> -        dsp = rx_index_addr(a->ldd, a->sz, ctx);
> -        if (dsp > 0) {
> -            prt("%d", dsp);
> -        }
> -        prt("[r%d]", a->rs);
> +        rx_index_addr(ctx, dspd, a->ldd, a->sz);
> +        prt("r%d, %s[r%d]", a->rs, dspd, a->rd);
>      } else if (a->ldd == 3) {
> -        dsp = rx_index_addr(a->lds, a->sz, ctx);
> -        if (dsp > 0) {
> -            prt("%d", dsp);
> -        }
> -        prt("[r%d], r%d", a->rs, a->rd);
> +        rx_index_addr(ctx, dsps, a->lds, a->sz);
> +        prt("%s[r%d], r%d", dsps, a->rs, a->rd);
>      } else {
> -        dsp = rx_index_addr(a->lds, a->sz, ctx);
> -        if (dsp > 0) {
> -            prt("%d", dsp);
> -        }
> -        prt("[r%d], ", a->rs);
> -        dsp = rx_index_addr(a->ldd, a->sz, ctx);
> -        if (dsp > 0) {
> -            prt("%d", dsp);
> -        }
> -        prt("[r%d]", a->rd);
> +        rx_index_addr(ctx, dsps, a->lds, a->sz);
> +        rx_index_addr(ctx, dspd, a->ldd, a->sz);
> +        prt("%s[r%d], %s[r%d]", dsps, a->rs, dspd, a->rd);
>      }
>      return true;
>  }
> @@ -357,12 +336,10 @@ static bool trans_PUSH_r(DisasContext *ctx, arg_PUSH_r *a)
>  /* push dsp[rs] */
>  static bool trans_PUSH_m(DisasContext *ctx, arg_PUSH_m *a)
>  {
> -    prt("push\t");
> -    int dsp = rx_index_addr(a->ld, a->sz, ctx);
> -    if (dsp > 0) {
> -        prt("%d", dsp);
> -    }
> -    prt("[r%d]", a->rs);
> +    char dsp[8];
> +
> +    rx_index_addr(ctx, dsp, a->ld, a->sz);
> +    prt("push\t%s[r%d]", dsp, a->rs);
>      return true;
>  }
>  
> @@ -389,17 +366,13 @@ static bool trans_XCHG_rr(DisasContext *ctx, arg_XCHG_rr *a)
>  /* xchg dsp[rs].<mi>,rd */
>  static bool trans_XCHG_mr(DisasContext *ctx, arg_XCHG_mr *a)
>  {
> -    int dsp;
>      static const char msize[][4] = {
>          "b", "w", "l", "ub", "uw",
>      };
> +    char dsp[8];
>  
> -    prt("xchg\t");
> -    dsp = rx_index_addr(a->ld, a->mi, ctx);
> -    if (dsp > 0) {
> -        prt("%d", dsp);
> -    }
> -    prt("[r%d].%s, r%d", a->rs, msize[a->mi], a->rd);
> +    rx_index_addr(ctx, dsp, a->ld, a->mi);
> +    prt("xchg\t%s[r%d].%s, r%d", dsp, a->rs, msize[a->mi], a->rd);
>      return true;
>  }
>  
> @@ -552,13 +525,10 @@ static bool trans_ADC_rr(DisasContext *ctx, arg_ADC_rr *a)
>  /* adc dsp[rs], rd */
>  static bool trans_ADC_mr(DisasContext *ctx, arg_ADC_mr *a)
>  {
> -    int dsp;
> -    prt("adc\t");
> -    dsp = rx_index_addr(a->ld, 2, ctx);
> -    if (dsp > 0) {
> -        prt("%d", dsp);
> -    }
> -    prt("[r%d], r%d", a->rs, a->rd);
> +    char dsp[8];
> +
> +    rx_index_addr(ctx, dsp, a->ld, 2);
> +    prt("adc\t%s[r%d], r%d", dsp, a->rs, a->rd);
>      return true;
>  }
>  
> @@ -1217,25 +1187,17 @@ static bool trans_ITOF(DisasContext *ctx, arg_ITOF *a)
>  
>  #define BOP_IM(name, reg)                                       \
>      do {                                                        \
> -        int dsp;                                                \
> -        prt("b%s\t#%d, ", #name, a->imm);                       \
> -        dsp = rx_index_addr(a->ld, RX_MEMORY_BYTE, ctx);        \
> -        if (dsp > 0) {                                          \
> -            prt("%d", dsp);                                     \
> -        }                                                       \
> -        prt("[r%d]", reg);                                      \
> +        char dsp[8];                                            \
> +        rx_index_addr(ctx, dsp, a->ld, RX_MEMORY_BYTE);         \
> +        prt("b%s\t#%d, %s[r%d]", #name, a->imm, dsp, reg);      \
>          return true;                                            \
>      } while (0)
>  
>  #define BOP_RM(name)                                            \
>      do {                                                        \
> -        int dsp;                                                \
> -        prt("b%s\tr%d, ", #name, a->rd);                        \
> -        dsp = rx_index_addr(a->ld, RX_MEMORY_BYTE, ctx);        \
> -        if (dsp > 0) {                                          \
> -            prt("%d", dsp);                                     \
> -        }                                                       \
> -        prt("[r%d]", a->rs);                                    \
> +        char dsp[8];                                            \
> +        rx_index_addr(ctx, dsp, a->ld, RX_MEMORY_BYTE);         \
> +        prt("b%s\tr%d, %s[r%d]", #name, a->rd, dsp, a->rs);     \
>          return true;                                            \
>      } while (0)
>  
> @@ -1346,12 +1308,10 @@ static bool trans_BNOT_ir(DisasContext *ctx, arg_BNOT_ir *a)
>  /* bmcond #imm, dsp[rd] */
>  static bool trans_BMCnd_im(DisasContext *ctx, arg_BMCnd_im *a)
>  {
> -    int dsp = rx_index_addr(a->ld, RX_MEMORY_BYTE, ctx);
> -    prt("bm%s\t#%d, ", cond[a->cd], a->imm);
> -    if (dsp > 0) {
> -        prt("%d", dsp);
> -    }
> -    prt("[%d]", a->rd);
> +    char dsp[8];
> +
> +    rx_index_addr(ctx, dsp, a->ld, RX_MEMORY_BYTE);
> +    prt("bm%s\t#%d, %s[r%d]", cond[a->cd], a->imm, dsp, a->rd);
>      return true;
>  }
>  
> @@ -1443,16 +1403,12 @@ static bool trans_WAIT(DisasContext *ctx, arg_WAIT *a)
>  /* sccnd.[bwl] dsp:[rd] */
>  static bool trans_SCCnd(DisasContext *ctx, arg_SCCnd *a)
>  {
> -    int dsp;
> -    prt("sc%s.%c\t", cond[a->cd], size[a->sz]);
>      if (a->ld < 3) {
> -        dsp = rx_index_addr(a->sz, a->ld, ctx);
> -        if (dsp > 0) {
> -            prt("%d", dsp);
> -        }
> -        prt("[r%d]", a->rd);
> +        char dsp[8];
> +        rx_index_addr(ctx, dsp, a->sz, a->ld);
> +        prt("sc%s.%c\t%s[r%d]", cond[a->cd], size[a->sz], dsp, a->rd);
>      } else {
> -        prt("r%d", a->rd);
> +        prt("sc%s.%c\tr%d", cond[a->cd], size[a->sz], a->rd);
>      }
>      return true;
>  }
> -- 
> 2.17.1
> 

Re: [Qemu-devel] [PATCH 2/6] target/rx: Replace operand with prt_ldmi in disassembler

[Yoshinori Sato]

On Fri, 24 May 2019 00:07:59 +0900,
Richard Henderson wrote:
> 
> This has consistency with prt_ri().  It loads all data before
> beginning output.  It uses exactly one call to prt() to emit
> the full instruction.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Yoshinori Sato <ysato@users.sourceforge.jp>
> ---
>  target/rx/disas.c | 77 +++++++++++++++++------------------------------
>  1 file changed, 27 insertions(+), 50 deletions(-)
> 
> diff --git a/target/rx/disas.c b/target/rx/disas.c
> index 64342537ee..515b365528 100644
> --- a/target/rx/disas.c
> +++ b/target/rx/disas.c
> @@ -135,18 +135,18 @@ static void rx_index_addr(DisasContext *ctx, char out[8], int ld, int mi)
>      sprintf(out, "%u", dsp << (mi < 3 ? mi : 4 - mi));
>  }
>  
> -static void operand(DisasContext *ctx, int ld, int mi, int rs, int rd)
> +static void prt_ldmi(DisasContext *ctx, const char *insn,
> +                     int ld, int mi, int rs, int rd)
>  {
>      static const char sizes[][4] = {".b", ".w", ".l", ".uw", ".ub"};
>      char dsp[8];
>  
>      if (ld < 3) {
>          rx_index_addr(ctx, dsp, ld, mi);
> -        prt("%s[r%d]%s", dsp, rs, sizes[mi]);
> +        prt("%s\t%s[r%d]%s, r%d", insn, dsp, rs, sizes[mi], rd);
>      } else {
> -        prt("r%d", rs);
> +        prt("%s\tr%d, r%d", insn, rs, rd);
>      }
> -    prt(", r%d", rd);
>  }
>  
>  static void prt_ir(DisasContext *ctx, const char *insn, int imm, int rd)
> @@ -416,8 +416,7 @@ static bool trans_AND_ir(DisasContext *ctx, arg_AND_ir *a)
>  /* and rs,rd */
>  static bool trans_AND_mr(DisasContext *ctx, arg_AND_mr *a)
>  {
> -    prt("and\t");
> -    operand(ctx, a->ld, a->mi, a->rs, a->rd);
> +    prt_ldmi(ctx, "and", a->ld, a->mi, a->rs, a->rd);
>      return true;
>  }
>  
> @@ -440,8 +439,7 @@ static bool trans_OR_ir(DisasContext *ctx, arg_OR_ir *a)
>  /* or rs,rd */
>  static bool trans_OR_mr(DisasContext *ctx, arg_OR_mr *a)
>  {
> -    prt("or\t");
> -    operand(ctx, a->ld, a->mi, a->rs, a->rd);
> +    prt_ldmi(ctx, "or", a->ld, a->mi, a->rs, a->rd);
>      return true;
>  }
>  
> @@ -463,8 +461,7 @@ static bool trans_XOR_ir(DisasContext *ctx, arg_XOR_ir *a)
>  /* xor rs,rd */
>  static bool trans_XOR_mr(DisasContext *ctx, arg_XOR_mr *a)
>  {
> -    prt("xor\t");
> -    operand(ctx, a->ld, a->mi, a->rs, a->rd);
> +    prt_ldmi(ctx, "xor", a->ld, a->mi, a->rs, a->rd);
>      return true;
>  }
>  
> @@ -479,8 +476,7 @@ static bool trans_TST_ir(DisasContext *ctx, arg_TST_ir *a)
>  /* tst rs, rd */
>  static bool trans_TST_mr(DisasContext *ctx, arg_TST_mr *a)
>  {
> -    prt("tst\t");
> -    operand(ctx, a->ld, a->mi, a->rs, a->rd);
> +    prt_ldmi(ctx, "tst", a->ld, a->mi, a->rs, a->rd);
>      return true;
>  }
>  
> @@ -548,8 +544,7 @@ static bool trans_ADD_irr(DisasContext *ctx, arg_ADD_irr *a)
>  /* add dsp[rs], rd */
>  static bool trans_ADD_mr(DisasContext *ctx, arg_ADD_mr *a)
>  {
> -    prt("add\t");
> -    operand(ctx, a->ld, a->mi, a->rs, a->rd);
> +    prt_ldmi(ctx, "add", a->ld, a->mi, a->rs, a->rd);
>      return true;
>  }
>  
> @@ -573,8 +568,7 @@ static bool trans_CMP_ir(DisasContext *ctx, arg_CMP_ir *a)
>  /* cmp dsp[rs], rs2 */
>  static bool trans_CMP_mr(DisasContext *ctx, arg_CMP_mr *a)
>  {
> -    prt("cmp\t");
> -    operand(ctx, a->ld, a->mi, a->rs, a->rd);
> +    prt_ldmi(ctx, "cmp", a->ld, a->mi, a->rs, a->rd);
>      return true;
>  }
>  
> @@ -589,8 +583,7 @@ static bool trans_SUB_ir(DisasContext *ctx, arg_SUB_ir *a)
>  /* sub dsp[rs], rd */
>  static bool trans_SUB_mr(DisasContext *ctx, arg_SUB_mr *a)
>  {
> -    prt("sub\t");
> -    operand(ctx, a->ld, a->mi, a->rs, a->rd);
> +    prt_ldmi(ctx, "sub", a->ld, a->mi, a->rs, a->rd);
>      return true;
>  }
>  
> @@ -611,8 +604,7 @@ static bool trans_SBB_rr(DisasContext *ctx, arg_SBB_rr *a)
>  /* sbb dsp[rs], rd */
>  static bool trans_SBB_mr(DisasContext *ctx, arg_SBB_mr *a)
>  {
> -    prt("sbb\t");
> -    operand(ctx, a->ld, RX_IM_LONG, a->rs, a->rd);
> +    prt_ldmi(ctx, "sbb", a->ld, RX_IM_LONG, a->rs, a->rd);
>      return true;
>  }
>  
> @@ -640,8 +632,7 @@ static bool trans_MAX_ir(DisasContext *ctx, arg_MAX_ir *a)
>  /* max dsp[rs], rd */
>  static bool trans_MAX_mr(DisasContext *ctx, arg_MAX_mr *a)
>  {
> -    prt("max\t");
> -    operand(ctx, a->ld, a->mi, a->rs, a->rd);
> +    prt_ldmi(ctx, "max", a->ld, a->mi, a->rs, a->rd);
>      return true;
>  }
>  
> @@ -656,8 +647,7 @@ static bool trans_MIN_ir(DisasContext *ctx, arg_MIN_ir *a)
>  /* min dsp[rs], rd */
>  static bool trans_MIN_mr(DisasContext *ctx, arg_MIN_mr *a)
>  {
> -    prt("max\t");
> -    operand(ctx, a->ld, a->mi, a->rs, a->rd);
> +    prt_ldmi(ctx, "min", a->ld, a->mi, a->rs, a->rd);
>      return true;
>  }
>  
> @@ -673,8 +663,7 @@ static bool trans_MUL_ir(DisasContext *ctx, arg_MUL_ir *a)
>  /* mul dsp[rs], rd */
>  static bool trans_MUL_mr(DisasContext *ctx, arg_MUL_mr *a)
>  {
> -    prt("mul\t");
> -    operand(ctx, a->ld, a->mi, a->rs, a->rd);
> +    prt_ldmi(ctx, "mul", a->ld, a->mi, a->rs, a->rd);
>      return true;
>  }
>  
> @@ -696,8 +685,7 @@ static bool trans_EMUL_ir(DisasContext *ctx, arg_EMUL_ir *a)
>  /* emul dsp[rs], rd */
>  static bool trans_EMUL_mr(DisasContext *ctx, arg_EMUL_mr *a)
>  {
> -    prt("emul\t");
> -    operand(ctx, a->ld, a->mi, a->rs, a->rd);
> +    prt_ldmi(ctx, "emul", a->ld, a->mi, a->rs, a->rd);
>      return true;
>  }
>  
> @@ -712,8 +700,7 @@ static bool trans_EMULU_ir(DisasContext *ctx, arg_EMULU_ir *a)
>  /* emulu dsp[rs], rd */
>  static bool trans_EMULU_mr(DisasContext *ctx, arg_EMULU_mr *a)
>  {
> -    prt("emulu\t");
> -    operand(ctx, a->ld, a->mi, a->rs, a->rd);
> +    prt_ldmi(ctx, "emulu", a->ld, a->mi, a->rs, a->rd);
>      return true;
>  }
>  
> @@ -728,8 +715,7 @@ static bool trans_DIV_ir(DisasContext *ctx, arg_DIV_ir *a)
>  /* div dsp[rs], rd */
>  static bool trans_DIV_mr(DisasContext *ctx, arg_DIV_mr *a)
>  {
> -    prt("div\t");
> -    operand(ctx, a->ld, a->mi, a->rs, a->rd);
> +    prt_ldmi(ctx, "div", a->ld, a->mi, a->rs, a->rd);
>      return true;
>  }
>  
> @@ -744,8 +730,7 @@ static bool trans_DIVU_ir(DisasContext *ctx, arg_DIVU_ir *a)
>  /* divu dsp[rs], rd */
>  static bool trans_DIVU_mr(DisasContext *ctx, arg_DIVU_mr *a)
>  {
> -    prt("divu\t");
> -    operand(ctx, a->ld, a->mi, a->rs, a->rd);
> +    prt_ldmi(ctx, "divu", a->ld, a->mi, a->rs, a->rd);
>      return true;
>  }
>  
> @@ -1089,8 +1074,7 @@ static bool trans_FADD_ir(DisasContext *ctx, arg_FADD_ir *a)
>  /* fadd rs, rd */
>  static bool trans_FADD_mr(DisasContext *ctx, arg_FADD_mr *a)
>  {
> -    prt("fadd\t");
> -    operand(ctx, a->ld, RX_IM_LONG, a->rs, a->rd);
> +    prt_ldmi(ctx, "fadd", a->ld, RX_IM_LONG, a->rs, a->rd);
>      return true;
>  }
>  
> @@ -1105,8 +1089,7 @@ static bool trans_FCMP_ir(DisasContext *ctx, arg_FCMP_ir *a)
>  /* fcmp rs, rd */
>  static bool trans_FCMP_mr(DisasContext *ctx, arg_FCMP_mr *a)
>  {
> -    prt("fcmp\t");
> -    operand(ctx, a->ld, RX_IM_LONG, a->rs, a->rd);
> +    prt_ldmi(ctx, "fcmp", a->ld, RX_IM_LONG, a->rs, a->rd);
>      return true;
>  }
>  
> @@ -1121,8 +1104,7 @@ static bool trans_FSUB_ir(DisasContext *ctx, arg_FSUB_ir *a)
>  /* fsub rs, rd */
>  static bool trans_FSUB_mr(DisasContext *ctx, arg_FSUB_mr *a)
>  {
> -    prt("fsub\t");
> -    operand(ctx, a->ld, RX_IM_LONG, a->rs, a->rd);
> +    prt_ldmi(ctx, "fsub", a->ld, RX_IM_LONG, a->rs, a->rd);
>      return true;
>  }
>  
> @@ -1130,8 +1112,7 @@ static bool trans_FSUB_mr(DisasContext *ctx, arg_FSUB_mr *a)
>  /* ftoi rs, rd */
>  static bool trans_FTOI(DisasContext *ctx, arg_FTOI *a)
>  {
> -    prt("ftoi\t");
> -    operand(ctx, a->ld, RX_IM_LONG, a->rs, a->rd);
> +    prt_ldmi(ctx, "ftoi", a->ld, RX_IM_LONG, a->rs, a->rd);
>      return true;
>  }
>  
> @@ -1146,8 +1127,7 @@ static bool trans_FMUL_ir(DisasContext *ctx, arg_FMUL_ir *a)
>  /* fmul rs, rd */
>  static bool trans_FMUL_mr(DisasContext *ctx, arg_FMUL_mr *a)
>  {
> -    prt("fmul\t");
> -    operand(ctx, a->ld, RX_IM_LONG, a->rs, a->rd);
> +    prt_ldmi(ctx, "fmul", a->ld, RX_IM_LONG, a->rs, a->rd);
>      return true;
>  }
>  
> @@ -1162,8 +1142,7 @@ static bool trans_FDIV_ir(DisasContext *ctx, arg_FDIV_ir *a)
>  /* fdiv rs, rd */
>  static bool trans_FDIV_mr(DisasContext *ctx, arg_FDIV_mr *a)
>  {
> -    prt("fdiv\t");
> -    operand(ctx, a->ld, RX_IM_LONG, a->rs, a->rd);
> +    prt_ldmi(ctx, "fdiv", a->ld, RX_IM_LONG, a->rs, a->rd);
>      return true;
>  }
>  
> @@ -1171,8 +1150,7 @@ static bool trans_FDIV_mr(DisasContext *ctx, arg_FDIV_mr *a)
>  /* round rs, rd */
>  static bool trans_ROUND(DisasContext *ctx, arg_ROUND *a)
>  {
> -    prt("round\t");
> -    operand(ctx, a->ld, RX_IM_LONG, a->rs, a->rd);
> +    prt_ldmi(ctx, "round", a->ld, RX_IM_LONG, a->rs, a->rd);
>      return true;
>  }
>  
> @@ -1180,8 +1158,7 @@ static bool trans_ROUND(DisasContext *ctx, arg_ROUND *a)
>  /* itof dsp[rs], rd */
>  static bool trans_ITOF(DisasContext *ctx, arg_ITOF *a)
>  {
> -    prt("itof\t");
> -    operand(ctx, a->ld, RX_IM_LONG, a->rs, a->rd);
> +    prt_ldmi(ctx, "itof", a->ld, RX_IM_LONG, a->rs, a->rd);
>      return true;
>  }
>  
> -- 
> 2.17.1
> 

Re: [Qemu-devel] [PATCH 3/6] target/rx: Use prt_ldmi for XCHG_mr disassembly

[Yoshinori Sato]

On Fri, 24 May 2019 00:08:00 +0900,
Richard Henderson wrote:
> 
> Note that the ld == 3 case handled by prt_ldmi is decoded as
> XCHG_rr and cannot appear here.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Yoshinori Sato <ysato@users.sourceforge.jp>

> ---
>  target/rx/disas.c | 8 +-------
>  1 file changed, 1 insertion(+), 7 deletions(-)
> 
> diff --git a/target/rx/disas.c b/target/rx/disas.c
> index 515b365528..db10385fd0 100644
> --- a/target/rx/disas.c
> +++ b/target/rx/disas.c
> @@ -366,13 +366,7 @@ static bool trans_XCHG_rr(DisasContext *ctx, arg_XCHG_rr *a)
>  /* xchg dsp[rs].<mi>,rd */
>  static bool trans_XCHG_mr(DisasContext *ctx, arg_XCHG_mr *a)
>  {
> -    static const char msize[][4] = {
> -        "b", "w", "l", "ub", "uw",
> -    };
> -    char dsp[8];
> -
> -    rx_index_addr(ctx, dsp, a->ld, a->mi);
> -    prt("xchg\t%s[r%d].%s, r%d", dsp, a->rs, msize[a->mi], a->rd);
> +    prt_ldmi(ctx, "xchg", a->ld, a->mi, a->rs, a->rd);
>      return true;
>  }
>  
> -- 
> 2.17.1
> 

Re: [Qemu-devel] [PATCH 6/6] target/rx: Dump bytes for each insn during disassembly

[Yoshinori Sato]

On Fri, 24 May 2019 00:08:03 +0900,
Richard Henderson wrote:
> 
> There are so many different forms of each RX instruction
> that it will be very useful to be able to look at the bytes
> to see on which path a bug may lie.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Yoshinori Sato <ysato@users.sourceforge.jp>

> ---
>  target/rx/disas.c | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/target/rx/disas.c b/target/rx/disas.c
> index 5a32a87534..d73b53db44 100644
> --- a/target/rx/disas.c
> +++ b/target/rx/disas.c
> @@ -102,7 +102,21 @@ static int bdsp_s(DisasContext *ctx, int d)
>  /* Include the auto-generated decoder.  */
>  #include "decode.inc.c"
>  
> -#define prt(...) (ctx->dis->fprintf_func)((ctx->dis->stream), __VA_ARGS__)
> +static void dump_bytes(DisasContext *ctx)
> +{
> +    int i, len = ctx->len;
> +
> +    for (i = 0; i < len; ++i) {
> +        ctx->dis->fprintf_func(ctx->dis->stream, "%02x ", ctx->bytes[i]);
> +    }
> +    ctx->dis->fprintf_func(ctx->dis->stream, "%*c", (8 - i) * 3, '\t');
> +}
> +
> +#define prt(...) \
> +    do {                                                        \
> +        dump_bytes(ctx);                                        \
> +        ctx->dis->fprintf_func(ctx->dis->stream, __VA_ARGS__);  \
> +    } while (0)
>  
>  #define RX_MEMORY_BYTE 0
>  #define RX_MEMORY_WORD 1
> -- 
> 2.17.1
> 

Re: [Qemu-devel] [PATCH 4/6] target/rx: Emit all disassembly in one prt()

[Yoshinori Sato]

On Fri, 24 May 2019 00:08:01 +0900,
Richard Henderson wrote:
> 
> Many of the multi-part prints have been eliminated by previous
> patches.  Eliminate the rest of them.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Yoshinori Sato <ysato@users.sourceforge.jp>

> ---
>  target/rx/disas.c | 75 ++++++++++++++++++++++++-----------------------
>  1 file changed, 39 insertions(+), 36 deletions(-)
> 
> diff --git a/target/rx/disas.c b/target/rx/disas.c
> index db10385fd0..ebc1a44249 100644
> --- a/target/rx/disas.c
> +++ b/target/rx/disas.c
> @@ -228,24 +228,21 @@ static bool trans_MOV_ra(DisasContext *ctx, arg_MOV_ra *a)
>  /* mov.[bwl] rs,rd */
>  static bool trans_MOV_mm(DisasContext *ctx, arg_MOV_mm *a)
>  {
> -    char dspd[8], dsps[8];
> +    char dspd[8], dsps[8], szc = size[a->sz];
>  
> -    prt("mov.%c\t", size[a->sz]);
>      if (a->lds == 3 && a->ldd == 3) {
>          /* mov.[bwl] rs,rd */
> -        prt("r%d, r%d", a->rs, a->rd);
> -        return true;
> -    }
> -    if (a->lds == 3) {
> +        prt("mov.%c\tr%d, r%d", szc, a->rs, a->rd);
> +    } else if (a->lds == 3) {
>          rx_index_addr(ctx, dspd, a->ldd, a->sz);
> -        prt("r%d, %s[r%d]", a->rs, dspd, a->rd);
> +        prt("mov.%c\tr%d, %s[r%d]", szc, a->rs, dspd, a->rd);
>      } else if (a->ldd == 3) {
>          rx_index_addr(ctx, dsps, a->lds, a->sz);
> -        prt("%s[r%d], r%d", dsps, a->rs, a->rd);
> +        prt("mov.%c\t%s[r%d], r%d", szc, dsps, a->rs, a->rd);
>      } else {
>          rx_index_addr(ctx, dsps, a->lds, a->sz);
>          rx_index_addr(ctx, dspd, a->ldd, a->sz);
> -        prt("%s[r%d], %s[r%d]", dsps, a->rs, dspd, a->rd);
> +        prt("mov.%c\t%s[r%d], %s[r%d]", szc, dsps, a->rs, dspd, a->rd);
>      }
>      return true;
>  }
> @@ -254,8 +251,11 @@ static bool trans_MOV_mm(DisasContext *ctx, arg_MOV_mm *a)
>  /* mov.[bwl] rs,[-rd] */
>  static bool trans_MOV_rp(DisasContext *ctx, arg_MOV_rp *a)
>  {
> -    prt("mov.%c\tr%d, ", size[a->sz], a->rs);
> -    prt((a->ad == 0) ? "[r%d+]" : "[-r%d]", a->rd);
> +    if (a->ad) {
> +        prt("mov.%c\tr%d, [-r%d]", size[a->sz], a->rs, a->rd);
> +    } else {
> +        prt("mov.%c\tr%d, [r%d+]", size[a->sz], a->rs, a->rd);
> +    }
>      return true;
>  }
>  
> @@ -263,9 +263,11 @@ static bool trans_MOV_rp(DisasContext *ctx, arg_MOV_rp *a)
>  /* mov.[bwl] [-rd],rs */
>  static bool trans_MOV_pr(DisasContext *ctx, arg_MOV_pr *a)
>  {
> -    prt("mov.%c\t", size[a->sz]);
> -    prt((a->ad == 0) ? "[r%d+]" : "[-r%d]", a->rd);
> -    prt(", r%d", a->rs);
> +    if (a->ad) {
> +        prt("mov.%c\t[-r%d], r%d", size[a->sz], a->rd, a->rs);
> +    } else {
> +        prt("mov.%c\t[r%d+], r%d", size[a->sz], a->rd, a->rs);
> +    }
>      return true;
>  }
>  
> @@ -299,9 +301,11 @@ static bool trans_MOVU_ar(DisasContext *ctx, arg_MOVU_ar *a)
>  /* movu.[bw] [-rs],rd */
>  static bool trans_MOVU_pr(DisasContext *ctx, arg_MOVU_pr *a)
>  {
> -    prt("movu.%c\t", size[a->sz]);
> -    prt((a->ad == 0) ? "[r%d+]" : "[-r%d]", a->rd);
> -    prt(", r%d", a->rs);
> +    if (a->ad) {
> +        prt("movu.%c\t[-r%d], r%d", size[a->sz], a->rd, a->rs);
> +    } else {
> +        prt("movu.%c\t[r%d+], r%d", size[a->sz], a->rd, a->rs);
> +    }
>      return true;
>  }
>  
> @@ -478,11 +482,11 @@ static bool trans_TST_mr(DisasContext *ctx, arg_TST_mr *a)
>  /* not rs, rd */
>  static bool trans_NOT_rr(DisasContext *ctx, arg_NOT_rr *a)
>  {
> -    prt("not\t");
>      if (a->rs != a->rd) {
> -        prt("r%d, ", a->rs);
> +        prt("not\tr%d, r%d", a->rs, a->rd);
> +    } else {
> +        prt("not\tr%d", a->rs);
>      }
> -    prt("r%d", a->rd);
>      return true;
>  }
>  
> @@ -490,11 +494,11 @@ static bool trans_NOT_rr(DisasContext *ctx, arg_NOT_rr *a)
>  /* neg rs, rd */
>  static bool trans_NEG_rr(DisasContext *ctx, arg_NEG_rr *a)
>  {
> -    prt("neg\t");
>      if (a->rs != a->rd) {
> -        prt("r%d, ", a->rs);
> +        prt("neg\tr%d, r%d", a->rs, a->rd);
> +    } else {
> +        prt("neg\tr%d", a->rs);
>      }
> -    prt("r%d", a->rd);
>      return true;
>  }
>  
> @@ -606,11 +610,10 @@ static bool trans_SBB_mr(DisasContext *ctx, arg_SBB_mr *a)
>  /* abs rs, rd */
>  static bool trans_ABS_rr(DisasContext *ctx, arg_ABS_rr *a)
>  {
> -    prt("abs\t");
> -    if (a->rs == a->rd) {
> -        prt("r%d", a->rd);
> +    if (a->rs != a->rd) {
> +        prt("abs\tr%d, r%d", a->rs, a->rd);
>      } else {
> -        prt("r%d, r%d", a->rs, a->rd);
> +        prt("abs\tr%d", a->rs);
>      }
>      return true;
>  }
> @@ -733,11 +736,11 @@ static bool trans_DIVU_mr(DisasContext *ctx, arg_DIVU_mr *a)
>  /* shll #imm:5, rs, rd */
>  static bool trans_SHLL_irr(DisasContext *ctx, arg_SHLL_irr *a)
>  {
> -    prt("shll\t#%d, ", a->imm);
>      if (a->rs2 != a->rd) {
> -        prt("r%d, ", a->rs2);
> +        prt("shll\t#%d, r%d, r%d", a->imm, a->rs2, a->rd);
> +    } else {
> +        prt("shll\t#%d, r%d", a->imm, a->rd);
>      }
> -    prt("r%d", a->rd);
>      return true;
>  }
>  
> @@ -752,11 +755,11 @@ static bool trans_SHLL_rr(DisasContext *ctx, arg_SHLL_rr *a)
>  /* shar #imm:5, rs, rd */
>  static bool trans_SHAR_irr(DisasContext *ctx, arg_SHAR_irr *a)
>  {
> -    prt("shar\t#%d,", a->imm);
>      if (a->rs2 != a->rd) {
> -        prt("r%d, ", a->rs2);
> +        prt("shar\t#%d, r%d, r%d", a->imm, a->rs2, a->rd);
> +    } else {
> +        prt("shar\t#%d, r%d", a->imm, a->rd);
>      }
> -    prt("r%d", a->rd);
>      return true;
>  }
>  
> @@ -771,11 +774,11 @@ static bool trans_SHAR_rr(DisasContext *ctx, arg_SHAR_rr *a)
>  /* shlr #imm:5, rs, rd */
>  static bool trans_SHLR_irr(DisasContext *ctx, arg_SHLR_irr *a)
>  {
> -    prt("shlr\t#%d, ", a->imm);
>      if (a->rs2 != a->rd) {
> -        prt("r%d, ", a->rs2);
> +        prt("shlr\t#%d, r%d, r%d", a->imm, a->rs2, a->rd);
> +    } else {
> +        prt("shlr\t#%d, r%d", a->imm, a->rd);
>      }
> -    prt("r%d", a->rd);
>      return true;
>  }
>  
> -- 
> 2.17.1
> 

Re: [Qemu-devel] [PATCH 5/6] target/rx: Collect all bytes during disassembly

[Yoshinori Sato]

On Fri, 24 May 2019 00:08:02 +0900,
Richard Henderson wrote:
> 
> Collected, to be used in the next patch.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Yoshinori Sato <ysato@users.sourceforge.jp>

> ---
>  target/rx/disas.c | 62 ++++++++++++++++++++++++++++++++---------------
>  1 file changed, 42 insertions(+), 20 deletions(-)
> 
> diff --git a/target/rx/disas.c b/target/rx/disas.c
> index ebc1a44249..5a32a87534 100644
> --- a/target/rx/disas.c
> +++ b/target/rx/disas.c
> @@ -25,43 +25,59 @@ typedef struct DisasContext {
>      disassemble_info *dis;
>      uint32_t addr;
>      uint32_t pc;
> +    uint8_t len;
> +    uint8_t bytes[8];
>  } DisasContext;
>  
>  
>  static uint32_t decode_load_bytes(DisasContext *ctx, uint32_t insn,
> -                           int i, int n)
> +                                  int i, int n)
>  {
> -    bfd_byte buf;
> +    uint32_t addr = ctx->addr;
> +
> +    g_assert(ctx->len == i);
> +    g_assert(n <= ARRAY_SIZE(ctx->bytes));
> +
>      while (++i <= n) {
> -        ctx->dis->read_memory_func(ctx->addr++, &buf, 1, ctx->dis);
> -        insn |= buf << (32 - i * 8);
> +        ctx->dis->read_memory_func(addr++, &ctx->bytes[i - 1], 1, ctx->dis);
> +        insn |= ctx->bytes[i - 1] << (32 - i * 8);
>      }
> +    ctx->addr = addr;
> +    ctx->len = n;
> +
>      return insn;
>  }
>  
>  static int32_t li(DisasContext *ctx, int sz)
>  {
> -    int32_t addr;
> -    bfd_byte buf[4];
> -    addr = ctx->addr;
> +    uint32_t addr = ctx->addr;
> +    uintptr_t len = ctx->len;
>  
>      switch (sz) {
>      case 1:
> +        g_assert(len + 1 <= ARRAY_SIZE(ctx->bytes));
>          ctx->addr += 1;
> -        ctx->dis->read_memory_func(addr, buf, 1, ctx->dis);
> -        return (int8_t)buf[0];
> +        ctx->len += 1;
> +        ctx->dis->read_memory_func(addr, ctx->bytes + len, 1, ctx->dis);
> +        return (int8_t)ctx->bytes[len];
>      case 2:
> +        g_assert(len + 2 <= ARRAY_SIZE(ctx->bytes));
>          ctx->addr += 2;
> -        ctx->dis->read_memory_func(addr, buf, 2, ctx->dis);
> -        return ldsw_le_p(buf);
> +        ctx->len += 2;
> +        ctx->dis->read_memory_func(addr, ctx->bytes + len, 2, ctx->dis);
> +        return ldsw_le_p(ctx->bytes + len);
>      case 3:
> +        g_assert(len + 3 <= ARRAY_SIZE(ctx->bytes));
>          ctx->addr += 3;
> -        ctx->dis->read_memory_func(addr, buf, 3, ctx->dis);
> -        return (int8_t)buf[2] << 16 | lduw_le_p(buf);
> +        ctx->len += 3;
> +        ctx->dis->read_memory_func(addr, ctx->bytes + len, 3, ctx->dis);
> +        return (int8_t)ctx->bytes[len + 2] << 16 | lduw_le_p(ctx->bytes + len);
>      case 0:
> +        g_assert(len + 4 <= ARRAY_SIZE(ctx->bytes));
>          ctx->addr += 4;
> -        ctx->dis->read_memory_func(addr, buf, 4, ctx->dis);
> -        return ldl_le_p(buf);
> +        ctx->len += 4;
> +        ctx->dis->read_memory_func(addr, ctx->bytes + len, 4, ctx->dis);
> +        return ldl_le_p(ctx->bytes + len);
>      default:
>          g_assert_not_reached();
>      }
> @@ -110,7 +126,7 @@ static const char psw[] = {
>  static void rx_index_addr(DisasContext *ctx, char out[8], int ld, int mi)
>  {
>      uint32_t addr = ctx->addr;
> -    uint8_t buf[2];
> +    uintptr_t len = ctx->len;
>      uint16_t dsp;
>  
>      switch (ld) {
> @@ -119,14 +135,18 @@ static void rx_index_addr(DisasContext *ctx, char out[8], int ld, int mi)
>          out[0] = '\0';
>          return;
>      case 1:
> +        g_assert(len + 1 <= ARRAY_SIZE(ctx->bytes));
>          ctx->addr += 1;
> -        ctx->dis->read_memory_func(addr, buf, 1, ctx->dis);
> -        dsp = buf[0];
> +        ctx->len += 1;
> +        ctx->dis->read_memory_func(addr, ctx->bytes + len, 1, ctx->dis);
> +        dsp = ctx->bytes[len];
>          break;
>      case 2:
> +        g_assert(len + 2 <= ARRAY_SIZE(ctx->bytes));
>          ctx->addr += 2;
> -        ctx->dis->read_memory_func(addr, buf, 2, ctx->dis);
> -        dsp = lduw_le_p(buf);
> +        ctx->len += 2;
> +        ctx->dis->read_memory_func(addr, ctx->bytes + len, 2, ctx->dis);
> +        dsp = lduw_le_p(ctx->bytes + len);
>          break;
>      default:
>          g_assert_not_reached();
> @@ -1392,8 +1412,10 @@ int print_insn_rx(bfd_vma addr, disassemble_info *dis)
>      DisasContext ctx;
>      uint32_t insn;
>      int i;
> +
>      ctx.dis = dis;
>      ctx.pc = ctx.addr = addr;
> +    ctx.len = 0;
>  
>      insn = decode_load(&ctx);
>      if (!decode(&ctx, insn)) {
> -- 
> 2.17.1
> 

Re: [Qemu-devel] [PATCH 0/6] target/rx: Improvements to disassembly

[Yoshinori Sato]

On Fri, 24 May 2019 00:07:57 +0900,
Richard Henderson wrote:
> 
> Here's a sample of the new output, taken from u-boot.bin:
> 
> IN:
> 0xfff8000a:  fb 12 00 01 00 00          mov.l   #0x00000100, r1
> 0xfff80010:  fb 32 f0 13 00 00          mov.l   #0x000013f0, r3
> 0xfff80016:  43 13                      sub     r1, r3
> 0xfff80018:  fb 22 00 ea f9 ff          mov.l   #-398848, r2
> 0xfff8001e:  7f 8f                      smovf
> 0xfff80020:  ef 01                      mov.l   r0, r1
> 0xfff80022:  05 1e 32 00                bsr.a   fff83240
> 
> IN:
> 0xfff83240:  72 11 5c fb                add     #-1188, r1
> 0xfff83244:  75 21 f0                   and     #-16, r1
> 0xfff83247:  02                         rts
> 
> Obviously there are still a few inconsistencies in the
> format strings used for the immediates, but the format
> is readable and it is easy to look at the opcode to see
> how our decode compares to the manual.
>

Hmm.
The output of the immediate value should be the same as the output of objdump.
I do not think that it is the proper format, but I did that because
it was useful for comparing the results.

> 
> r~
> 
> 
> Richard Henderson (6):
>   target/rx: Disassemble rx_index_addr into a string
>   target/rx: Replace operand with prt_ldmi in disassembler
>   target/rx: Use prt_ldmi for XCHG_mr disassembly
>   target/rx: Emit all disassembly in one prt()
>   target/rx: Collect all bytes during disassembly
>   target/rx: Dump bytes for each insn during disassembly
> 
>  target/rx/disas.c | 366 +++++++++++++++++++++-------------------------
>  1 file changed, 166 insertions(+), 200 deletions(-)
> 
> -- 
> 2.17.1
> 

-- 
Yosinori Sato

Re: [Qemu-devel] [PATCH 0/6] target/rx: Improvements to disassembly

[Aleksandar Markovic]

On May 27, 2019 5:44 PM, "Yoshinori Sato" <ysato@users.sourceforge.jp>
wrote:
>
> On Fri, 24 May 2019 00:07:57 +0900,
> Richard Henderson wrote:
> >
> > Here's a sample of the new output, taken from u-boot.bin:
> >
> > IN:
> > 0xfff8000a:  fb 12 00 01 00 00          mov.l   #0x00000100, r1
> > 0xfff80010:  fb 32 f0 13 00 00          mov.l   #0x000013f0, r3
> > 0xfff80016:  43 13                      sub     r1, r3
> > 0xfff80018:  fb 22 00 ea f9 ff          mov.l   #-398848, r2
> > 0xfff8001e:  7f 8f                      smovf
> > 0xfff80020:  ef 01                      mov.l   r0, r1
> > 0xfff80022:  05 1e 32 00                bsr.a   fff83240
> >
> > IN:
> > 0xfff83240:  72 11 5c fb                add     #-1188, r1
> > 0xfff83244:  75 21 f0                   and     #-16, r1
> > 0xfff83247:  02                         rts
> >
> > Obviously there are still a few inconsistencies in the
> > format strings used for the immediates, but the format
> > is readable and it is easy to look at the opcode to see
> > how our decode compares to the manual.
> >
>
> Hmm.
> The output of the immediate value should be the same as the output of
objdump.
> I do not think that it is the proper format, but I did that because
> it was useful for comparing the results.
>

We in MIPS also use objdump output as the reference and desired output for
QEMU disassembler (not that we are always succeeding in doing that)

Inventing propriatery QEMU output for some instructions is in my view
counterproductive and confusing.

Sincerely,
Aleksandar

> >
> > r~
> >
> >
> > Richard Henderson (6):
> >   target/rx: Disassemble rx_index_addr into a string
> >   target/rx: Replace operand with prt_ldmi in disassembler
> >   target/rx: Use prt_ldmi for XCHG_mr disassembly
> >   target/rx: Emit all disassembly in one prt()
> >   target/rx: Collect all bytes during disassembly
> >   target/rx: Dump bytes for each insn during disassembly
> >
> >  target/rx/disas.c | 366 +++++++++++++++++++++-------------------------
> >  1 file changed, 166 insertions(+), 200 deletions(-)
> >
> > --
> > 2.17.1
> >
>
> --
> Yosinori Sato
>

Re: [Qemu-devel] [PATCH 0/6] target/rx: Improvements to disassembly

[Yoshinori Sato]

On Tue, 28 May 2019 02:47:40 +0900,
Aleksandar Markovic wrote:
> 
> On May 27, 2019 5:44 PM, "Yoshinori Sato" <ysato@users.sourceforge.jp>
> wrote:
> >
> > On Fri, 24 May 2019 00:07:57 +0900,
> > Richard Henderson wrote:
> > >
> > > Here's a sample of the new output, taken from u-boot.bin:
> > >
> > > IN:
> > > 0xfff8000a:  fb 12 00 01 00 00          mov.l   #0x00000100, r1
> > > 0xfff80010:  fb 32 f0 13 00 00          mov.l   #0x000013f0, r3
> > > 0xfff80016:  43 13                      sub     r1, r3
> > > 0xfff80018:  fb 22 00 ea f9 ff          mov.l   #-398848, r2
> > > 0xfff8001e:  7f 8f                      smovf
> > > 0xfff80020:  ef 01                      mov.l   r0, r1
> > > 0xfff80022:  05 1e 32 00                bsr.a   fff83240
> > >
> > > IN:
> > > 0xfff83240:  72 11 5c fb                add     #-1188, r1
> > > 0xfff83244:  75 21 f0                   and     #-16, r1
> > > 0xfff83247:  02                         rts
> > >
> > > Obviously there are still a few inconsistencies in the
> > > format strings used for the immediates, but the format
> > > is readable and it is easy to look at the opcode to see
> > > how our decode compares to the manual.
> > >
> >
> > Hmm.
> > The output of the immediate value should be the same as the output of
> objdump.
> > I do not think that it is the proper format, but I did that because
> > it was useful for comparing the results.
> >
> 
> We in MIPS also use objdump output as the reference and desired output for
> QEMU disassembler (not that we are always succeeding in doing that)
> 
> Inventing propriatery QEMU output for some instructions is in my view
> counterproductive and confusing.
> 
> Sincerely,
> Aleksandar

I also considered it. Although the opcodes of RX were licensed as GPLv3,
they were not usable as they were.

> 
> > >
> > > r~
> > >
> > >
> > > Richard Henderson (6):
> > >   target/rx: Disassemble rx_index_addr into a string
> > >   target/rx: Replace operand with prt_ldmi in disassembler
> > >   target/rx: Use prt_ldmi for XCHG_mr disassembly
> > >   target/rx: Emit all disassembly in one prt()
> > >   target/rx: Collect all bytes during disassembly
> > >   target/rx: Dump bytes for each insn during disassembly
> > >
> > >  target/rx/disas.c | 366 +++++++++++++++++++++-------------------------
> > >  1 file changed, 166 insertions(+), 200 deletions(-)
> > >
> > > --
> > > 2.17.1
> > >
> >
> > --
> > Yosinori Sato
> >

-- 
Yosinori Sato

Re: [Qemu-devel] [PATCH 0/6] target/rx: Improvements to disassembly

[Richard Henderson]

On 5/27/19 10:39 AM, Yoshinori Sato wrote:
> On Fri, 24 May 2019 00:07:57 +0900,
> Richard Henderson wrote:
>> Obviously there are still a few inconsistencies in the
>> format strings used for the immediates, but the format
>> is readable and it is easy to look at the opcode to see
>> how our decode compares to the manual.
>>
> 
> Hmm.
> The output of the immediate value should be the same as the output of objdump.
> I do not think that it is the proper format, but I did that because
> it was useful for comparing the results.

This is a fair comment.

Of course, we could also simultaneously fix inconsistencies in objdump.  I
doubt that any of these cases are intentionally different.

Something for later, perhaps.


r~