Hi Robin,

I have posted a V2 patch to implement the method I mentioned. I wonder if that makes it a little easier to understand?

V2 patch: https://gcc.gnu.org/pipermail/gcc-patches/2023-September/630985.html

On 2023/9/20 12:02, Lehua Ding wrote:


On 2023/9/20 6:02, Robin Dapp wrote:
Hi Lehua,

thanks for the explanation.

My current method is still to keep the operand 2 of vcond_mask as a
register, but the pattern of mov_vec_const_0 is simplified, so that
the corresponding combine pattern can be more simple. That's the only
reason I split the vcond_mask into three patterns.

My "problem" with the separate split it that it really sticks out
and everybody seeing it would wonder why we need it.  It's not that
bad of course but it appears as if we messed up somewhere else.

Can not agree more.

I checked and I don't see additional FAILs with the vmask pattern
that additionally allows a const0 operand (that is forced into a register)
and a force_reg in abs:VF.

Would you mind re-checking if we can avoid the extra
"vec_duplicate_const_0" by changing the other affected patterns
in a similar manner?  I really didn't verify in-depth so if we needed
to add a force_reg to every pattern we might need to reconsider.
Still, I'd be unsure if I preferred the "vec_dup_const_0" over
additional force_regs ;)

I think that's a little weird, too. I prefer to add a single define_insn_and_split move_const_0 pattern like following diff, How do you feel about that?

diff --git a/gcc/config/riscv/riscv-v.cc b/gcc/config/riscv/riscv-v.cc
index f4dab9fceb8..c0ab96ae8ab 100644
--- a/gcc/config/riscv/riscv-v.cc
+++ b/gcc/config/riscv/riscv-v.cc
@@ -973,7 +973,11 @@ expand_const_vector (rtx target, rtx src)
       rtx tmp = register_operand (target, mode) ? target : gen_reg_rtx (mode);
        /* Element in range -16 ~ 15 integer or 0.0 floating-point,
       we use vmv.v.i instruction.  */
-      if (satisfies_constraint_vi (src) || satisfies_constraint_Wc0 (src))
+      /* For const int or float 0, we keep the simple pattern before split1
+     pass. */
+      if (can_create_pseudo_p () && satisfies_constraint_Wc0 (src))
+    emit_insn (gen_mov_vec_const_0 (mode, tmp, src));
+      else if (satisfies_constraint_vi (src))
      {
        rtx ops[] = {tmp, src};
        emit_vlmax_insn (code_for_pred_mov (mode), UNARY_OP, ops);

diff --git a/gcc/config/riscv/vector.md b/gcc/config/riscv/vector.md
index f66ffebba24..b4973125d04 100644
--- a/gcc/config/riscv/vector.md
+++ b/gcc/config/riscv/vector.md
@@ -1640,6 +1640,19 @@
    "TARGET_VECTOR"
    {})

+(define_insn_and_split "@mov_vec_const_0<mode>"
+  [(set (match_operand:V_VLS 0 "register_operand")
+        (match_operand:V_VLS 1 "vector_const_0_operand"))]
+  "TARGET_VECTOR && can_create_pseudo_p ()"
+  "#"
+  "&& 1"
+  [(const_int 0)]
+  {
+    emit_move_insn (operands[0], operands[1]);
+    DONE;
+  }
+  [(set_attr "type" "vector")])
+
  ;; vle.v/vse.v,vmv.v.v
  (define_insn_and_split "*pred_mov<mode>"
   [(set (match_operand:V_VLS 0 "nonimmediate_operand"            "=vr,    vr,    vd,     m,    vr,    vr")


--
Best,
Lehua

Reply via email to