On Tue, Jun 24, 2025 at 4:39 AM liuhongt <hongtao....@intel.com> wrote:
>
> From: "hongtao.liu" <hongtao....@intel.com>
>
> - /* Uses in a group can share setup code, so only add setup cost once. */
> - cost -= cost.scratch;
>
> It looks like the original code took into account avoiding double
> counting, but unfortunately cost is reset inside the follow loop which
> invalidates the upper code, and makes same setup code cost duplicated in
> each use of the group.
>
> The patch fix the issue. It can also improve 548.exchange_r by 6% with
> -march=x86-64-v3 -O2 due to better ivopt on EMR.
>
> No big performance impact for SPEC2017 on graviton4/SPR with -mcpu=native
> -Ofast -fomit-framepointer -flto=auto.
>
> Bootstrapped and regtested on x86_64-pc-linux-gnu{-m32,}.
> Ok for trunk?
OK.
Is it possible to add a testcase verifying we do not double-cost? I realize
that unit-testing IVOPTs is a bit awkward ...
> gcc/ChangeLog:
>
> PR target/115842
> * tree-ssa-loop-ivopts.cc (determine_group_iv_cost_address):
> Don't recalculate inv_expr when group-candidate cost
> calucalution.
> ---
> gcc/tree-ssa-loop-ivopts.cc | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/gcc/tree-ssa-loop-ivopts.cc b/gcc/tree-ssa-loop-ivopts.cc
> index e37b24062f7..80c1955572f 100644
> --- a/gcc/tree-ssa-loop-ivopts.cc
> +++ b/gcc/tree-ssa-loop-ivopts.cc
> @@ -5016,8 +5016,6 @@ determine_group_iv_cost_address (struct ivopts_data
> *data,
> sum_cost = infinite_cost;
> }
>
> - /* Uses in a group can share setup code, so only add setup cost once. */
> - cost -= cost.scratch;
> /* Compute and add costs for rest uses of this group. */
> for (i = 1; i < group->vuses.length () && !sum_cost.infinite_cost_p ();
> i++)
> {
> @@ -5033,7 +5031,12 @@ determine_group_iv_cost_address (struct ivopts_data
> *data,
> if (!inv_exprs)
> inv_exprs = BITMAP_ALLOC (NULL);
>
> - bitmap_set_bit (inv_exprs, inv_expr->id);
> + /* Uses in a group can share setup code,
> + so only add setup cost once. */
> + if (bitmap_bit_p (inv_exprs, inv_expr->id))
> + cost -= cost.scratch;
> + else
> + bitmap_set_bit (inv_exprs, inv_expr->id);
> }
> sum_cost += cost;
> }
> --
> 2.34.1
>
