On Fri, Oct 13, 2017 at 8:02 AM, Richard Biener <rguent...@suse.de> wrote:
>
> Now that SCEV instantiation handles regions properly (see hunk below
> for a minor fix) we can use it consistently from GRAPHITE and thus
> simplify scalar_evolution_in_region greatly.
>
> Bootstrap and regtest running on x86_64-unknown-linux-gnu.
>
> A lot of the parameter renaming stuff looks dead but a more "complete"
> patch causes some more SPEC miscompares and also a bootstrap issue
> (warning only, but an uninitialized use of 'int tem = 0;' ...).
>
> This is probably all latent issues coming up more easily now.
>
> Note that formerly we'd support invariant "parameters" defined in
> the region by copying those out but now SCEV instantiation should
> lead chrec_dont_know for stuff we cannot gobble up (anythin not
> affine). This probably only worked for the outermost scop in the
> region and it means we need some other way to handle those.
How important is it to move defs out the region?
Can we postpone handling those cases until we have an interesting case?
The
> original issue is probably that "parameters" cannot occur in
> dependences and thus an array index cannot "depend" on the computation
> of a parameter (and array indexes coming from "data" cannot be handled
> anyway?).
Correct. Parameters can occur in array indexes as long as they cancel out.
For example, the following dependence can be computed:
A[p] vs. A[p+3]
and the following dependence cannot be computed
A[p] vs. A[0]
as the value of the parameter p is not known at compilation time.
We don't seem to have any functional testcase for those
> parameters that are not parameters.
>
>
Ok. Let's wait for a testcase that needs this functionality.
> Richard.
>
> 2017-10-13 Richard Biener <rguent...@suse.de>
>
> * graphite-scop-detection.c
> (scop_detection::stmt_has_simple_data_refs_p): Always use
> the full nest as region.
> (try_generate_gimple_bb): Likewise.
> (build_scops): First split edges, then compute RPO order.
> * sese.c (scalar_evolution_in_region): Simplify now that
> SCEV can handle instantiation in regions.
> * tree-scalar-evolution.c (instantiate_scev_name): Also instantiate
> in the non-loop part of a function if requested.
>
Looks good.
Thanks.
>
> Index: gcc/graphite-scop-detection.c
> ===================================================================
> --- gcc/graphite-scop-detection.c (revision 253721)
> +++ gcc/graphite-scop-detection.c (working copy)
> @@ -1005,15 +1005,10 @@ scop_detection::graphite_can_represent_e
> bool
> scop_detection::stmt_has_simple_data_refs_p (sese_l scop, gimple *stmt)
> {
> - edge nest;
> + edge nest = scop.entry;;
> loop_p loop = loop_containing_stmt (stmt);
> if (!loop_in_sese_p (loop, scop))
> - {
> - nest = scop.entry;
> - loop = NULL;
> - }
> - else
> - nest = loop_preheader_edge (outermost_loop_in_sese (scop, gimple_bb
> (stmt)));
> + loop = NULL;
>
> auto_vec<data_reference_p> drs;
> if (! graphite_find_data_references_in_stmt (nest, loop, stmt, &drs))
> @@ -1381,15 +1350,10 @@ try_generate_gimple_bb (scop_p scop, bas
> vec<scalar_use> reads = vNULL;
>
> sese_l region = scop->scop_info->region;
> - edge nest;
> + edge nest = region.entry;
> loop_p loop = bb->loop_father;
> if (!loop_in_sese_p (loop, region))
> - {
> - nest = region.entry;
> - loop = NULL;
> - }
> - else
> - nest = loop_preheader_edge (outermost_loop_in_sese (region, bb));
> + loop = NULL;
>
> for (gimple_stmt_iterator gsi = gsi_start_bb (bb); !gsi_end_p (gsi);
> gsi_next (&gsi))
> @@ -1696,6 +1660,13 @@ build_scops (vec<scop_p> *scops)
> /* Now create scops from the lightweight SESEs. */
> vec<sese_l> scops_l = sb.get_scops ();
>
> + /* For our out-of-SSA we need a block on s->entry, similar to how
> + we include the LCSSA block in the region. */
> + int i;
> + sese_l *s;
> + FOR_EACH_VEC_ELT (scops_l, i, s)
> + s->entry = single_pred_edge (split_edge (s->entry));
> +
> /* Domwalk needs a bb to RPO mapping. Compute it once here. */
> int *postorder = XNEWVEC (int, n_basic_blocks_for_fn (cfun));
> int postorder_num = pre_and_rev_post_order_compute (NULL, postorder,
> true);
> @@ -1704,14 +1675,8 @@ build_scops (vec<scop_p> *scops)
> bb_to_rpo[postorder[i]] = i;
> free (postorder);
>
> - int i;
> - sese_l *s;
> FOR_EACH_VEC_ELT (scops_l, i, s)
> {
> - /* For our out-of-SSA we need a block on s->entry, similar to how
> - we include the LCSSA block in the region. */
> - s->entry = single_pred_edge (split_edge (s->entry));
> -
> scop_p scop = new_scop (s->entry, s->exit);
>
> /* Record all basic blocks and their conditions in REGION. */
> Index: gcc/sese.c
> ===================================================================
> --- gcc/sese.c (revision 253721)
> +++ gcc/sese.c (working copy)
> @@ -459,41 +447,16 @@ scev_analyzable_p (tree def, sese_l ®
> tree
> scalar_evolution_in_region (const sese_l ®ion, loop_p loop, tree t)
> {
> - gimple *def;
> - struct loop *def_loop;
> -
> /* SCOP parameters. */
> if (TREE_CODE (t) == SSA_NAME
> && !defined_in_sese_p (t, region))
> return t;
>
> - if (TREE_CODE (t) != SSA_NAME
> - || loop_in_sese_p (loop, region))
> - /* FIXME: we would need instantiate SCEV to work on a region, and be
> more
> - flexible wrt. memory loads that may be invariant in the region. */
> - return instantiate_scev (region.entry, loop,
> - analyze_scalar_evolution (loop, t));
> -
> - def = SSA_NAME_DEF_STMT (t);
> - def_loop = loop_containing_stmt (def);
> -
> - if (loop_in_sese_p (def_loop, region))
> - {
> - t = analyze_scalar_evolution (def_loop, t);
> - def_loop = superloop_at_depth (def_loop, loop_depth (loop) + 1);
> - t = compute_overall_effect_of_inner_loop (def_loop, t);
> - return t;
> - }
> -
> - bool has_vdefs = false;
> - if (invariant_in_sese_p_rec (t, region, &has_vdefs))
> - return t;
> -
> - /* T variates in REGION. */
> - if (has_vdefs)
> - return chrec_dont_know;
> + if (!loop_in_sese_p (loop, region))
> + loop = NULL;
>
> - return instantiate_scev (region.entry, loop, t);
> + return instantiate_scev (region.entry, loop,
> + analyze_scalar_evolution (loop, t));
> }
>
> /* Return true if BB is empty, contains only DEBUG_INSNs. */
> Index: gcc/tree-scalar-evolution.c
> ===================================================================
> --- gcc/tree-scalar-evolution.c (revision 253721)
> +++ gcc/tree-scalar-evolution.c (working copy)
> @@ -2358,11 +2358,9 @@ instantiate_scev_name (edge instantiate_
> struct loop *def_loop;
> basic_block def_bb = gimple_bb (SSA_NAME_DEF_STMT (chrec));
>
> - /* A parameter (or loop invariant and we do not want to include
> - evolutions in outer loops), nothing to do. */
> + /* A parameter, nothing to do. */
> if (!def_bb
> - || loop_depth (def_bb->loop_father) == 0
> - || ! dominated_by_p (CDI_DOMINATORS, def_bb,
> instantiate_below->dest))
> + || !dominated_by_p (CDI_DOMINATORS, def_bb,
> instantiate_below->dest))
> return chrec;
>
> /* We cache the value of instantiated variable to avoid exponential
>
