> -----Original Message-----
> From: Robin Dapp <[email protected]>
> Sent: 2024年7月17日 22:43
> To: Demin Han <[email protected]>; [email protected]
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]
> Subject: Re: [PATCH] RISC-V: More support of vx and vf for autovec
> comparison
>
> Hi Demin,
>
> > + void add_integer_operand (rtx x)
> > + {
> > + create_integer_operand (&m_ops[m_opno++], INTVAL (x));
> > + gcc_assert (m_opno <= MAX_OPERANDS); }
>
> Can that be folded into add_input_operand somehow?
It's really redundant now. Firstly I want to follow rvv intrinsic.
> > void add_input_operand (rtx x, machine_mode mode)
> > {
> > create_input_operand (&m_ops[m_opno++], x, mode); @@ -284,12
> > +289,13 @@ public:
> > for (; num_ops; num_ops--, opno++)
> > {
> > any_mem_p |= MEM_P (ops[opno]);
> > - machine_mode mode = insn_data[(int) icode].operand[m_opno].mode;
> > + machine_mode orig_mode = insn_data[(int)
> icode].operand[m_opno].mode;
> > + machine_mode mode = orig_mode;
> > /* 'create_input_operand doesn't allow VOIDmode.
> > According to vector.md, we may have some patterns that do not have
> > explicit machine mode specifying the operand. Such operands are
> > always Pmode. */
> > - if (mode == VOIDmode)
> > + if (orig_mode == VOIDmode)
> > mode = Pmode;
>
> Maybe source_mode and dest_mode would be a bit clearer.
Without add_integer_operand, this change can be removed.
> > - add_input_operand (ops[opno], mode);
> > + if (CONST_INT_P (ops[opno]) && orig_mode != E_VOIDmode)
> > + add_integer_operand (ops[opno]);
> > + else
> > + add_input_operand (ops[opno], mode);
>
> Indents look odd from here. Could you double-check with clang-format?
There are two additional spaces.
> > - icode = code_for_pred_cmp (mode);
> > + icode = !scalar_p ? code_for_pred_cmp (mode)
> > + icode = code_for_pred_cmp_scalar (mode);
>
> Ditto.
This is ok.
> > diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> > index 19b9b2daa95..ad5668b2c5a 100644
> > --- a/gcc/config/riscv/riscv.cc
> > +++ b/gcc/config/riscv/riscv.cc
> > @@ -2140,7 +2140,7 @@ riscv_const_insns (rtx x)
> > register vec_duplicate into vmv.v.x. */
> > scalar_mode smode = GET_MODE_INNER (GET_MODE (x));
> > if (maybe_gt (GET_MODE_SIZE (smode), UNITS_PER_WORD)
> > - && !immediate_operand (elt, Pmode))
> > + && !FLOAT_MODE_P (smode) && !immediate_operand (elt,
> Pmode))
>
> FLOAT_MODE is a bit broad here. Maybe rather add a case before all others
> that always allows zero constants for any mode (as well as a comment)?
>
> > - if (maybe_gt (GET_MODE_SIZE (<VEL>mode), GET_MODE_SIZE
> (Pmode)))
> > + bool gt_p = maybe_gt (GET_MODE_SIZE (<VEL>mode),
> GET_MODE_SIZE (Pmode));
> > + if (!FLOAT_MODE_P (<VEL>mode) && gt_p)
> > {
> > riscv_vector::emit_vlmax_insn (code_for_pred_broadcast
> (<MODE>mode),
> > riscv_vector::UNARY_OP, operands);
>
> Same here basically. Isn't it just the zero constant?
Integer mode need special process under RV32.
I think this additional constrain is not harmful.
Once double float operand enter these two branches and expanded to broadcast,
then last_conbine can't work.
After the processing in expand_vec_cmp, this change actually don't affect cmp
operation.
I will split this patch.
> > diff --git
> > a/gcc/testsuite/gcc.target/riscv/rvv/autovec/binop/vadd-rv32gcv-nofm.c
> > b/gcc/testsuite/gcc.target/riscv/rvv/autovec/binop/vadd-rv32gcv-nofm.c
> > index db8c653b179..b9a040f2f78 100644
>
> I suppose the -rv64 tests also need adjustment?
RV64 is OK, gt_p is false.
> Regards
> Robin
Regards,
Demin
