Re: [PATCH] tcg/loongarch64: Support every TCGCond for cmp_vec ops

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

On 12/6/25 12:24, WANG Xuerui wrote:
> From: WANG Xuerui <git@xen0n.name>
> 
> Support for TCGCond's in loongarch64 cmp_vec codegen is not uniform: NE
> is not supported at all and will trip over assertions, and legalization
> (currently just operand-swapping) is not done for reg-imm comparisons.
> Since the TCG middle-end will not legalize the comparison conditions for
> us, we have to do it ourselves like other targets.
> 
> Because EQ/LT/LTU/LE/LEU are natively supported, we only have to keep
> the current operand swapping treatment for GT/GTU/GE/GEU but ensure it
> is done for both reg-reg and reg-imm cases, and use a bitwise NOT to
> help legalize NE.
> 
> While at it, lift the cmp_vec handling to own function to make it easier
> for readers.
> 
> Fixes: d8b6fa593d2d ("tcg/loongarch64: Lower cmp_vec to vseq/vsle/vslt")
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3237
> Cc: Richard Henderson <richard.henderson@linaro.org>
> Cc: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reported-by: xtex <xtex@aosc.io>
> Signed-off-by: WANG Xuerui <git@xen0n.name>
> ---
>   tcg/loongarch64/tcg-target.c.inc | 119 +++++++++++++++++++------------
>   1 file changed, 75 insertions(+), 44 deletions(-)
> 
> diff --git a/tcg/loongarch64/tcg-target.c.inc b/tcg/loongarch64/tcg-target.c.inc
> index 10c69211ac5..1a243a57beb 100644
> --- a/tcg/loongarch64/tcg-target.c.inc
> +++ b/tcg/loongarch64/tcg-target.c.inc
> @@ -2179,15 +2179,38 @@ static void tcg_out_addsub_vec(TCGContext *s, bool lasx, unsigned vece,
>       tcg_out32(s, encode_vdvjvk_insn(insn, a0, a1, a2));
>   }
>   
> -static void tcg_out_vec_op(TCGContext *s, TCGOpcode opc,
> -                           unsigned vecl, unsigned vece,
> -                           const TCGArg args[TCG_MAX_OP_ARGS],
> -                           const int const_args[TCG_MAX_OP_ARGS])
> +static void tcg_out_cmp_vec(TCGContext *s, bool lasx, unsigned vece,
> +                            TCGArg a0, TCGArg a1, TCGArg a2,
> +                            bool a2_is_const, TCGCond cond)
>   {
> -    TCGType type = vecl + TCG_TYPE_V64;
> -    bool lasx = type == TCG_TYPE_V256;
> -    TCGArg a0, a1, a2, a3;
>       LoongArchInsn insn;
> +    bool need_invert = false;
> +
> +    switch (cond) {
> +    case TCG_COND_EQ:
> +    case TCG_COND_LE:
> +    case TCG_COND_LEU:
> +    case TCG_COND_LT:
> +    case TCG_COND_LTU:
> +        /* These are directly expressible. */
> +        break;
> +    case TCG_COND_NE:
> +        need_invert = true;
> +        cond = TCG_COND_EQ;
> +        break;
> +    case TCG_COND_GE:
> +    case TCG_COND_GEU:
> +    case TCG_COND_GT:
> +    case TCG_COND_GTU:
> +        {
> +            TCGArg t;
> +            t = a1, a1 = a2, a2 = t;
> +            cond = tcg_swap_cond(cond);
> +            break;
> +        }

You can't just swap if a2_is_const.

You can get better results by using tcg_expand_vec_op() to transform the comparison early.

You probably want to do something like s390x, where you have a helper that transforms the 
comparision but without the inversion for NE, and then applies the inversion for 
INDEX_op_cmp_vec and swaps operands for INDEX_op_cmpsel_vec.


r~