Re: [PATCH 1/2] Add support for IVOPT

[Kugan Vivekanandarajah <kugan dot vivekanandarajah at linaro dot org>]

Hi Richard,

On Thu, 16 May 2019 at 21:14, Richard Biener <richard.guenther@gmail.com> wrote:
>
> On Wed, May 15, 2019 at 4:40 AM <kugan.vivekanandarajah@linaro.org> wrote:
> >
> > From: Kugan Vivekanandarajah <kugan.vivekanandarajah@linaro.org>
> >
> > gcc/ChangeLog:
> >
> > 2019-05-15  Kugan Vivekanandarajah  <kugan.vivekanandarajah@linaro.org>
> >
> >         PR target/88834
> >         * tree-ssa-loop-ivopts.c (get_mem_type_for_internal_fn): Handle
> >         IFN_MASK_LOAD_LANES and IFN_MASK_STORE_LANES.
> >         (find_interesting_uses_stmt): Likewise.
> >         (get_alias_ptr_type_for_ptr_address): Likewise.
> >         (add_iv_candidate_for_use): Add scaled index candidate if useful.
> >
> > Change-Id: I8e8151fe2dde2845dedf38b090103694da6fc9d1
> > ---
> >  gcc/tree-ssa-loop-ivopts.c | 60 +++++++++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 59 insertions(+), 1 deletion(-)
> >
> > diff --git a/gcc/tree-ssa-loop-ivopts.c b/gcc/tree-ssa-loop-ivopts.c
> > index 9864b59..115a70c 100644
> > --- a/gcc/tree-ssa-loop-ivopts.c
> > +++ b/gcc/tree-ssa-loop-ivopts.c
> > @@ -2451,11 +2451,13 @@ get_mem_type_for_internal_fn (gcall *call, tree *op_p)
> >    switch (gimple_call_internal_fn (call))
> >      {
> >      case IFN_MASK_LOAD:
> > +    case IFN_MASK_LOAD_LANES:
> >        if (op_p == gimple_call_arg_ptr (call, 0))
> >         return TREE_TYPE (gimple_call_lhs (call));
> >        return NULL_TREE;
> >
> >      case IFN_MASK_STORE:
> > +    case IFN_MASK_STORE_LANES:
> >        if (op_p == gimple_call_arg_ptr (call, 0))
> >         return TREE_TYPE (gimple_call_arg (call, 3));
> >        return NULL_TREE;
> > @@ -2545,7 +2547,7 @@ find_interesting_uses_stmt (struct ivopts_data *data, gimple *stmt)
> >           return;
> >         }
> >
> > -      /* TODO -- we should also handle address uses of type
> > +      /* TODO -- we should also handle all address uses of type
> >
> >          memory = call (whatever);
> >
> > @@ -2553,6 +2555,27 @@ find_interesting_uses_stmt (struct ivopts_data *data, gimple *stmt)
> >
> >          call (memory).  */
> >      }
> > +  else if (is_gimple_call (stmt))
> > +    {
> > +      gcall *call = dyn_cast <gcall *> (stmt);
> > +      if (call
>
> that's testing things twice, just do
>
>        else if (gcall *call = dyn_cast <gcall *> (stmt))
>          {
> ...
>
> no other comments besides why do you need _LANES handling here where
> the w/o _LANES handling didn't need anything.
Right,  I have now changed this in the revised patch.

Thanks,
Kugan

>
> > +         && gimple_call_internal_p (call)
> > +         && (gimple_call_internal_fn (call) == IFN_MASK_LOAD_LANES
> > +             || gimple_call_internal_fn (call) == IFN_MASK_STORE_LANES))
> > +       {
> > +         tree *arg = gimple_call_arg_ptr (call, 0);
> > +         struct iv *civ = get_iv (data, *arg);
> > +         tree mem_type = get_mem_type_for_internal_fn (call, arg);
> > +         if (civ && mem_type)
> > +           {
> > +             civ = alloc_iv (data, civ->base, civ->step);
> > +             record_group_use (data, arg, civ, stmt, USE_PTR_ADDRESS,
> > +                               mem_type);
> > +             return;
> > +           }
> > +       }
> > +    }
> > +
> >
> >    if (gimple_code (stmt) == GIMPLE_PHI
> >        && gimple_bb (stmt) == data->current_loop->header)
> > @@ -3500,6 +3523,39 @@ add_iv_candidate_for_use (struct ivopts_data *data, struct iv_use *use)
> >      basetype = sizetype;
> >    record_common_cand (data, build_int_cst (basetype, 0), iv->step, use);
> >
> > +  /* Compare the cost of an address with an unscaled index with the cost of
> > +    an address with a scaled index and add candidate if useful. */
> > +  if (use != NULL && use->type == USE_PTR_ADDRESS)
> > +    {
> > +      struct mem_address parts = {NULL_TREE, integer_one_node,
> > +                                 NULL_TREE, NULL_TREE, NULL_TREE};
> > +      poly_uint64 temp;
> > +      poly_int64 fact;
> > +      bool speed = optimize_loop_for_speed_p (data->current_loop);
> > +      poly_int64 poly_step = tree_to_poly_int64 (iv->step);
> > +      machine_mode mem_mode = TYPE_MODE (use->mem_type);
> > +      addr_space_t as = TYPE_ADDR_SPACE (TREE_TYPE (use->iv->base));
> > +
> > +      fact = GET_MODE_SIZE (GET_MODE_INNER (TYPE_MODE (use->mem_type)));
> > +      parts.index = integer_one_node;
> > +
> > +      if (fact.is_constant ()
> > +         && can_div_trunc_p (poly_step, fact, &temp))
> > +       {
> > +         /* Addressing mode "base + index".  */
> > +         rtx addr = addr_for_mem_ref (&parts, as, false);
> > +         unsigned cost = address_cost (addr, mem_mode, as, speed);
> > +         tree step = wide_int_to_tree (sizetype,
> > +                                       exact_div (poly_step, fact));
> > +         parts.step = wide_int_to_tree (sizetype, fact);
> > +         /* Addressing mode "base + index << scale".  */
> > +         addr = addr_for_mem_ref (&parts, as, false);
> > +         unsigned new_cost = address_cost (addr, mem_mode, as, speed);
> > +         if (new_cost < cost)
> > +           add_candidate (data, size_int (0), step, true, NULL);
> > +       }
> > +    }
> > +
> >    /* Record common candidate with constant offset stripped in base.
> >       Like the use itself, we also add candidate directly for it.  */
> >    base = strip_offset (iv->base, &offset);
> > @@ -7112,6 +7168,8 @@ get_alias_ptr_type_for_ptr_address (iv_use *use)
> >      {
> >      case IFN_MASK_LOAD:
> >      case IFN_MASK_STORE:
> > +    case IFN_MASK_LOAD_LANES:
> > +    case IFN_MASK_STORE_LANES:
> >        /* The second argument contains the correct alias type.  */
> >        gcc_assert (use->op_p = gimple_call_arg_ptr (call, 0));
> >        return TREE_TYPE (gimple_call_arg (call, 1));
> > --
> > 2.7.4
> >