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

[Aurelien Jarno]

On Sat, Mar 30, 2013 at 09:00:40AM -0700, Richard Henderson wrote:
> 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.

It comes from copy and paste from other code, dating from before the
optimizer pass. It looks like some code can be removed.

> 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.

Yes, the code would be basically the same, but with more bundles, so I
still think it make sense to implement it.

> Otherwise, the code looks correct,
> 
> Reviewed-by: Richard Henderson <address@hidden>

-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
address@hidden                 http://www.aurel32.net