Re: [Qemu-devel] [PATCH 1/2] tcg-ia64: implement mulu2_i32/i64 and muls2_i32/i64

[Richard Henderson <rth@twiddle.net>]

On 03/30/2013 08:33 AM, Aurelien Jarno wrote:
> +    if (const_arg1 && arg1 != 0) {
> +        opc2 = tcg_opc_a5(TCG_REG_P0, OPC_ADDL_A5,
> +                          TCG_REG_R2, arg1, TCG_REG_R0);
> +    } else if (is_signed) {
> +        opc2 = tcg_opc_i29(TCG_REG_P0, OPC_SXT4_I29, TCG_REG_R2, arg1);
> +    } else {
> +        opc2 = tcg_opc_i29(TCG_REG_P0, OPC_ZXT4_I29, TCG_REG_R2, arg1);
> +    }
> +    if (const_arg2 && arg2 != 0) {
> +        opc3 = tcg_opc_a5(TCG_REG_P0, OPC_ADDL_A5,
> +                          TCG_REG_R3, arg2, TCG_REG_R0);
> +    } else if (is_signed) {
> +        opc3 = tcg_opc_i29(TCG_REG_P0, OPC_SXT4_I29, TCG_REG_R3, arg2);
> +    } else {
> +        opc3 = tcg_opc_i29(TCG_REG_P0, OPC_ZXT4_I29, TCG_REG_R3, arg2);
> +    }
> +

Why the check form arg[12] != 0?  Seems like a wasted test, especially if
the optimizer is enabled and we ought never see it.

I would have said you shouldn't bother implementing the _i32 versions for
ia64, because the generic fallback would produce the same code.  But in
this case it's more about doing the job with fewer bundles.

Otherwise, the code looks correct,

Reviewed-by: Richard Henderson <rth@twiddle.net>


r~