Re: [Qemu-devel] [PATCH] target-mips: Fix warning about shifting negative value

[Paolo Bonzini]

On 07/08/2016 03:15, Pranith Kumar wrote:
> With latest clang you get the following warning:
> 
> warning: shifting a negative signed value is undefined 
> [-Wshift-negative-value]
> 
> Fix the warning by changing negative shift to inverse 0 shift.
> 
> Signed-off-by: Pranith Kumar <address@hidden>
> ---
>  target-mips/translate.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/target-mips/translate.c b/target-mips/translate.c
> index bab52cb..d224de0 100644
> --- a/target-mips/translate.c
> +++ b/target-mips/translate.c
> @@ -3282,7 +3282,7 @@ static void gen_r6_muldiv(DisasContext *ctx, int opc, 
> int rd, int rs, int rt)
>          {
>              TCGv t2 = tcg_temp_new();
>              TCGv t3 = tcg_temp_new();
> -            tcg_gen_setcondi_tl(TCG_COND_EQ, t2, t0, -1LL << 63);
> +            tcg_gen_setcondi_tl(TCG_COND_EQ, t2, t0, ~0ULL << 63);
>              tcg_gen_setcondi_tl(TCG_COND_EQ, t3, t1, -1LL);
>              tcg_gen_and_tl(t2, t2, t3);
>              tcg_gen_setcondi_tl(TCG_COND_EQ, t3, t1, 0);
> @@ -3298,7 +3298,7 @@ static void gen_r6_muldiv(DisasContext *ctx, int opc, 
> int rd, int rs, int rt)
>          {
>              TCGv t2 = tcg_temp_new();
>              TCGv t3 = tcg_temp_new();
> -            tcg_gen_setcondi_tl(TCG_COND_EQ, t2, t0, -1LL << 63);
> +            tcg_gen_setcondi_tl(TCG_COND_EQ, t2, t0, ~0ULL << 63);
>              tcg_gen_setcondi_tl(TCG_COND_EQ, t3, t1, -1LL);
>              tcg_gen_and_tl(t2, t2, t3);
>              tcg_gen_setcondi_tl(TCG_COND_EQ, t3, t1, 0);
> @@ -3444,7 +3444,7 @@ static void gen_muldiv(DisasContext *ctx, uint32_t opc,
>          {
>              TCGv t2 = tcg_temp_new();
>              TCGv t3 = tcg_temp_new();
> -            tcg_gen_setcondi_tl(TCG_COND_EQ, t2, t0, -1LL << 63);
> +            tcg_gen_setcondi_tl(TCG_COND_EQ, t2, t0, ~0ULL << 63);
>              tcg_gen_setcondi_tl(TCG_COND_EQ, t3, t1, -1LL);
>              tcg_gen_and_tl(t2, t2, t3);
>              tcg_gen_setcondi_tl(TCG_COND_EQ, t3, t1, 0);
> @@ -3791,7 +3791,7 @@ static void gen_loongson_integer(DisasContext *ctx, 
> uint32_t opc,
>              tcg_gen_movi_tl(cpu_gpr[rd], 0);
>              tcg_gen_br(l3);
>              gen_set_label(l1);
> -            tcg_gen_brcondi_tl(TCG_COND_NE, t0, -1LL << 63, l2);
> +            tcg_gen_brcondi_tl(TCG_COND_NE, t0, ~0ULL << 63, l2);
>              tcg_gen_brcondi_tl(TCG_COND_NE, t1, -1LL, l2);
>              tcg_gen_mov_tl(cpu_gpr[rd], t0);
>              tcg_gen_br(l3);
> @@ -3820,7 +3820,7 @@ static void gen_loongson_integer(DisasContext *ctx, 
> uint32_t opc,
>              TCGLabel *l2 = gen_new_label();
>              TCGLabel *l3 = gen_new_label();
>              tcg_gen_brcondi_tl(TCG_COND_EQ, t1, 0, l1);
> -            tcg_gen_brcondi_tl(TCG_COND_NE, t0, -1LL << 63, l2);
> +            tcg_gen_brcondi_tl(TCG_COND_NE, t0, ~0ULL << 63, l2);
>              tcg_gen_brcondi_tl(TCG_COND_NE, t1, -1LL, l2);
>              gen_set_label(l1);
>              tcg_gen_movi_tl(cpu_gpr[rd], 0);
> 

No, please no.

clang is wrong in putting this under -Wall.

Paolo