Richard Biener <richard.guent...@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
