On June 15, 2015 5:46:46 PM GMT+02:00, Jakub Jelinek <ja...@redhat.com> wrote:
>Hi!
>
>As Tom has reported, the for-2.c testcase ICEs at -O2 -fopenmp,
>because it has a noreturn function in the body and so while in omplower
>we decide to use "omp simd array" arrays, in ompexp there is no loop
>to attach the simd stuff to and I forgot to set the has_simduid_loops
>flag
>in that case (and propagate it to the outlined functions).
>
>But there is actually a bigger problem, the cleanup of the simduid
>internal
>calls and arrays has been done only in the vectorizer, but the
>vectorizer
>is part of the loop pass list that isn't run if there are no loops
>left,
>so e.g. in the testcase of noreturn loop where in the end there is no
>loop
>the cleanup wasn't performed.
>
>This patch (the omp-low.c part I can ack myself) thus clears the
>cfun->has_simduid_loops in the vectorizer after cleaning them up and
>adds a new pass gated on cfun->has_simduid_loops that performs just the
>cleanups. That pass will be invoked only for these pathological cases
>(when the vectorizer pass has not been run because there were no loops
>to vectorize, but still with OpenMP / Cilk+ code which has the simd
>directives).
>
>Bootstrapped/regtested on x86_64-linux and i686-linux, ok for
>trunk/5.2?
OK. Though I wonder whether this should be a property like the vector and
complex lowered state properties we have.
Thanks
Richard.
>2015-06-15 Jakub Jelinek <ja...@redhat.com>
>
> PR middle-end/66429
> * omp-low.c (expand_omp_taskreg): Use child_cfun instead of
> DECL_STRUCT_FUNCTION (child_fn). Or in has_simduid_loops
> and has_force_vectorize_loops flags from cfun into
> child_cfun.
> (expand_omp_simd): For broken loop, set cfun->has_simduid_loops
> if simduid is non-NULL.
> * tree-pass.h (make_pass_simduid_cleanup): New prototype.
> * passes.def (pass_simduid_cleanup): Add new pass after loop
> passes.
> * tree-vectorizer.c (adjust_simduid_builtins): Remove one unnecessary
> indirection from htab argument's type.
> (shrink_simd_arrays): New function.
> (vectorize_loops): Use it. Adjust adjust_simduid_builtins caller.
> Don't call adjust_simduid_builtins if there are no loops.
> (pass_data_simduid_cleanup, pass_simduid_cleanup): New variables.
> (pass_simduid_cleanup::execute): New method.
> (make_pass_simduid_cleanup): New function.
>
> * c-c++-common/gomp/pr66429.c: New test.
>
>--- gcc/omp-low.c.jj 2015-06-10 11:06:13.000000000 +0200
>+++ gcc/omp-low.c 2015-06-15 13:36:45.644277964 +0200
>@@ -5589,7 +5589,9 @@ expand_omp_taskreg (struct omp_region *r
> vec_safe_truncate (child_cfun->local_decls, dstidx);
>
> /* Inform the callgraph about the new function. */
>- DECL_STRUCT_FUNCTION (child_fn)->curr_properties =
>cfun->curr_properties;
>+ child_cfun->curr_properties = cfun->curr_properties;
>+ child_cfun->has_simduid_loops |= cfun->has_simduid_loops;
>+ child_cfun->has_force_vectorize_loops |=
>cfun->has_force_vectorize_loops;
> cgraph_node *node = cgraph_node::get_create (child_fn);
> node->parallelized_function = 1;
> cgraph_node::add_new_function (child_fn, true);
>@@ -7838,6 +7840,8 @@ expand_omp_simd (struct omp_region *regi
> cfun->has_force_vectorize_loops = true;
> }
> }
>+ else if (simduid)
>+ cfun->has_simduid_loops = true;
> }
>
>
>--- gcc/tree-pass.h.jj 2015-04-17 13:50:55.000000000 +0200
>+++ gcc/tree-pass.h 2015-06-15 14:18:50.299679523 +0200
>@@ -372,6 +372,7 @@ extern gimple_opt_pass *make_pass_graphi
> extern gimple_opt_pass *make_pass_if_conversion (gcc::context *ctxt);
>extern gimple_opt_pass *make_pass_loop_distribution (gcc::context
>*ctxt);
> extern gimple_opt_pass *make_pass_vectorize (gcc::context *ctxt);
>+extern gimple_opt_pass *make_pass_simduid_cleanup (gcc::context
>*ctxt);
> extern gimple_opt_pass *make_pass_slp_vectorize (gcc::context *ctxt);
>extern gimple_opt_pass *make_pass_complete_unroll (gcc::context *ctxt);
>extern gimple_opt_pass *make_pass_complete_unrolli (gcc::context
>*ctxt);
>--- gcc/passes.def.jj 2015-06-10 08:18:25.000000000 +0200
>+++ gcc/passes.def 2015-06-15 14:21:01.616671365 +0200
>@@ -270,6 +270,7 @@ along with GCC; see the file COPYING3.
> PUSH_INSERT_PASSES_WITHIN (pass_tree_no_loop)
> NEXT_PASS (pass_slp_vectorize);
> POP_INSERT_PASSES ()
>+ NEXT_PASS (pass_simduid_cleanup);
> NEXT_PASS (pass_lower_vector_ssa);
> NEXT_PASS (pass_cse_reciprocals);
> NEXT_PASS (pass_reassoc);
>--- gcc/tree-vectorizer.c.jj 2015-06-10 08:18:29.000000000 +0200
>+++ gcc/tree-vectorizer.c 2015-06-15 14:31:02.548482422 +0200
>@@ -171,7 +171,7 @@ simd_array_to_simduid::equal (const simd
> into their corresponding constants. */
>
> static void
>-adjust_simduid_builtins (hash_table<simduid_to_vf> **htab)
>+adjust_simduid_builtins (hash_table<simduid_to_vf> *htab)
> {
> basic_block bb;
>
>@@ -203,10 +203,12 @@ adjust_simduid_builtins (hash_table<simd
> gcc_assert (TREE_CODE (arg) == SSA_NAME);
> simduid_to_vf *p = NULL, data;
> data.simduid = DECL_UID (SSA_NAME_VAR (arg));
>- if (*htab)
>- p = (*htab)->find (&data);
>- if (p)
>- vf = p->vf;
>+ if (htab)
>+ {
>+ p = htab->find (&data);
>+ if (p)
>+ vf = p->vf;
>+ }
> switch (ifn)
> {
> case IFN_GOMP_SIMD_VF:
>@@ -309,6 +311,38 @@ note_simd_array_uses (hash_table<simd_ar
> walk_gimple_op (use_stmt, note_simd_array_uses_cb, &wi);
> }
> }
>+
>+/* Shrink arrays with "omp simd array" attribute to the corresponding
>+ vectorization factor. */
>+
>+static void
>+shrink_simd_arrays
>+ (hash_table<simd_array_to_simduid> *simd_array_to_simduid_htab,
>+ hash_table<simduid_to_vf> *simduid_to_vf_htab)
>+{
>+ for (hash_table<simd_array_to_simduid>::iterator iter
>+ = simd_array_to_simduid_htab->begin ();
>+ iter != simd_array_to_simduid_htab->end (); ++iter)
>+ if ((*iter)->simduid != -1U)
>+ {
>+ tree decl = (*iter)->decl;
>+ int vf = 1;
>+ if (simduid_to_vf_htab)
>+ {
>+ simduid_to_vf *p = NULL, data;
>+ data.simduid = (*iter)->simduid;
>+ p = simduid_to_vf_htab->find (&data);
>+ if (p)
>+ vf = p->vf;
>+ }
>+ tree atype
>+ = build_array_type_nelts (TREE_TYPE (TREE_TYPE (decl)), vf);
>+ TREE_TYPE (decl) = atype;
>+ relayout_decl (decl);
>+ }
>+
>+ delete simd_array_to_simduid_htab;
>+}
> >
> /* A helper function to free data refs. */
>
>@@ -445,11 +479,7 @@ vectorize_loops (void)
>
> /* Bail out if there are no loops. */
> if (vect_loops_num <= 1)
>- {
>- if (cfun->has_simduid_loops)
>- adjust_simduid_builtins (&simduid_to_vf_htab);
>- return 0;
>- }
>+ return 0;
>
> if (cfun->has_simduid_loops)
> note_simd_array_uses (&simd_array_to_simduid_htab);
>@@ -558,37 +588,14 @@ vectorize_loops (void)
>
> /* Fold IFN_GOMP_SIMD_{VF,LANE,LAST_LANE} builtins. */
> if (cfun->has_simduid_loops)
>- adjust_simduid_builtins (&simduid_to_vf_htab);
>+ adjust_simduid_builtins (simduid_to_vf_htab);
>
> /* Shrink any "omp array simd" temporary arrays to the
> actual vectorization factors. */
> if (simd_array_to_simduid_htab)
>- {
>- for (hash_table<simd_array_to_simduid>::iterator iter
>- = simd_array_to_simduid_htab->begin ();
>- iter != simd_array_to_simduid_htab->end (); ++iter)
>- if ((*iter)->simduid != -1U)
>- {
>- tree decl = (*iter)->decl;
>- int vf = 1;
>- if (simduid_to_vf_htab)
>- {
>- simduid_to_vf *p = NULL, data;
>- data.simduid = (*iter)->simduid;
>- p = simduid_to_vf_htab->find (&data);
>- if (p)
>- vf = p->vf;
>- }
>- tree atype
>- = build_array_type_nelts (TREE_TYPE (TREE_TYPE (decl)), vf);
>- TREE_TYPE (decl) = atype;
>- relayout_decl (decl);
>- }
>-
>- delete simd_array_to_simduid_htab;
>- }
>- delete simduid_to_vf_htab;
>- simduid_to_vf_htab = NULL;
>+ shrink_simd_arrays (simd_array_to_simduid_htab,
>simduid_to_vf_htab);
>+ delete simduid_to_vf_htab;
>+ cfun->has_simduid_loops = false;
>
> if (num_vectorized_loops > 0)
> {
>@@ -603,6 +610,64 @@ vectorize_loops (void)
> }
>
>
>+/* Entry point to the simduid cleanup pass. */
>+
>+namespace {
>+
>+const pass_data pass_data_simduid_cleanup =
>+{
>+ GIMPLE_PASS, /* type */
>+ "simduid", /* name */
>+ OPTGROUP_NONE, /* optinfo_flags */
>+ TV_NONE, /* tv_id */
>+ ( PROP_ssa | PROP_cfg ), /* properties_required */
>+ 0, /* properties_provided */
>+ 0, /* properties_destroyed */
>+ 0, /* todo_flags_start */
>+ 0, /* todo_flags_finish */
>+};
>+
>+class pass_simduid_cleanup : public gimple_opt_pass
>+{
>+public:
>+ pass_simduid_cleanup (gcc::context *ctxt)
>+ : gimple_opt_pass (pass_data_simduid_cleanup, ctxt)
>+ {}
>+
>+ /* opt_pass methods: */
>+ opt_pass * clone () { return new pass_simduid_cleanup (m_ctxt); }
>+ virtual bool gate (function *fun) { return fun->has_simduid_loops; }
>+ virtual unsigned int execute (function *);
>+
>+}; // class pass_simduid_cleanup
>+
>+unsigned int
>+pass_simduid_cleanup::execute (function *fun)
>+{
>+ hash_table<simd_array_to_simduid> *simd_array_to_simduid_htab =
>NULL;
>+
>+ note_simd_array_uses (&simd_array_to_simduid_htab);
>+
>+ /* Fold IFN_GOMP_SIMD_{VF,LANE,LAST_LANE} builtins. */
>+ adjust_simduid_builtins (NULL);
>+
>+ /* Shrink any "omp array simd" temporary arrays to the
>+ actual vectorization factors. */
>+ if (simd_array_to_simduid_htab)
>+ shrink_simd_arrays (simd_array_to_simduid_htab, NULL);
>+ fun->has_simduid_loops = false;
>+ return 0;
>+}
>+
>+} // anon namespace
>+
>+gimple_opt_pass *
>+make_pass_simduid_cleanup (gcc::context *ctxt)
>+{
>+ return new pass_simduid_cleanup (ctxt);
>+}
>+
>+
> /* Entry point to basic block SLP phase. */
>
> namespace {
>--- gcc/testsuite/c-c++-common/gomp/pr66429.c.jj 2015-06-15
>14:36:28.193502945 +0200
>+++ gcc/testsuite/c-c++-common/gomp/pr66429.c 2015-06-15
>14:35:41.000000000 +0200
>@@ -0,0 +1,41 @@
>+/* PR middle-end/66429 */
>+/* { dg-do compile } */
>+/* { dg-options "-O2 -fopenmp" } */
>+
>+float b[10][15][10];
>+
>+__attribute__ ((noreturn)) void
>+noreturn (void)
>+{
>+ for (;;);
>+}
>+
>+__attribute__ ((noinline, noclone)) void
>+foo (int n)
>+{
>+ int i;
>+
>+#pragma omp parallel for simd schedule(static, 32) collapse(3)
>+ for (i = 0; i < 10; i++)
>+ for (int j = n; j < 8; j++)
>+ for (long k = -10; k < 10; k++)
>+ {
>+ b[i][j][k] += 16;
>+ noreturn ();
>+ b[i][j][k] -= 32;
>+ }
>+}
>+
>+__attribute__ ((noinline, noclone)) void
>+bar (void)
>+{
>+ int i;
>+
>+#pragma omp parallel for simd schedule(static, 32)
>+ for (i = 0; i < 10; i++)
>+ {
>+ b[0][0][i] += 16;
>+ noreturn ();
>+ b[0][0][i] -= 32;
>+ }
>+}
>
> Jakub
