[PATCH] Fix ICEs with "omp declare simd" attribute on versioned fns or omp_fn* (PR middle-end/83977)

Richard Biener rguenther@suse.de
Wed Jan 24 16:18:00 GMT 2018


On January 24, 2018 4:47:06 PM GMT+01:00, Jakub Jelinek <jakub@redhat.com> wrote:
>Hi!
>
>The "omp declare simd" attribute refers to argument numbers of the
>functions, so trying to apply it on versioned functions that can
>perhaps
>have different number and types of arguments results in ICEs or
>wrong-code.
>Unfortunately, if simd attribute or #pragma omp declare simd is used
>on C++ ctors or dtors, those have DECL_ABSTRACT_ORIGIN of something
>that
>really doesn't exist, abstract ctor or dtor, so checking if
>the types of node->decl and its DECL_ABSTRACT_ORIGIN are compatible
>function
>types doesn't work.

So if the attribute is on the decl we clone it should be in the list of things that cloning adjusts or blocks cloning. 

>The following patch just keeps optimizing only the original functions,
>not
>any versioned copies of them, but still allows simd attribute handling
>on
>e.g. __builtin_sin.
>
>Bootstrapped/regtested on x86_64-linux and i686-linux.
>
>Richard, is the first hunk ok, the rest is OpenMP related and I can ack
>myself.

Yes. 

>2018-01-24  Jakub Jelinek  <jakub@redhat.com>
>
>	PR middle-end/83977
>	* tree.c (free_lang_data_in_decl): Don't clear DECL_ABSTRACT_ORIGIN
>	here.
>	* omp-low.c (create_omp_child_function): Remove "omp declare simd"
>	attributes from DECL_ATTRIBUTES (decl) without affecting
>	DECL_ATTRIBUTES (current_function_decl).
>	* omp-simd-clone.c (expand_simd_clones): Ignore DECL_ARTIFICIAL
>	functions with non-NULL DECL_ABSTRACT_ORIGIN.
>
>	* c-c++-common/gomp/pr83977-1.c: New test.
>	* c-c++-common/gomp/pr83977-2.c: New test.
>	* c-c++-common/gomp/pr83977-3.c: New test.
>	* gfortran.dg/gomp/pr83977.f90: New test.
>
>--- gcc/tree.c.jj	2018-01-23 14:48:50.216265866 +0100
>+++ gcc/tree.c	2018-01-24 11:40:30.845519905 +0100
>@@ -5329,16 +5329,6 @@ free_lang_data_in_decl (tree decl)
> 	 At this point, it is not needed anymore.  */
>       DECL_SAVED_TREE (decl) = NULL_TREE;
> 
>-      /* Clear the abstract origin if it refers to a method.
>-         Otherwise dwarf2out.c will ICE as we splice functions out of
>-         TYPE_FIELDS and thus the origin will not be output
>-         correctly.  */
>-      if (DECL_ABSTRACT_ORIGIN (decl)
>-	  && DECL_CONTEXT (DECL_ABSTRACT_ORIGIN (decl))
>-	  && RECORD_OR_UNION_TYPE_P
>-	       (DECL_CONTEXT (DECL_ABSTRACT_ORIGIN (decl))))
>-	DECL_ABSTRACT_ORIGIN (decl) = NULL_TREE;
>-
>  /* Sometimes the C++ frontend doesn't manage to transform a temporary
>        DECL_VINDEX referring to itself into a vtable slot number as it
> 	 should.  Happens with functions that are copied and then forgotten
>--- gcc/omp-low.c.jj	2018-01-04 00:43:16.106702767 +0100
>+++ gcc/omp-low.c	2018-01-24 12:59:37.566218901 +0100
>@@ -1585,6 +1585,23 @@ create_omp_child_function (omp_context *
>   DECL_INITIAL (decl) = make_node (BLOCK);
>   BLOCK_SUPERCONTEXT (DECL_INITIAL (decl)) = decl;
>   DECL_ATTRIBUTES (decl) = DECL_ATTRIBUTES (current_function_decl);
>+  /* Remove omp declare simd attribute from the new attributes.  */
>+  if (tree a = lookup_attribute ("omp declare simd", DECL_ATTRIBUTES
>(decl)))
>+    {
>+      while (tree a2 = lookup_attribute ("omp declare simd",
>TREE_CHAIN (a)))
>+	a = a2;
>+      a = TREE_CHAIN (a);
>+      for (tree *p = &DECL_ATTRIBUTES (decl); *p != a;)
>+	if (is_attribute_p ("omp declare simd", get_attribute_name (*p)))
>+	  *p = TREE_CHAIN (*p);
>+	else
>+	  {
>+	    tree chain = TREE_CHAIN (*p);
>+	    *p = copy_node (*p);
>+	    p = &TREE_CHAIN (*p);
>+	    *p = chain;
>+	  }
>+    }
>   DECL_FUNCTION_SPECIFIC_OPTIMIZATION (decl)
>     = DECL_FUNCTION_SPECIFIC_OPTIMIZATION (current_function_decl);
>   DECL_FUNCTION_SPECIFIC_TARGET (decl)
>--- gcc/omp-simd-clone.c.jj	2018-01-23 14:48:47.275261492 +0100
>+++ gcc/omp-simd-clone.c	2018-01-24 11:45:28.484494749 +0100
>@@ -1574,6 +1574,10 @@ expand_simd_clones (struct cgraph_node *
>   tree attr = lookup_attribute ("omp declare simd",
> 				DECL_ATTRIBUTES (node->decl));
>   if (attr == NULL_TREE
>+      /* Ignore artificial decls with an abstract origin, results of
>function
>+	 cloning, versioning etc.  We want to handle certain builtins
>+	 with simd attribute, like __builtin_sin.  */
>+      || (DECL_ARTIFICIAL (node->decl) && DECL_ABSTRACT_ORIGIN
>(node->decl))
>       || node->global.inlined_to
>       || lookup_attribute ("noclone", DECL_ATTRIBUTES (node->decl)))
>     return;
>--- gcc/testsuite/c-c++-common/gomp/pr83977-1.c.jj	2018-01-24
>11:46:00.946492004 +0100
>+++ gcc/testsuite/c-c++-common/gomp/pr83977-1.c	2018-01-24
>11:46:29.181489615 +0100
>@@ -0,0 +1,19 @@
>+/* PR middle-end/83977 */
>+/* { dg-do compile } */
>+/* { dg-additional-options "-O2" } */
>+
>+struct S { int a, b, c; };
>+
>+#pragma omp declare simd uniform(z) linear(v:1)
>+__attribute__((noinline)) static int
>+foo (int x, int y, struct S z, int u, int v)
>+{
>+  return x + y + z.a;
>+}
>+
>+int
>+bar (int x, int y, int z)
>+{
>+  struct S s = { z, 1, 1 };
>+  return foo (x, y, s, 0, 0);
>+}
>--- gcc/testsuite/c-c++-common/gomp/pr83977-2.c.jj	2018-01-24
>11:46:42.259488509 +0100
>+++ gcc/testsuite/c-c++-common/gomp/pr83977-2.c	2018-01-24
>11:50:08.173471101 +0100
>@@ -0,0 +1,18 @@
>+/* PR middle-end/83977 */
>+/* { dg-do compile } */
>+
>+void bar (void);
>+
>+#pragma omp declare simd uniform (b) linear(a:b)
>+int
>+foo (int a, int b)
>+{
>+  a = a + 1;
>+/* This function can't be called from simd loops,
>+   because it violates declare simd restrictions.
>+   We shouldn't ICE on it though, nor attempt to generate
>+   simd clones for the *omp_fn* functions.  */
>+  #pragma omp parallel
>+  bar ();  
>+  return a;
>+}
>--- gcc/testsuite/c-c++-common/gomp/pr83977-3.c.jj	2018-01-24
>13:11:27.421111722 +0100
>+++ gcc/testsuite/c-c++-common/gomp/pr83977-3.c	2018-01-24
>13:12:08.945106675 +0100
>@@ -0,0 +1,21 @@
>+/* PR middle-end/83977 */
>+/* { dg-do compile } */
>+
>+void bar (void);
>+int foo (int, int) __attribute__((used));
>+
>+#pragma omp declare simd uniform (b) linear(a:b)
>+int
>+foo (int a, int b)
>+{
>+  a = a + 1;
>+/* This function can't be called from simd loops,
>+   because it violates declare simd restrictions.
>+   We shouldn't ICE on it though, nor attempt to generate
>+   simd clones for the *omp_fn* functions.  */
>+  #pragma omp parallel
>+  bar ();  
>+  return a;
>+}
>+
>+int foo (int, int)  __attribute__((unused));
>--- gcc/testsuite/gfortran.dg/gomp/pr83977.f90.jj	2018-01-24
>11:26:11.425592553 +0100
>+++ gcc/testsuite/gfortran.dg/gomp/pr83977.f90	2018-01-24
>11:26:11.425592553 +0100
>@@ -0,0 +1,15 @@
>+! PR middle-end/83977
>+! { dg-do compile }
>+
>+integer function foo (a, b)
>+   integer :: a, b
>+!$omp declare simd uniform(b) linear(ref(a):b)
>+   a = a + 1
>+! This function can't be called from simd loops,
>+! because it violates declare simd restrictions.
>+! We shouldn't ICE on it though, nor attempt to generate
>+! simd clones for the *omp_fn* functions.
>+!$omp parallel
>+   call sub
>+!$omp end parallel
>+end
>
>	Jakub



More information about the Gcc-patches mailing list