[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PATCH v2] target-mips: Fix ALIGN instruction when bp=0
From: |
Miodrag Dinic |
Subject: |
[Qemu-devel] [PATCH v2] target-mips: Fix ALIGN instruction when bp=0 |
Date: |
Mon, 4 Jan 2016 15:52:04 +0000 |
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 address@hidden
Sent: Friday, January 01, 2016 2:02 PM
To: Miodrag Dinic
Cc: address@hidden; 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 <address@hidden>
> 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 <address@hidden>
> ---
> 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
address@hidden http://www.aurel32.net
0001-PATCH-v2-target-mips-Fix-ALIGN-instruction-when-bp-0.patch
Description: 0001-PATCH-v2-target-mips-Fix-ALIGN-instruction-when-bp-0.patch
- [Qemu-devel] [PATCH v2] target-mips: Fix ALIGN instruction when bp=0,
Miodrag Dinic <=