On 16.10.18. 16:00, Peter Maydell wrote:
On 23 August 2018 at 14:34, Aleksandar Markovic
<aleksandar.marko...@rt-rk.com> wrote:
From: Stefan Markovic <smarko...@wavecomp.com>

Add emulation of DSP ASE instructions for nanoMIPS - part 1.

Reviewed-by: Aleksandar Markovic <amarko...@wavecomp.com>
Signed-off-by: Aleksandar Markovic <amarko...@wavecomp.com>
Signed-off-by: Stefan Markovic <smarko...@wavecomp.com>
Hi. Coverity points out a bug in this patch (CID 1395627):

---
  target/mips/translate.c | 554 ++++++++++++++++++++++++++++++++++++++++++++++++
  1 file changed, 554 insertions(+)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index 95632dd..d3635e7 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -18061,6 +18061,554 @@ static void gen_pool32f_nanomips_insn(DisasContext 
*ctx)
      }
  }

+static void gen_pool32a5_nanomips_insn(DisasContext *ctx, int opc,
+                                       int rd, int rs, int rt)
+{
[...]

+    case NM_SHRA_R_PH:
+        check_dsp(ctx);
+        tcg_gen_movi_tl(t0, rd >> 1);
+        switch (extract32(ctx->opcode, 10, 1)) {
+        case 0:
+            /* SHRA_PH */
+            gen_helper_shra_ph(v1_t, t0, v1_t);
+            break;
+            gen_store_gpr(v1_t, rt);
This gen_store_gpr() call is unreachable because it
is after the 'break'. Should the two lines be in the
other order?


Yes, those two lines should be in the other order:

+        case 0:
+            /* SHRA_PH */
+            gen_helper_shra_ph(v1_t, t0, v1_t);
+            gen_store_gpr(v1_t, rt);
+            break;


+        case 1:
+            /* SHRA_R_PH */
+            gen_helper_shra_r_ph(v1_t, t0, v1_t);
+            gen_store_gpr(v1_t, rt);
+            break;
+        }
+        break;
thanks
-- PMM

Reply via email to