public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
From: Tobias Burnus <burnus@gcc.gnu.org>
To: gcc-cvs@gcc.gnu.org
Subject: [gcc r14-3868] OpenMP (C only): For 'omp allocate', really walk tree for 'alloctor' check
Date: Tue, 12 Sep 2023 09:15:20 +0000 (GMT)	[thread overview]
Message-ID: <20230912091520.C2FBA3858C50@sourceware.org> (raw)

https://gcc.gnu.org/g:27144cc05c4e12f58998b6e30d23098664dd51db

commit r14-3868-g27144cc05c4e12f58998b6e30d23098664dd51db
Author: Tobias Burnus <tobias@codesourcery.com>
Date:   Tue Sep 12 11:12:16 2023 +0200

    OpenMP (C only): For 'omp allocate', really walk tree for 'alloctor' check
    
    Walk expression tree of the 'allocator' clause of 'omp allocate' to
    detect more cases where the allocator expression depends on code between
    a variable declaration and its associated '#pragma omp allocate'. It also
    contains the fix for the 'allocator((omp_allocator_handle_t)-1)' ICE, also
    tested for in previous commit.
    
    The changes of this commit were supposed to be part of
      r14-3863-g35f498d8dfc8e579eaba2ff2d2b96769c632fd58
      OpenMP (C only): omp allocate - extend parsing support, improve diagnostic
    which also contains the associated testcase changes but were left out (oops!).
    
    gcc/c/ChangeLog:
    
            * c-parser.cc (struct c_omp_loc_tree): New.
            (c_check_omp_allocate_allocator_r): New; checking moved from ...
            (c_parser_omp_allocate): ... here. Call it via walk_tree. Avoid
            ICE with tree_to_shwi for invalid too-large value.

Diff:
---
 gcc/c/c-parser.cc | 102 ++++++++++++++++++++++++++++++++----------------------
 1 file changed, 61 insertions(+), 41 deletions(-)

diff --git a/gcc/c/c-parser.cc b/gcc/c/c-parser.cc
index 643ec02706b2..b9a1b75ca43c 100644
--- a/gcc/c/c-parser.cc
+++ b/gcc/c/c-parser.cc
@@ -19343,6 +19343,61 @@ c_parser_oacc_wait (location_t loc, c_parser *parser, char *p_name)
   return stmt;
 }
 
+struct c_omp_loc_tree
+{
+  location_t loc;
+  tree var;
+};
+
+/* Check whether the expression used in the allocator clause is declared or
+   modified between the variable declaration and its allocate directive.  */
+static tree
+c_check_omp_allocate_allocator_r (tree *tp, int *, void *data)
+{
+  tree var = ((struct c_omp_loc_tree *) data)->var;
+  location_t loc = ((struct c_omp_loc_tree *) data)->loc;
+  if (TREE_CODE (*tp) == VAR_DECL && c_check_in_current_scope (*tp))
+    {
+      if (linemap_location_before_p (line_table, DECL_SOURCE_LOCATION (var),
+				     DECL_SOURCE_LOCATION (*tp)))
+	{
+	  error_at (loc, "variable %qD used in the %<allocator%> clause must "
+			 "be declared before %qD", *tp, var);
+	  inform (DECL_SOURCE_LOCATION (*tp), "declared here");
+	  inform (DECL_SOURCE_LOCATION (var),
+		  "to be allocated variable declared here");
+	  return *tp;
+	}
+      else
+	{
+	  gcc_assert (cur_stmt_list
+		      && TREE_CODE (cur_stmt_list) == STATEMENT_LIST);
+
+	  tree_stmt_iterator l = tsi_last (cur_stmt_list);
+	  while (!tsi_end_p (l))
+	    {
+	      if (linemap_location_before_p (line_table, EXPR_LOCATION (*l),
+					     DECL_SOURCE_LOCATION (var)))
+		  break;
+	      if (TREE_CODE (*l) == MODIFY_EXPR
+		  && TREE_OPERAND (*l, 0) == *tp)
+		{
+		  error_at (loc,
+			    "variable %qD used in the %<allocator%> clause "
+			    "must not be modified between declaration of %qD "
+			    "and its %<allocate%> directive", *tp, var);
+		  inform (EXPR_LOCATION (*l), "modified here");
+		  inform (DECL_SOURCE_LOCATION (var),
+			  "to be allocated variable declared here");
+		  return *tp;
+		}
+	      --l;
+	    }
+	}
+    }
+  return NULL_TREE;
+}
+
 /* OpenMP 5.x:
    # pragma omp allocate (list)  clauses
 
@@ -19465,8 +19520,8 @@ c_parser_omp_allocate (c_parser *parser)
 	    error_at (loc, "%<allocator%> clause required for "
 			   "static variable %qD", var);
 	  else if (allocator
-		   && (tree_int_cst_sgn (allocator) != 1
-		       || tree_to_shwi (allocator) > 8))
+		   && (wi::to_widest (allocator) < 1
+		       || wi::to_widest (allocator) > 8))
 	    /* 8 = largest predefined memory allocator. */
 	    error_at (allocator_loc,
 		      "%<allocator%> clause requires a predefined allocator as "
@@ -19477,46 +19532,11 @@ c_parser_omp_allocate (c_parser *parser)
 		      "%qD not yet supported", var);
 	  continue;
 	}
-      if (allocator
-	  && TREE_CODE (allocator) == VAR_DECL
-	  && c_check_in_current_scope (var))
+      if (allocator)
 	{
-	  if (linemap_location_before_p (line_table, DECL_SOURCE_LOCATION (var),
-					 DECL_SOURCE_LOCATION (allocator)))
-	    {
-	      error_at (OMP_CLAUSE_LOCATION (nl),
-			"allocator variable %qD must be declared before %qD",
-			allocator, var);
-	      inform (DECL_SOURCE_LOCATION (allocator),
-		      "allocator declared here");
-	      inform (DECL_SOURCE_LOCATION (var), "declared here");
-	    }
-	  else
-	   {
-	     gcc_assert (cur_stmt_list
-			 && TREE_CODE (cur_stmt_list) == STATEMENT_LIST);
-	     tree_stmt_iterator l = tsi_last (cur_stmt_list);
-	     while (!tsi_end_p (l))
-	       {
-		 if (linemap_location_before_p (line_table, EXPR_LOCATION (*l),
-						DECL_SOURCE_LOCATION (var)))
-		   break;
-		 if (TREE_CODE (*l) == MODIFY_EXPR
-		     && TREE_OPERAND (*l, 0) == allocator)
-		   {
-		     error_at (EXPR_LOCATION (*l),
-			       "allocator variable %qD, used in the "
-			       "%<allocate%> directive for %qD, must not be "
-			       "modified between declaration of %qD and its "
-			       "%<allocate%> directive",
-			       allocator, var, var);
-		     inform (DECL_SOURCE_LOCATION (var), "declared here");
-		     inform (OMP_CLAUSE_LOCATION (nl), "used here");
-		     break;
-		  }
-		--l;
-	     }
-	   }
+	  struct c_omp_loc_tree data
+	    = {EXPR_LOC_OR_LOC (allocator, OMP_CLAUSE_LOCATION (nl)), var};
+	  walk_tree (&allocator, c_check_omp_allocate_allocator_r, &data, NULL);
 	}
       DECL_ATTRIBUTES (var) = tree_cons (get_identifier ("omp allocate"),
 					 build_tree_list (allocator, alignment),

                 reply	other threads:[~2023-09-12  9:15 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230912091520.C2FBA3858C50@sourceware.org \
    --to=burnus@gcc.gnu.org \
    --cc=gcc-cvs@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).