This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH][GRAPHITE] Consistently use region analysis
On Sat, 14 Oct 2017, Sebastian Pop wrote:
> On Fri, Oct 13, 2017 at 8:02 AM, Richard Biener <rguenther@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?
I have no idea...
> Can we postpone handling those cases until we have an interesting case?
That would be my preference as well.
> 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.
Good. I'll install this and hope to get some spare cycles to look
at the latent wrong-code issues that have popped up on SPEC 2k6.
Richard.
>
> > Richard.
> >
> > 2017-10-13 Richard Biener <rguenther@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
> >
>
--
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)