Thanks for the cleanup, the new interface is really much simpler than before!
Only few minor comment, you can go ahead to commit that after address those comment. (OK, I don't want to review whole patch again, it's really huge change :P > > - void set_rounding_mode (enum floating_point_rounding_mode mode) > + void check_insn_flags () void check_insn_flags () const > void emit_insn (enum insn_code icode, rtx *ops) > { > int opno = 0; > + int num_ops = 0; > /* It's true if any operand is memory operand. */ > bool any_mem_p = false; > - /* It's true if all operands are mask operand. */ > - bool all_mask_p = true; > - if (m_has_dest_p) > + > + /* Add dest operand. */ > + if (m_insn_flags & HAS_DEST_P) > { > any_mem_p |= MEM_P (ops[opno]); > - all_mask_p &= GET_MODE_CLASS (GET_MODE (ops[opno])) == > MODE_VECTOR_BOOL; > add_output_operand (ops[opno++], m_dest_mode); > + num_ops += 1; Drop this > } > > - if (m_fully_unmasked_p) > + /* Add mask operand. */ > + if (m_insn_flags & USE_ONE_TRUE_MASK_P) > + add_input_operand (gen_scalar_move_mask (m_mask_mode), m_mask_mode); > + else if (m_insn_flags & USE_ALL_TRUES_MASK_P) > add_all_one_mask_operand (); > + else if (m_insn_flags & HAS_MASK_P) > + { > + machine_mode mode = insn_data[(int) icode].operand[m_opno].mode; > + gcc_assert (mode != VOIDmode); > + add_input_operand (ops[opno++], mode); > + num_ops += 1; > + } Drop this > - if (!m_use_real_merge_p) > + /* Add merge operand. */ > + if (m_insn_flags & USE_VUNDEF_MERGE_P) > add_vundef_operand (); > + else if (m_insn_flags & HAS_MERGE_P) > + { > + machine_mode mode = insn_data[(int) icode].operand[m_opno].mode; > + gcc_assert (mode != VOIDmode); > + add_input_operand (ops[opno++], mode); > + num_ops += 1; > + } > + > + if (m_insn_flags & NULLARY_OP_P) > + num_ops += 0; > + else if (m_insn_flags & UNARY_OP_P) > + num_ops += 1; > + else if (m_insn_flags & BINARY_OP_P) > + num_ops += 2; > + else if (m_insn_flags & TERNARY_OP_P) > + num_ops += 3; num_ops = rather than += here. > + else > + gcc_unreachable (); > > - for (; opno < m_op_num; opno++) > + /* Add the remain operands. */ > + for (; opno < num_ops; opno++) for (;num_ops;num_ops--, opno++) > { > any_mem_p |= MEM_P (ops[opno]); > - all_mask_p &= GET_MODE_CLASS (GET_MODE (ops[opno])) == > MODE_VECTOR_BOOL; > machine_mode mode = insn_data[(int) icode].operand[m_opno].mode; > /* 'create_input_operand doesn't allow VOIDmode. > According to vector.md, we may have some patterns that do not have > @@ -194,46 +251,51 @@ public: > add_input_operand (ops[opno], mode); > } > > - if (m_needs_avl_p) > + /* Add vl operand. */ > + rtx len = m_vl_op; > + machine_mode mode = VECTOR_MODE_P (m_dest_mode) ? m_dest_mode : > m_mask_mode; > + if (m_vlmax_p) > { > - rtx len = m_vl_op; > - machine_mode mode > - = VECTOR_MODE_P (m_dest_mode) ? m_dest_mode : m_mask_mode; > - if (m_vlmax_p) > + if (riscv_v_ext_vls_mode_p (mode)) > + { > + /* VLS modes always set VSETVL by > + "vsetvl zero, rs1/imm". */ > + poly_uint64 nunits = GET_MODE_NUNITS (mode); > + len = gen_int_mode (nunits, Pmode); > + if (!satisfies_constraint_K (len)) > + len = force_reg (Pmode, len); > + m_vlmax_p = false; > + } > + else if (const_vlmax_p (mode)) > + { > + /* Optimize VLS-VLMAX code gen, we can use vsetivli instead of > + the vsetvli to obtain the value of vlmax. */ > + poly_uint64 nunits = GET_MODE_NUNITS (mode); > + len = gen_int_mode (nunits, Pmode); > + m_vlmax_p = false; > + } > + else if (can_create_pseudo_p ()) > { > - if (riscv_v_ext_vls_mode_p (mode)) > - { > - /* VLS modes always set VSETVL by > - "vsetvl zero, rs1/imm". */ > - poly_uint64 nunits = GET_MODE_NUNITS (mode); > - len = gen_int_mode (nunits, Pmode); > - if (!satisfies_constraint_K (len)) > - len = force_reg (Pmode, len); > - m_vlmax_p = false; /* It has became NONVLMAX now. */ > - } > - else if (const_vlmax_p (mode)) > - { > - /* Optimize VLS-VLMAX code gen, we can use vsetivli instead of > - the vsetvli to obtain the value of vlmax. */ > - poly_uint64 nunits = GET_MODE_NUNITS (mode); > - len = gen_int_mode (nunits, Pmode); > - m_vlmax_p = false; /* It has became NONVLMAX now. */ > - } > - else if (can_create_pseudo_p ()) > - { > - len = gen_reg_rtx (Pmode); > - emit_vlmax_vsetvl (mode, len); > - } > + len = gen_reg_rtx (Pmode); > + emit_vlmax_vsetvl (mode, len); > } > - add_input_operand (len, Pmode); > + else > + gcc_assert (len != NULL_RTX); I don't see the gcc_assert here before, and seems like that should move to another place > } > + else > + /* Must specify vl when not vlmax. */ > + gcc_assert (len != NULL_RTX); Like here and drop the else since we need this assert no matter m_vlmax_p true or false diff --git a/gcc/config/riscv/riscv-v.cc b/gcc/config/riscv/riscv-v.cc index 3e7caf48742..64e22177e81 100644 --- a/gcc/config/riscv/riscv-v.cc +++ b/gcc/config/riscv/riscv-v.cc @@ -279,12 +279,8 @@ public: len = gen_reg_rtx (Pmode); emit_vlmax_vsetvl (mode, len); } - else - gcc_assert (len != NULL_RTX); } - else - /* Must specify vl when not vlmax. */ - gcc_assert (len != NULL_RTX); + gcc_assert (len != NULL_RTX); add_input_operand (len, Pmode); /* Add tail and mask policy operands. */ > @@ -1212,7 +834,7 @@ emit_vlmax_masked_gather_mu_insn (rtx target, rtx op, > rtx sel, rtx mask) > p q r s t u v w # v11 destination register > e d c b a # v1 source vector > 1 0 0 1 1 1 0 1 # v0 mask vector > - > +make Ha, this must be a mistake, right? :P