diff mbox

[gomp4.1] handle undeclared sink variables gracefully

Message ID 55A8508D.8030201@redhat.com
State New
Headers show

Commit Message

Aldy Hernandez July 17, 2015, 12:47 a.m. UTC 
The following:

#pragma omp ordered depend(sink:asdf)

...where asdf is undeclared, is ICEing in *finish_omp_clauses because we 
create a clause with NULL, and we're expecting a TREE_LIST.

In the attached patch, I have opted to avoid creating the ordered depend 
clause if we have a parse error.

I also noticed that we gave up after one undeclared sink variable.  We 
can do better.  We can keep parsing and generate error messages 
appropriately, but avoid adding the undeclared variables to the 
TREE_LIST.  As an alternative, we could avoid generating ANY sink clause 
(even for declared variables) if we encounter any problem, but so far we 
fail gracefully later, so I haven't done this.

OK for branch?
commit 6ec528841cee875cfd0bcac0e35f5a6db1df0f6b
Author: Aldy Hernandez <aldyh@redhat.com>
Date:   Thu Jul 16 16:38:19 2015 -0700

    c/
    	* c-parser.c (c_parser_omp_clause_depend_sink): Handle multiple
    	undeclared sink variables gracefully.
    cp/
    	* parser.c (cp_parser_omp_clause_depend_sink): Handle multiple
    	undeclared sink variables gracefully.
    testsuite/
    	* c-c++-common/gomp/sink-3.c: New test.

Comments

Jakub Jelinek July 17, 2015, 7:02 a.m. UTC | #1 
On Thu, Jul 16, 2015 at 05:47:09PM -0700, Aldy Hernandez wrote:
> commit 6ec528841cee875cfd0bcac0e35f5a6db1df0f6b
> Author: Aldy Hernandez <aldyh@redhat.com>
> Date:   Thu Jul 16 16:38:19 2015 -0700
> 
>     c/
>     	* c-parser.c (c_parser_omp_clause_depend_sink): Handle multiple
>     	undeclared sink variables gracefully.
>     cp/
>     	* parser.c (cp_parser_omp_clause_depend_sink): Handle multiple
>     	undeclared sink variables gracefully.
>     testsuite/
>     	* c-c++-common/gomp/sink-3.c: New test.

Ok, thanks.

	Jakub
diff mbox

Patch

diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index 0909223..2d43cd7 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -11878,55 +11878,55 @@  c_parser_omp_clause_depend_sink (c_parser *parser, location_t clause_loc,
 
       c_parser_consume_token (parser);
 
-      if (t != error_mark_node)
+      bool neg;
+      if (c_parser_next_token_is (parser, CPP_MINUS))
+	neg = true;
+      else if (c_parser_next_token_is (parser, CPP_PLUS))
+	neg = false;
+      else
 	{
-	  bool neg;
-
-	  if (c_parser_next_token_is (parser, CPP_MINUS))
-	    neg = true;
-	  else if (c_parser_next_token_is (parser, CPP_PLUS))
-	    neg = false;
-	  else
-	    {
-	      addend = integer_zero_node;
-	      goto add_to_vector;
-	    }
-	  c_parser_consume_token (parser);
+	  addend = integer_zero_node;
+	  goto add_to_vector;
+	}
+      c_parser_consume_token (parser);
 
-	  if (c_parser_next_token_is_not (parser, CPP_NUMBER))
-	    {
-	      c_parser_error (parser, "expected integer");
-	      return list;
-	    }
+      if (c_parser_next_token_is_not (parser, CPP_NUMBER))
+	{
+	  c_parser_error (parser, "expected integer");
+	  return list;
+	}
 
-	  addend = c_parser_peek_token (parser)->value;
-	  if (TREE_CODE (addend) != INTEGER_CST)
-	    {
-	      c_parser_error (parser, "expected integer");
-	      return list;
-	    }
-	  if (neg)
-	    {
-	      bool overflow;
-	      wide_int offset = wi::neg (addend, &overflow);
-	      addend = wide_int_to_tree (TREE_TYPE (addend), offset);
-	      if (overflow)
-		warning_at (c_parser_peek_token (parser)->location,
-			    OPT_Woverflow,
-			    "overflow in implicit constant conversion");
-	    }
-	  c_parser_consume_token (parser);
+      addend = c_parser_peek_token (parser)->value;
+      if (TREE_CODE (addend) != INTEGER_CST)
+	{
+	  c_parser_error (parser, "expected integer");
+	  return list;
+	}
+      if (neg)
+	{
+	  bool overflow;
+	  wide_int offset = wi::neg (addend, &overflow);
+	  addend = wide_int_to_tree (TREE_TYPE (addend), offset);
+	  if (overflow)
+	    warning_at (c_parser_peek_token (parser)->location,
+			OPT_Woverflow,
+			"overflow in implicit constant conversion");
+	}
+      c_parser_consume_token (parser);
 
-	add_to_vector:
-	  vec = tree_cons (addend, t, vec);
+    add_to_vector:
+      if (t != error_mark_node)
+	vec = tree_cons (addend, t, vec);
 
-	  if (c_parser_next_token_is_not (parser, CPP_COMMA))
-	    break;
+      if (c_parser_next_token_is_not (parser, CPP_COMMA))
+	break;
 
-	  c_parser_consume_token (parser);
-	}
+      c_parser_consume_token (parser);
     }
 
+  if (vec == NULL_TREE)
+    return list;
+
   tree u = build_omp_clause (clause_loc, OMP_CLAUSE_DEPEND);
   OMP_CLAUSE_DEPEND_KIND (u) = OMP_CLAUSE_DEPEND_SINK;
   OMP_CLAUSE_DECL (u) = nreverse (vec);
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 2b6ed0a..3e1b167 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -29483,61 +29483,61 @@  cp_parser_omp_clause_depend_sink (cp_parser *parser, location_t clause_loc,
 					 id_loc);
 	}
 
-      if (t != error_mark_node)
+      bool neg;
+      if (cp_lexer_next_token_is (parser->lexer, CPP_MINUS))
+	neg = true;
+      else if (cp_lexer_next_token_is (parser->lexer, CPP_PLUS))
+	neg = false;
+      else
 	{
-	  bool neg;
-
-	  if (cp_lexer_next_token_is (parser->lexer, CPP_MINUS))
-	    neg = true;
-	  else if (cp_lexer_next_token_is (parser->lexer, CPP_PLUS))
-	    neg = false;
-	  else
-	    {
-	      addend = integer_zero_node;
-	      goto add_to_vector;
-	    }
-	  cp_lexer_consume_token (parser->lexer);
+	  addend = integer_zero_node;
+	  goto add_to_vector;
+	}
+      cp_lexer_consume_token (parser->lexer);
 
-	  if (cp_lexer_next_token_is_not (parser->lexer, CPP_NUMBER))
-	    {
-	      cp_parser_error (parser, "expected integer");
-	      return list;
-	    }
+      if (cp_lexer_next_token_is_not (parser->lexer, CPP_NUMBER))
+	{
+	  cp_parser_error (parser, "expected integer");
+	  return list;
+	}
 
-	  addend = cp_lexer_peek_token (parser->lexer)->u.value;
-	  if (TREE_CODE (addend) != INTEGER_CST)
-	    {
-	      cp_parser_error (parser, "expected integer");
-	      return list;
-	    }
-	  if (neg)
-	    {
-	      bool overflow;
-	      wide_int offset = wi::neg (addend, &overflow);
-	      addend = wide_int_to_tree (TREE_TYPE (addend), offset);
-	      if (overflow)
-		warning_at (cp_lexer_peek_token (parser->lexer)->location,
-			    OPT_Woverflow,
-			    "overflow in implicit constant conversion");
-	    }
-	  cp_lexer_consume_token (parser->lexer);
+      addend = cp_lexer_peek_token (parser->lexer)->u.value;
+      if (TREE_CODE (addend) != INTEGER_CST)
+	{
+	  cp_parser_error (parser, "expected integer");
+	  return list;
+	}
+      if (neg)
+	{
+	  bool overflow;
+	  wide_int offset = wi::neg (addend, &overflow);
+	  addend = wide_int_to_tree (TREE_TYPE (addend), offset);
+	  if (overflow)
+	    warning_at (cp_lexer_peek_token (parser->lexer)->location,
+			OPT_Woverflow,
+			"overflow in implicit constant conversion");
+	}
+      cp_lexer_consume_token (parser->lexer);
 
-	add_to_vector:
-	  vec = tree_cons (addend, t, vec);
+    add_to_vector:
+      if (t != error_mark_node)
+	vec = tree_cons (addend, t, vec);
 
-	  if (cp_lexer_next_token_is_not (parser->lexer, CPP_COMMA))
-	    break;
+      if (cp_lexer_next_token_is_not (parser->lexer, CPP_COMMA))
+	break;
 
-	  cp_lexer_consume_token (parser->lexer);
-	}
+      cp_lexer_consume_token (parser->lexer);
     }
-  cp_parser_require (parser, CPP_CLOSE_PAREN, RT_CLOSE_PAREN);
 
-  tree u = build_omp_clause (clause_loc, OMP_CLAUSE_DEPEND);
-  OMP_CLAUSE_DEPEND_KIND (u) = OMP_CLAUSE_DEPEND_SINK;
-  OMP_CLAUSE_DECL (u) = nreverse (vec);
-  OMP_CLAUSE_CHAIN (u) = list;
-  return u;
+  if (cp_parser_require (parser, CPP_CLOSE_PAREN, RT_CLOSE_PAREN) && vec)
+    {
+      tree u = build_omp_clause (clause_loc, OMP_CLAUSE_DEPEND);
+      OMP_CLAUSE_DEPEND_KIND (u) = OMP_CLAUSE_DEPEND_SINK;
+      OMP_CLAUSE_DECL (u) = nreverse (vec);
+      OMP_CLAUSE_CHAIN (u) = list;
+      return u;
+    }
+  return list;
 }
 
 /* OpenMP 4.0:
diff --git a/gcc/testsuite/c-c++-common/gomp/sink-3.c b/gcc/testsuite/c-c++-common/gomp/sink-3.c
new file mode 100644
index 0000000..7cb16ed
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/gomp/sink-3.c
@@ -0,0 +1,20 @@ 
+/* { dg-do compile } */
+/* { dg-options "-fopenmp" } */
+
+/* Test that we can handle multiple undeclared sink variables
+   gracefully.  */
+
+void bar (int *);
+
+void
+foo ()
+{
+  int i,j;
+#pragma omp parallel for ordered(1)
+  for (i=0; i < 100; ++i)
+    {
+#pragma omp ordered depend(sink:poo-1,paa+1) /* { dg-error "poo.*declared.*paa.*declared" } */
+    bar(&i);
+#pragma omp ordered depend(source)
+    }
+}