> -----Original Message-----
> From: Richard Henderson <richard.hender...@linaro.org>
> Sent: Tuesday, April 6, 2021 4:12 PM
> To: Taylor Simpson <tsimp...@quicinc.com>; qemu-devel@nongnu.org
> Cc: phi...@redhat.com; a...@rev.ng; Brian Cain <bc...@quicinc.com>
> Subject: Re: [PATCH v2 16/21] Hexagon (target/hexagon) add
> A4_addp_c/A4_subp_c
>
> On 3/31/21 8:53 PM, Taylor Simpson wrote:
> > +#define fGEN_TCG_A4_addp_c(SHORTCODE) \
> > + do { \
> > + TCGv_i64 carry = tcg_temp_new_i64(); \
> > + TCGv_i64 zero = tcg_const_i64(0); \
> > + tcg_gen_extu_i32_i64(carry, PxV); \
> > + tcg_gen_andi_i64(carry, carry, 1); \
> > + tcg_gen_add2_i64(RddV, carry, RssV, zero, carry, zero); \
> > + tcg_gen_add2_i64(RddV, carry, RddV, carry, RttV, zero); \
> > + tcg_gen_extrl_i64_i32(PxV, carry); \
>
> Note that this is already a single bit, always.
Yes, we just need an _i32 instead of an _i64.
> > +static TCGv gen_8bitsof(TCGv result, TCGv value)
> > +{
> > + TCGv zero = tcg_const_tl(0);
> > + TCGv ones = tcg_const_tl(0xff);
> > + tcg_gen_movcond_tl(TCG_COND_NE, result, value, zero, ones, zero);
> > + tcg_temp_free(zero);
> > + tcg_temp_free(ones);
> > +
> > + return result;
>
> There's little point in a conditional move.
> Just multiply by 0xff.
>
> Unless you had another non-boolean use for gen_8bitsof?
Yes, there are instances of that.
> Anyway, I guess it's all sane,
> Reviewed-by: Richard Henderson <richard.hender...@linaro.org>
Thanks!
