On Thu, Jun 18, 2026 at 2:13 PM Kewen Lin <[email protected]> wrote:
>
> Hi Richi,
>
> Thanks for the review!
>
> 在 2026/6/18 19:31, Richard Biener 写道:
> > On Thu, Jun 18, 2026 at 1:18 PM Kewen Lin <[email protected]> wrote:
> >>
> >> Hi,
> >>
> >> This patch is authored-by: Zhongjie Guo <[email protected]>
> >>
> >> AVX512 vector comparisons produce mask results, but the instructions
> >> operate on vector operands. The instruction cost should therefore be
> >> based on the operand vector mode rather than the mask result mode.
> >>
> >> The mismatch is especially visible on AVX512 split-regs targets, where
> >> ix86_vec_cost scales 512-bit vector operations. Using the mask result
> >> type misses that scaling and can make 512-bit vectorization look too
> >> cheap.
> >>
> >> For mask-producing comparisons, use the operand vector type recorded on
> >> the SLP children. If no operand type is recorded, derive a related vector
> >> type from the scalar comparison type without changing the vectorizer's
> >> chosen modes. Mask-vs-mask comparisons keep their mask result type and
> >> continue to be costed as mask operations.
> >>
> >> Bootstrapped and regtested on a hygon c86-4g-m7 machine.
> >>
> >> Is it ok for trunk?
> >>
> >> BR,
> >> Kewen
> >> -----
> >>
> >> gcc/ChangeLog:
> >>
> >> * config/i386/i386.cc (ix86_vector_costs::add_stmt_cost): Use the
> >> comparison operand vector mode for mask-producing vector
> >> comparisons.
> >>
> >> gcc/testsuite/ChangeLog:
> >>
> >> * gcc.target/i386/vect-compare-cost.c: New test.
> >>
> >> Signed-off-by: Zhongjie Guo <[email protected]>
> >> ---
> >> gcc/config/i386/i386.cc | 21 +++++++++++++++++++
> >> .../gcc.target/i386/vect-compare-cost.c | 14 +++++++++++++
> >> 2 files changed, 35 insertions(+)
> >> create mode 100644 gcc/testsuite/gcc.target/i386/vect-compare-cost.c
> >>
> >> diff --git a/gcc/config/i386/i386.cc b/gcc/config/i386/i386.cc
> >> index 2945081234b..53d47596fb7 100644
> >> --- a/gcc/config/i386/i386.cc
> >> +++ b/gcc/config/i386/i386.cc
> >> @@ -26497,6 +26497,27 @@ ix86_vector_costs::add_stmt_cost (int count,
> >> vect_cost_for_stmt kind,
> >> default:
> >> if (truth_value_p (subcode))
> >> {
> >> + if (kind == vector_stmt
> >> + && vectype
> >> + && VECTOR_BOOLEAN_TYPE_P (vectype))
> >> + {
> >> + tree operand_vectype
> >> + = SLP_TREE_VECTYPE (SLP_TREE_CHILDREN (node)[0]);
> >> + if (!operand_vectype)
> >
> > when does this happen? (it should not) I'll note that truth_value_p above
> > is
>
> I think it follows the handlings in vectorizable_comparison_1, which checks
> for (!vectype) after vect_is_simple_use applied on rhs1/rhs2.
Sure, but that's before costing is invoked and this function eventually will
call vect_maybe_update_slp_op_vectype to set a vector type if none
was set.
> > worrying as it would include TRUTH_NOT_EXPR (unary). Of course only
> > tcc_comparison ever happens here so can you replace truth_value_p
> > with TREE_CODE_CLASS (subcode) == tcc_comparison?
>
> Good point! Will do.
>
> >
> >> + operand_vectype
> >> + = SLP_TREE_VECTYPE (SLP_TREE_CHILDREN (node)[1]);
> >> +
> >> + if (!operand_vectype)
> >
> > See above.
> >
> >> + if (tree cmp_type = vect_comparison_type (stmt_info))
> >> + if (!VECT_SCALAR_BOOLEAN_TYPE_P (cmp_type))
> >> + operand_vectype
> >> + = get_related_vectype_for_scalar_type
> >> + (m_vinfo->vector_mode, cmp_type);
> >> +
> >> + if (operand_vectype)
> >> + mode = TYPE_MODE (operand_vectype);
> >
> > IMO setting mode should either happen earlier and unconditional
> > (also for scalar compares) or you should set cost directly.
> >
> >> + }
> >> +
> >> if (SSE_FLOAT_MODE_SSEMATH_OR_HFBF_P (mode))
> >
> > But I agree in general that 'mode' for compares should be the mode of
> > the operands.
> > The
> >
> > else if (X87_FLOAT_MODE_P (mode))
> > /* fcmp + setcc. */
> > stmt_cost = ix86_cost->fadd + ix86_cost->add;
> >
> > path is similarly affected for scalar code, so I'd prefer a more global
> > 'mode'
> > adjustment.
>
> Do you suggest something like:
>
> diff --git a/gcc/config/i386/i386.cc b/gcc/config/i386/i386.cc
> index b1fd86c2b32..964c4bdef90 100644
> --- a/gcc/config/i386/i386.cc
> +++ b/gcc/config/i386/i386.cc
> @@ -26510,8 +26510,15 @@ ix86_vector_costs::add_stmt_cost (int count,
> vect_cost_for_stmt kind,
> break;
>
> default:
> - if (truth_value_p (subcode))
> + if (TREE_CODE_CLASS (subcode) == tcc_comparison)
> {
> + tree op_type = vect_comparison_type (stmt_info);
> + if (VECTOR_TYPE_P (vectype))
> + op_type
> + = get_related_vectype_for_scalar_type
> (m_vinfo->vector_mode,
> + op_type);
> + mode = TYPE_MODE (op_type);
More like
if (kind == vector_stmt)
op_type = SLP_TREE_VECTYPE (SLP_TREE_CHILDREN (node)[0]);
else
op_type = vect_comparison_type (stmt_info);
mode = TYPE_MODE (op_type);
> +
> if (SSE_FLOAT_MODE_SSEMATH_OR_HFBF_P (mode))
> /* CMPccS? insructions are cheap, so use sse_op. While they
> produce a mask which may need to be turned to 0/1 by and,
>
>
> ? // it's untested yet.
>
> BR,
> Kewen
>
