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?

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

Reply via email to