On 4/17/23 12:36, Michael Collison wrote:
2023-03-02  Michael Collison  <colli...@rivosinc.com>
            Juzhe Zhong  <juzhe.zh...@rivai.ai>

        * config/riscv/riscv.md (riscv_vector_preferred_simd_mode): Include
        vector-iterators.md.
        * config/riscv/vector-auto.md: New file containing
        autovectorization patterns.
        * config/riscv/vector-iterators.md (UNSPEC_VADD/UNSPEC_VSUB):
        New unspecs for autovectorization patterns.
        * config/riscv/vector.md: Remove include of vector-iterators.md
        and include vector-auto.md.
So the basic idea here appears to be to have a define_expand with the well known names (for the optab interface) generate RTL that is subsequently matched by the intrinsics that Juzhe has already defined and integrated.

That seems like a reasonable model to start with and get the basic functionality in place. I'm all for focusing on that basic functionality first.


diff --git a/gcc/config/riscv/vector-auto.md b/gcc/config/riscv/vector-auto.md
new file mode 100644
index 00000000000..dc62f9af705
--- /dev/null
+++ b/gcc/config/riscv/vector-auto.md
So basically vector-auto.md provides the interface to utilize the builtins found in vector.md. Given the size of vector.md I can certainly see the desire to separate that out.


+
+
+;; -------------------------------------------------------------------------
+;; ---- [INT] Addition
Just a note. This patch actually wires up plus, minus, and, ior, xor, ashift, ashiftrt and lshiftrt. So it's quite a bit more than just addition. So updating the comments is probably warranted.


+;; -------------------------------------------------------------------------
+;; Includes:
+;; - vadd.vv
+;; - vadd.vx
+;; - vadd.vi
+;; -------------------------------------------------------------------------
+
+(define_expand "<optab><mode>3"
+  [(set (match_operand:VI 0 "register_operand")
+       (any_int_binop:VI (match_operand:VI 1 "register_operand")
+                         (match_operand:VI 2 "register_operand")))]
+  "TARGET_VECTOR"
+{
+  using namespace riscv_vector;
+
+  rtx merge = RVV_VUNDEF (<MODE>mode);
+  rtx vl = gen_reg_rtx (Pmode);
+  emit_vlmax_vsetvl (<MODE>mode, vl);
+  rtx mask_policy = get_mask_policy_no_pred();
+  rtx tail_policy = get_tail_policy_no_pred();
+  rtx mask = CONSTM1_RTX(<VM>mode);
+  rtx vlmax_avl_p = get_avl_type_rtx(NONVLMAX);
+
+  emit_insn(gen_pred_<optab><mode>(operands[0], mask, merge, operands[1], 
operands[2],
+                               vl, tail_policy, mask_policy, vlmax_avl_p));
Just nits. Make sure to put a space before the open paren of an argument list, even when the argument list is empty. Similarly for the other expander in here. And update the comment. You may not want to list every instruction handled by the expander. Your call, though clearly if you're going to include them, the list ought to be reasonably complete.

No objections to this code. It obviously depends on some bits earlier in the patchset which I still need to look at, but I wanted to look at this one first as it shows the basic formula for how to wire up the basic vector patterns.

Please wait for the prereqs to get reviewed before installing on the trunk.

jeff

Reply via email to