Re: [PATCH v3 08/14] tcg/riscv: Implement vector cmp ops

[Richard Henderson]

On 9/4/24 07:27, LIU Zhiwei wrote:
@@ -2322,6 +2411,51 @@ static void tcg_out_vec_op(TCGContext *s, TCGOpcode opc,
          riscv_set_vec_config_vl(s, type);
          tcg_out_opc_vi(s, OPC_VXOR_VI, a0, a1, -1, true);
          break;
+    case INDEX_op_cmpsel_vec:
+        TCGArg a3, a4;
+        int c3, c4;
+        TCGCond cond;

While I suppose this compiles, it's not great to have new variables added randomly within 
a switch.  At minimum, add { } around the block, but consider breaking out a separate 
tcg_out_cmpsel function, akin to tcg_out_movcond et al.

@@ -2332,10 +2466,27 @@ static void tcg_out_vec_op(TCGContext *s, TCGOpcode opc,
  void tcg_expand_vec_op(TCGOpcode opc, TCGType type, unsigned vece,
                         TCGArg a0, ...)
  {
+    va_list va;
+    TCGv_vec v0, v1;
+    TCGArg a2, a3;
+
+    va_start(va, a0);
+    v0 = temp_tcgv_vec(arg_temp(a0));
+    v1 = temp_tcgv_vec(arg_temp(va_arg(va, TCGArg)));
+    a2 = va_arg(va, TCGArg);
+
      switch (opc) {
+    case INDEX_op_cmp_vec:
+        a3 = va_arg(va, TCGArg);
+        vec_gen_6(INDEX_op_cmpsel_vec, type, vece, tcgv_vec_arg(v0),
+                    tcgv_vec_arg(v1), a2,
+                    tcgv_i64_arg(tcg_constant_i64(-1)),
+                    tcgv_i64_arg(tcg_constant_i64(0)), a3);
+        break;
      default:
          g_assert_not_reached();
      }
+    va_end(va);
  }

Better to use "TCGArg a0, a1".  Converting through arg_tmp + temp_tcgv_vec to v0/v1 and 
then undoing that with tcgv_vec_arg is confusing.


r~