Hello Aurelien, thanks for your comments and review. Version 2 of the patch is in the attachment.
Diff between versions 1 & 2 according to your comments is : diff --git a/target-mips/translate.c b/target-mips/translate.c index f20678c..d2443d3 100644 --- a/target-mips/translate.c +++ b/target-mips/translate.c @@ -4632,12 +4632,13 @@ static void gen_align(DisasContext *ctx, int opc, int rd, int rs, int rt, if (bp == 0) { switch (opc) { case OPC_ALIGN: + tcg_gen_ext32s_tl(cpu_gpr[rd], t0); + break; #if defined(TARGET_MIPS64) - tcg_gen_ext32s_i64(cpu_gpr[rd], t0); + case OPC_DALIGN: + tcg_gen_mov_tl(cpu_gpr[rd], t0); break; #endif - default: - tcg_gen_mov_tl(cpu_gpr[rd], t0); } } else { TCGv t1 = tcg_temp_new(); * As you suggested I used tcg_gen_ext32s_tl() instead of tcg_gen_ext32s_i64() for the OPC_ALIGN case. * I've kept the "TARGET_MIPS64" ifdef guard for the OPC_DALIGN case, to keep the change in-line with the rest of the code where this 64-bit instruction opcode is used. Thank you. Regards, Miodrag ________________________________________ From: Aurelien Jarno [aurel...@aurel32.net] Sent: Friday, January 01, 2016 2:02 PM To: Miodrag Dinic Cc: qemu-devel@nongnu.org; Petar Jovanovic Subject: Re: [PATCH] target-mips: Fix ALIGN instruction when bp=0 [snip] > From e01539a11061c447bece8dccde1715da9534024d Mon Sep 17 00:00:00 2001 > From: Miodrag Dinic <miodrag.di...@imgtec.com> > Date: Thu, 3 Dec 2015 16:48:57 +0100 > Subject: [PATCH] target-mips: Fix ALIGN instruction when bp=0 > > If executing ALIGN with shift count bp=0 within mips64 emulation, > the result of the operation should be sign extended. > > Taken from the official documentation (pseudo code) : > > ALIGN: > tmp_rt_hi = unsigned_word(GPR[rt]) << (8*bp) > tmp_rs_lo = unsigned_word(GPR[rs]) >> (8*(4-bp)) > tmp = tmp_rt_hi || tmp_rt_lo > GPR[rd] = sign_extend.32(tmp) > > Signed-off-by: Miodrag Dinic <miodrag.di...@imgtec.com> > --- > target-mips/translate.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/target-mips/translate.c b/target-mips/translate.c > index 5626647..f20678c 100644 > --- a/target-mips/translate.c > +++ b/target-mips/translate.c > @@ -4630,7 +4630,15 @@ static void gen_align(DisasContext *ctx, int opc, int > rd, int rs, int rt, > t0 = tcg_temp_new(); > gen_load_gpr(t0, rt); > if (bp == 0) { > - tcg_gen_mov_tl(cpu_gpr[rd], t0); > + switch (opc) { > + case OPC_ALIGN: > +#if defined(TARGET_MIPS64) > + tcg_gen_ext32s_i64(cpu_gpr[rd], t0); > + break; > +#endif The way to fix that is basically ok. However you should just use tcg_gen_ext32s_tl instead of tcg_gen_ext32s_i64 and drop the TARGET_MIPS64 #ifdef. > + default: Then you can replace this by OPC_DALIGN for more clarity. > + tcg_gen_mov_tl(cpu_gpr[rd], t0); > + } > } else { > TCGv t1 = tcg_temp_new(); > gen_load_gpr(t1, rs); The resulting binary code should be the same, but less #ifdef means less risk of breakage, as the code is always compiled. -- Aurelien Jarno GPG: 4096R/1DDD8C9B aurel...@aurel32.net http://www.aurel32.net
From e48787a25de9a04985226cd7651795403d5752e6 Mon Sep 17 00:00:00 2001 From: Miodrag Dinic <miodrag.di...@imgtec.com> Date: Thu, 3 Dec 2015 16:48:57 +0100 Subject: [PATCH] [PATCH v2] target-mips: Fix ALIGN instruction when bp=0 If executing ALIGN with shift count bp=0 within mips64 emulation, the result of the operation should be sign extended. Taken from the official documentation (pseudo code) : ALIGN: tmp_rt_hi = unsigned_word(GPR[rt]) << (8*bp) tmp_rs_lo = unsigned_word(GPR[rs]) >> (8*(4-bp)) tmp = tmp_rt_hi || tmp_rt_lo GPR[rd] = sign_extend.32(tmp) Signed-off-by: Miodrag Dinic <miodrag.di...@imgtec.com> --- target-mips/translate.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/target-mips/translate.c b/target-mips/translate.c index 5626647..d2443d3 100644 --- a/target-mips/translate.c +++ b/target-mips/translate.c @@ -4630,7 +4630,16 @@ static void gen_align(DisasContext *ctx, int opc, int rd, int rs, int rt, t0 = tcg_temp_new(); gen_load_gpr(t0, rt); if (bp == 0) { - tcg_gen_mov_tl(cpu_gpr[rd], t0); + switch (opc) { + case OPC_ALIGN: + tcg_gen_ext32s_tl(cpu_gpr[rd], t0); + break; +#if defined(TARGET_MIPS64) + case OPC_DALIGN: + tcg_gen_mov_tl(cpu_gpr[rd], t0); + break; +#endif + } } else { TCGv t1 = tcg_temp_new(); gen_load_gpr(t1, rs); -- 1.9.1