Re: PR81635: Use chrecs to help find related data refs

[Richard Sandiford <richard dot sandiford at linaro dot org>]

Richard Biener <richard.guenther@gmail.com> writes:
> @@ -787,14 +821,14 @@ canonicalize_base_object_address (tree a
>
>  bool
>  dr_analyze_innermost (innermost_loop_behavior *drb, tree ref,
> -                     struct loop *loop)
> +                     gimple *stmt, struct loop *loop)
>  {
>
> please amend the function comment with what STMT is about
> (DR_STMT I suppose).

I'd done that with:

------
@@ -765,7 +778,28 @@ canonicalize_base_object_address (tree a
   return build_fold_addr_expr (TREE_OPERAND (addr, 0));
 }
 
-/* Analyze the behavior of memory reference REF.  There are two modes:
[...]
+/* Analyze the behavior of memory reference REF, which occurs in STMT.
+   There are two modes:
 
    - BB analysis.  In this case we simply split the address into base,
      init and offset components, without reference to any containing loop.
------

but it wasn't obvious because of the elided stuff.

> @@ -893,14 +927,14 @@ dr_analyze_innermost (innermost_loop_beh
>    init = size_binop (PLUS_EXPR, init, dinit);
>    base_misalignment -= TREE_INT_CST_LOW (dinit);
>
> -  split_constant_offset (offset_iv.base, &offset_iv.base, &dinit);
> -  init = size_binop (PLUS_EXPR, init, dinit);
> -
> -  step = size_binop (PLUS_EXPR,
> -                    fold_convert (ssizetype, base_iv.step),
> -                    fold_convert (ssizetype, offset_iv.step));
> -
>    base = canonicalize_base_object_address (base_iv.base);
> +  tree offset = size_binop (PLUS_EXPR,
> +                           fold_convert (ssizetype, offset_iv.base),
> +                           init);
>
> so you remove the split_constant_offset handling on offset_iv.base.
> This may end up no longer walking and expanding def stmts of
> SSA names contained therein.  I suppose this is fully intended
> so that re-computing the ref address using DR_BASE/DR_OFFSET
> doesn't end up expanding that redundant code?

Yeah.  I think DR_OFFSET is only going to be used by code that
wants to construct a new address, so there doesn't seem much
point taking apart the offset only to regimplify it when building
a new reference.

> For analysis relying on this one now needs to resort to
> DR_VAR/CONST_OFFSET where SCEV analysis hopefully performs similar
> expansions?

We still apply split_constant_offset to the DR_VAR_OFFSET as well;
the scev analysis applies on top of that rather than replacing it.

> Just sth to watch at ...
>
> @@ -921,12 +955,12 @@ dr_analyze_innermost (innermost_loop_beh
>      }
>
>    drb->base_address = base;
> -  drb->offset = fold_convert (ssizetype, offset_iv.base);
> -  drb->init = init;
> +  drb->offset = offset;
>    drb->step = step;
> +  split_constant_offset (scev, &drb->var_offset, &drb->const_offset);
>
> so is the full-fledged split_constant_offset (with its SSA name handling)
> still needed here?  Sth to eventually address with a followup.

Yeah, I think we still need it.  The SCEV stuff is only really useful
if the starting offset has a simple evolution in a containing loop.
For other cases we punt and just use the original generic expression.
In particular, if we're doing SLP on a single-block function, we need
everything that split_constant_offset currently does.

> @@ -1490,6 +1482,7 @@ ref_at_iteration (data_reference_p dr, i
>    tree ref_op2 = NULL_TREE;
>    tree new_offset;
>
> +  split_constant_offset (DR_OFFSET (dr), &off, &coff);
>    if (iter != 0)
>      {
>        new_offset = size_binop (MULT_EXPR, DR_STEP (dr), ssize_int (iter));
>
> likewise here?  Why do you think ref_at_iteration cares?  Is that because
> of codegen quality?  I'd have done with coff == size_zero_node plus
> simplifications that arise from that.

Yeah, I assumed it was codegen quality.  But maybe it was just a way to
compensate for the fact that the MEM_REF is built directly (rather than
via a fold) and so this was the easiest way of getting a nonzero second
operand to the MEM_REF.

Thanks,
Richard