Re: [PATCH v4 1/2] target/riscv: refactor Zicond support

[Richard Henderson <richard.henderson@linaro.org>]

On 3/6/23 07:23, Philipp Tomsich wrote:
> After the original Zicond support was stuck/fell through the cracks on
> the mailing list at v3 (and a different implementation was merged in
> the meanwhile), we need to refactor Zicond to prepare it to be reused
> by XVentanaCondOps.
> 
> This commit lifts the common logic out into gen_czero and uses this
> via gen_logic and 2 helper functions (effectively partial closures).
> 
> Signed-off-by: Philipp Tomsich <philipp.tomsich@vrull.eu>

For any multi-patch series, you should use a cover letter.
Lacking this causes the set to be missed by tooling.

> -static bool trans_czero_eqz(DisasContext *ctx, arg_czero_eqz *a)
> +/* Emits "$rd = ($rs2 <cond> $zero) ? $zero : $rs1" */
> +static inline void gen_czero(TCGv dest, TCGv src1, TCGv src2, TCGCond cond)

Drop the inline markers and let the compiler choose.

> +static inline void gen_czero_eqz(TCGv dest, TCGv src1, TCGv src2)
> +static inline void gen_czero_nez(TCGv dest, TCGv src1, TCGv src2)

These especially, where we use their function pointer...

> +    return gen_logic(ctx, a, gen_czero_eqz);
> +    return gen_logic(ctx, a, gen_czero_nez);

here, so they will most definitely exist out-of-line.

Otherwise,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~