Re: [Qemu-devel] [PATCH 2/2] target-mips/translate.c: Add judgement for msb and lsb

[Aurelien Jarno <aurelien@aurel32.net>]

On Mon, Jul 28, 2014 at 11:58:22PM +0800, Dongxue Zhang wrote:
> Use 'if' to make sure the real msb greater than the lsb. As the compiler may
> not do this.

What are you trying to fix exactly? These cases are defined as
"unpredictable" in the MIPS ISA manual, which is what is implemented in
QEMU. In addition on the MIPS64R2 implementations I tested (Cavium
Octeon, Loongson 3) these cases do not trigger a reserved
instruction exception.

> Signed-off-by: Dongxue Zhang <elta.era@gmail.com>
> ---
>  target-mips/translate.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/target-mips/translate.c b/target-mips/translate.c
> index c381366..e2cce31 100644
> --- a/target-mips/translate.c
> +++ b/target-mips/translate.c
> @@ -3946,14 +3946,23 @@ static void gen_bitops (DisasContext *ctx, uint32_t opc, int rt,
>          break;
>  #if defined(TARGET_MIPS64)
>      case OPC_DINSM:
> +        if (lsb > (msb + 32)) {
> +            goto fail;
> +        }

This test is always false, as lsb and msb are 5 bits values.

>          gen_load_gpr(t0, rt);
>          tcg_gen_deposit_tl(t0, t0, t1, lsb, msb + 32 - lsb + 1);
>          break;
>      case OPC_DINSU:
> +        if (lsb > msb) {
> +            goto fail;
> +        }
>          gen_load_gpr(t0, rt);
>          tcg_gen_deposit_tl(t0, t0, t1, lsb + 32, msb - lsb + 1);
>          break;
>      case OPC_DINS:
> +        if (lsb > msb) {
> +            goto fail;
> +        }
>          gen_load_gpr(t0, rt);
>          tcg_gen_deposit_tl(t0, t0, t1, lsb, msb - lsb + 1);
>          break;
> -- 
> 1.8.1.2
> 
> 
> 

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net