On Sat, 14 Oct 2017, Sebastian Pop wrote: > 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?
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 <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 > > > -- Richard Biener <rguent...@suse.de> SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)