> -----Original Message-----
> From: Richard Biener <[email protected]>
> Sent: Thursday, September 3, 2026 6:52 PM
> To: [email protected]
> Cc: Liu, Hongtao <[email protected]>
> Subject: [PATCH] [x86] move CTOR gpr<->xmm move costing to add_slp_cost
> overload
> 
> The vectorizer currently works around the fact that the x86 backend in its
> add_stmt_cost hook walks all SLP node scalar defs to perform gpr<->xmm
> move costing when costing vector splat/constructor costing for invariants by
> aoviding to pass the SLP node to each part it is costing.  Say for the integer
> vector { 0, 0, 0, 0, a, b, c, d } and V4SImode vect_prologue_cost_for_slp
> creates two cost entries, one for the V4SI { 0, 0, 0, 0 } (a load) and one 
> for { a,
> b, c, d }, a vec_construct.  If node was attached to both we'd currently cost 
> the
> grp<->xmm moves for a, b, c, d twice.  The following removes this workaround
> and instead applies this costing in the add_slp_cost hook which is invoked 
> only
> once per SLP node.  The actual vec_construct cost is still handled by
> add_stmt_cost.
> 
> Bootstrapped and tested on x86_64-unknown-linux-gnu, OK for the x86
> parts?
Ok.

> 
> Thanks,
> Richard.
> 
>       * config/i386/i386.cc (ix86_vector_costs::add_stmt_cost):
>       Move gpr<->xmm move cost ...
>       (ix86_vector_costs::add_slp_cost): ... here, to new overload
>       of add_slp_cost.
>       * tree-vect-slp.cc (vect_prologue_cost_for_slp): Consistently
>       set SLP node for all cost components to make grouping for
>       the add_slp_cost hook work.
>       (add_slp_costs): Avoid calling vector_costs::add_slp_cost
>       with NULL SLP node.
> ---
>  gcc/config/i386/i386.cc | 136 +++++++++++++++++++++++-----------------
>  gcc/tree-vect-slp.cc    |  21 +++----
>  2 files changed, 86 insertions(+), 71 deletions(-)
> 
> diff --git a/gcc/config/i386/i386.cc b/gcc/config/i386/i386.cc index
> 9f2196d54de..3f00ddaa547 100644
> --- a/gcc/config/i386/i386.cc
> +++ b/gcc/config/i386/i386.cc
> @@ -26518,6 +26518,7 @@ public:
>                             stmt_vec_info stmt_info, slp_tree node,
>                             tree vectype, int misalign,
>                             vect_cost_model_location where) override;
> +  unsigned int add_slp_cost (slp_tree, const
> + array_slice<stmt_info_for_cost> &);
>    void finish_cost (const vector_costs *) override;
>    bool better_main_loop_than_p (const vector_costs *) const override;
>    bool better_epilogue_loop_than_p (const vector_costs *other, @@ -
> 27111,11 +27112,74 @@ ix86_vector_costs::add_stmt_cost (int count,
> vect_cost_for_stmt kind,
>        stmt_cost *= (GET_MODE_BITSIZE (TYPE_MODE (ls_type))
>                   / GET_MODE_BITSIZE (TYPE_MODE (ls_eltype)) + 1);
>      }
> -  else if ((kind == vec_construct || kind == scalar_to_vec)
> -        && node
> -        && SLP_TREE_DEF_TYPE (node) == vect_external_def)
> +  if (stmt_cost == -1)
> +    stmt_cost = ix86_default_vector_cost (kind, mode);
> +
> +  /* BIT_FIELD_REF <vect_**, 64, 0> with count 0 costs 0 in body.  */
> + if (kind == vec_perm && vectype && count != 0)
> +    {
> +      unsigned vec_size = GET_MODE_SIZE (TYPE_MODE (vectype));
> +      unsigned nunits = TYPE_VECTOR_SUBPARTS (vectype);
> +      unsigned *num_vec_perm = NULL;
> +
> +      if (vec_size == 32)
> +     num_vec_perm = m_num_avx256_vec_perm;
> +      else if (vec_size == 64)
> +     num_vec_perm = m_num_avx512_vec_perm;
> +
> +      if (num_vec_perm && ix86_count_cross_lane_perm_p (m_vinfo, node,
> nunits))
> +     {
> +       num_vec_perm[where] += count;
> +       if (dump_file && (dump_flags & TDF_DETAILS))
> +         {
> +           fprintf (dump_file,
> +                    "Detected avx%u cross-lane permutation: ", vec_size * 8);
> +           if (stmt_info)
> +             print_gimple_expr (dump_file, stmt_info->stmt, 0, TDF_SLIM);
> +           fprintf (dump_file, " \n");
> +         }
> +     }
> +    }
> +
> +  /* Penalize DFmode vector operations for Bonnell.  */  if
> + (TARGET_CPU_P (BONNELL) && kind == vector_stmt
> +      && vectype && GET_MODE_INNER (TYPE_MODE (vectype)) == DFmode)
> +    stmt_cost *= 5;  /* FIXME: The value here is arbitrary.  */
> +
> +  /* Statements in an inner loop relative to the loop being
> +     vectorized are weighted more heavily.  The value here is
> +     arbitrary and could potentially be improved with analysis.  */
> + retval = adjust_cost_for_freq (stmt_info, where, count * stmt_cost);
> +
> +  /* We need to multiply all vector stmt cost by 1.7 (estimated cost)
> +     for Silvermont as it has out of order integer pipeline and can execute
> +     2 scalar instruction per tick, but has in order SIMD pipeline.  */
> +  if ((TARGET_CPU_P (SILVERMONT) || TARGET_CPU_P (GOLDMONT)
> +       || TARGET_CPU_P (GOLDMONT_PLUS) || TARGET_CPU_P (INTEL))
> +      && stmt_info && stmt_info->stmt)
> +    {
> +      tree lhs_op = gimple_get_lhs (stmt_info->stmt);
> +      if (lhs_op && TREE_CODE (TREE_TYPE (lhs_op)) == INTEGER_TYPE)
> +     retval = (retval * 17) / 10;
> +    }
> +
> +  m_costs[where] += retval;
> +
> +  return retval;
> +}
> +
> +unsigned
> +ix86_vector_costs::add_slp_cost (slp_tree node,
> +                              const array_slice<stmt_info_for_cost>
> &parts) {
> +  int stmt_cost = 0;
> +
> +  /* For vector construction account for the cost of moving data between
> +     GRP and XMM.  As we are looking at the SLP nodes elements, avoid
> +     duplicate costs by doing this in add_slp_cost, leaving the actual
> +     splat/ctor cost to add_stmt_cost.  */  if (SLP_TREE_DEF_TYPE
> + (node) == vect_external_def)
>      {
> -      stmt_cost = ix86_default_vector_cost (kind, mode);
>        unsigned i;
>        tree op;
>        FOR_EACH_VEC_ELT (SLP_TREE_SCALAR_OPS (node), i, op) @@ -27153,7
> +27217,7 @@ ix86_vector_costs::add_stmt_cost (int count,
> vect_cost_for_stmt kind,
>           ;
>         else
>           {
> -           if (fp)
> +           if (FLOAT_TYPE_P (TREE_TYPE (op)))
>               {
>                 /* Scalar FP values residing in x87 registers need to be
>                    spilled and reloaded.  */
> @@ -27166,11 +27230,11 @@ ix86_vector_costs::add_stmt_cost (int count,
> vect_cost_for_stmt kind,
>                          + ix86_cost->sse_load[sse_store_index (mode2)]);
>                     stmt_cost += COSTS_N_INSNS (cost) / 2;
>                   }
> -               m_num_sse_needed[where]++;
> +               m_num_sse_needed[vect_prologue]++;
>               }
>             else
>               {
> -               m_num_gpr_needed[where]++;
> +               m_num_gpr_needed[vect_prologue]++;
> 
>                 stmt_cost += COSTS_N_INSNS (ix86_cost->integer_to_sse) /
> 2;
>               }
> @@ -27179,61 +27243,15 @@ ix86_vector_costs::add_stmt_cost (int count,
> vect_cost_for_stmt kind,
>        FOR_EACH_VEC_ELT (SLP_TREE_SCALAR_OPS (node), i, op)
>       if (TREE_CODE (op) == SSA_NAME)
>         TREE_VISITED (op) = 0;
> -    }
> -  if (stmt_cost == -1)
> -    stmt_cost = ix86_default_vector_cost (kind, mode);
> -
> -  /* BIT_FIELD_REF <vect_**, 64, 0> with count 0 costs 0 in body.  */
> -  if (kind == vec_perm && vectype && count != 0)
> -    {
> -      unsigned vec_size = GET_MODE_SIZE (TYPE_MODE (vectype));
> -      unsigned nunits = TYPE_VECTOR_SUBPARTS (vectype);
> -      unsigned *num_vec_perm = NULL;
> -
> -      if (vec_size == 32)
> -     num_vec_perm = m_num_avx256_vec_perm;
> -      else if (vec_size == 64)
> -     num_vec_perm = m_num_avx512_vec_perm;
> 
> -      if (num_vec_perm && ix86_count_cross_lane_perm_p (m_vinfo, node,
> nunits))
> -     {
> -       num_vec_perm[where] += count;
> -       if (dump_file && (dump_flags & TDF_DETAILS))
> -         {
> -           fprintf (dump_file,
> -                    "Detected avx%u cross-lane permutation: ", vec_size * 8);
> -           if (stmt_info)
> -             print_gimple_expr (dump_file, stmt_info->stmt, 0, TDF_SLIM);
> -           fprintf (dump_file, " \n");
> -         }
> -     }
> +      if (stmt_cost > 0
> +       && dump_file && (dump_flags & TDF_DETAILS))
> +     fprintf (dump_file, "node %p gpr->xmm moves costs %d in
> prologue\n",
> +              (void *)node, stmt_cost);
> +      m_costs[vect_prologue] += stmt_cost;
>      }
> 
> -  /* Penalize DFmode vector operations for Bonnell.  */
> -  if (TARGET_CPU_P (BONNELL) && kind == vector_stmt
> -      && vectype && GET_MODE_INNER (TYPE_MODE (vectype)) == DFmode)
> -    stmt_cost *= 5;  /* FIXME: The value here is arbitrary.  */
> -
> -  /* Statements in an inner loop relative to the loop being
> -     vectorized are weighted more heavily.  The value here is
> -     arbitrary and could potentially be improved with analysis.  */
> -  retval = adjust_cost_for_freq (stmt_info, where, count * stmt_cost);
> -
> -  /* We need to multiply all vector stmt cost by 1.7 (estimated cost)
> -     for Silvermont as it has out of order integer pipeline and can execute
> -     2 scalar instruction per tick, but has in order SIMD pipeline.  */
> -  if ((TARGET_CPU_P (SILVERMONT) || TARGET_CPU_P (GOLDMONT)
> -       || TARGET_CPU_P (GOLDMONT_PLUS) || TARGET_CPU_P (INTEL))
> -      && stmt_info && stmt_info->stmt)
> -    {
> -      tree lhs_op = gimple_get_lhs (stmt_info->stmt);
> -      if (lhs_op && TREE_CODE (TREE_TYPE (lhs_op)) == INTEGER_TYPE)
> -     retval = (retval * 17) / 10;
> -    }
> -
> -  m_costs[where] += retval;
> -
> -  return retval;
> +  return stmt_cost + vector_costs::add_slp_cost (node, parts);
>  }
> 
>  void
> diff --git a/gcc/tree-vect-slp.cc b/gcc/tree-vect-slp.cc index
> 96e55cce337..665a433a48f 100644
> --- a/gcc/tree-vect-slp.cc
> +++ b/gcc/tree-vect-slp.cc
> @@ -9005,7 +9005,6 @@ vect_prologue_cost_for_slp (slp_tree node,
> unsigned nvectors,
>      }
>    /* ???  We're just tracking whether vectors in a single node are the same.
>       Ideally we'd do something more global.  */
> -  bool passed = false;
>    for (unsigned int start : starts)
>      {
>        vect_cost_for_stmt kind;
> @@ -9015,15 +9014,8 @@ vect_prologue_cost_for_slp (slp_tree node,
> unsigned nvectors,
>       kind = scalar_to_vec;
>        else
>       kind = vec_construct;
> -      /* The target cost hook has no idea which part of the SLP node
> -      we are costing so avoid passing it down more than once.  Pass
> -      it to the first vec_construct or scalar_to_vec part since for those
> -      the x86 backend tries to account for GPR to XMM register moves.  */
> -      record_stmt_cost (cost_vec, 1, kind, nullptr,
> -                     (kind != vector_load && !passed) ? node : nullptr,
> +      record_stmt_cost (cost_vec, 1, kind, nullptr, node,
>                       vectype, 0, vect_prologue);
> -      if (kind != vector_load)
> -     passed = true;
>      }
>  }
> 
> @@ -9397,9 +9389,14 @@ add_slp_costs (vector_costs *costs,
> stmt_vector_for_cost& cost_vec)
>        while (end < cost_vec.length ()
>            && cost_vec[start].node == cost_vec[end].node)
>       end++;
> -      costs->add_slp_cost (cost_vec[start].node,
> -                        array_slice<stmt_info_for_cost>
> -                          (cost_vec.begin () + start, end - start));
> +      if (cost_vec[start].node)
> +     costs->add_slp_cost (cost_vec[start].node,
> +                          array_slice<stmt_info_for_cost>
> +                            (cost_vec.begin () + start, end - start));
> +      else
> +     costs->vector_costs::add_slp_cost (cost_vec[start].node,
> +                          array_slice<stmt_info_for_cost>
> +                            (cost_vec.begin () + start, end - start));
>        start = end;
>      }
>  }
> --
> 2.51.0

Reply via email to