Re: [PATCH v2 16/21] Hexagon (target/hexagon) add A4_addp_c/A4_subp_c

[Richard Henderson]

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.

> +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?

Anyway, I guess it's all sane,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~