public inbox for fortran@gcc.gnu.org
 help / color / mirror / Atom feed
From: Tobias Burnus <tobias@codesourcery.com>
To: Jakub Jelinek <jakub@redhat.com>, <gcc-patches@gcc.gnu.org>
Cc: <fortran@gcc.gnu.org>, Thomas Schwinge <thomas@codesourcery.com>
Subject: [Patch] OpenMP: Avoid ICE with LTO and 'omp allocate (was: [Patch] Fortran: Support OpenMP's 'allocate' directive for stack vars)
Date: Wed, 18 Oct 2023 12:56:01 +0200	[thread overview]
Message-ID: <8d407d7b-c546-4454-92c1-707ef00f0ba0@codesourcery.com> (raw)
In-Reply-To: <ZS+nN/ep57izBafT@tucnak>

[-- Attachment #1: Type: text/plain, Size: 1138 bytes --]

On 18.10.23 11:36, Jakub Jelinek wrote:
> On Wed, Oct 18, 2023 at 11:12:44AM +0200, Thomas Schwinge wrote:
>>      +FAIL: gfortran.dg/gomp/allocate-13.f90   -O  (internal compiler error: tree code 'statement_list' is not supported in LTO streams)
> Any references to GENERIC code in clauses etc. should have been gimplified
> or cleared during gimplification, we shouldn't support STATEMENT_LIST
> in LTO streaming.

We only needed the statement list as aid during the gimplify.cc handling
of GOMP_alloc/GOMP_free for Fortran. How about just remove_attribute it
in that case? As discussed, as DECL_ATTRIBUTE gets added from the left
to the DECL_CHAIN, there shouldn't be a problem of introducing shared
trees; note that 'omp allocate' itself is added per DECL, i.e. it does
not introduce sharing itself, either.

Tested with x86-64-gnu-linux.

Tobias
-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955

[-- Attachment #2: fix-lto.diff --]
[-- Type: text/x-patch, Size: 3585 bytes --]

OpenMP: Avoid ICE with LTO and 'omp allocate'

gcc/ChangeLog:

	* gimplify.cc (gimplify_bind_expr): Remove "omp allocate" attribute
	to avoid that auxillary statement list reaches to LTO.

gcc/testsuite/ChangeLog:

	* gfortran.dg/gomp/allocate-13a.f90: New test.

 gcc/gimplify.cc                                 | 18 ++++++++-----
 gcc/testsuite/gfortran.dg/gomp/allocate-13a.f90 | 34 +++++++++++++++++++++++++
 2 files changed, 45 insertions(+), 7 deletions(-)

diff --git a/gcc/gimplify.cc b/gcc/gimplify.cc
index 9c617c21381..22ff1075abb 100644
--- a/gcc/gimplify.cc
+++ b/gcc/gimplify.cc
@@ -1426,7 +1426,8 @@ gimplify_bind_expr (tree *expr_p, gimple_seq *pre_p)
 		      DECL_ATTRIBUTES (v)
 			= tree_cons (get_identifier ("omp allocate var"),
 				     build_tree_list (NULL_TREE, t),
-				     DECL_ATTRIBUTES (t));
+				     remove_attribute ("omp allocate",
+						       DECL_ATTRIBUTES (t)));
 		      tmp = build_fold_indirect_ref (v);
 		      TREE_THIS_NOTRAP (tmp) = 1;
 		      SET_DECL_VALUE_EXPR (t, tmp);
@@ -1473,7 +1474,12 @@ gimplify_bind_expr (tree *expr_p, gimple_seq *pre_p)
 			 at the top, unless an allocator or size expression
 			 requires to put it afterward; note that the size is
 			 always later in generated code; for strings, no
-			 size expr but still an expr might be available.  */
+			 size expr but still an expr might be available.
+			 As LTO does not handle a statement list, 'sl' has
+			 to be removed; done so by removing the attribute.  */
+		      DECL_ATTRIBUTES (t)
+			= remove_attribute ("omp allocate",
+					    DECL_ATTRIBUTES (t));
 		      tree sl = TREE_PURPOSE (TREE_CHAIN (TREE_VALUE (attr)));
 		      tree_stmt_iterator e = tsi_start (sl);
 		      tree needle = NULL_TREE;
@@ -1631,16 +1637,14 @@ gimplify_bind_expr (tree *expr_p, gimple_seq *pre_p)
 	  && !is_global_var (t)
 	  && DECL_CONTEXT (t) == current_function_decl)
 	{
-	  tree attr;
 	  if (flag_openmp
 	      && DECL_HAS_VALUE_EXPR_P (t)
 	      && TREE_USED (t)
-	      && ((attr = lookup_attribute ("omp allocate",
-					    DECL_ATTRIBUTES (t))) != NULL_TREE)
-	      && TREE_CHAIN (TREE_VALUE (attr)) == NULL_TREE)
+	      && lookup_attribute ("omp allocate", DECL_ATTRIBUTES (t)))
 	    {
 	      /* For Fortran, TREE_CHAIN (TREE_VALUE (attr)) is set, which
-		 causes that the GOMP_free call is already added above.  */
+		 causes that the GOMP_free call is already added above;
+		 and "omp allocate" is removed from DECL_ATTRIBUTES.  */
 	      tree v = TREE_OPERAND (DECL_VALUE_EXPR (t), 0);
 	      tree tmp = builtin_decl_explicit (BUILT_IN_GOMP_FREE);
 	      tmp = build_call_expr_loc (end_locus, tmp, 2, v,
diff --git a/gcc/testsuite/gfortran.dg/gomp/allocate-13a.f90 b/gcc/testsuite/gfortran.dg/gomp/allocate-13a.f90
new file mode 100644
index 00000000000..4b297cdb4aa
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/gomp/allocate-13a.f90
@@ -0,0 +1,34 @@
+! { dg-do compile { target lto } }
+! { dg-additional-options "-flto" }
+
+! Same as allocate-13.f90 but compiled with -flto.
+
+! This was failing before as the statement list,
+! used for placing the GOMP_alloc/GOMP_free leaked
+! through to LTO.
+
+module m
+  implicit none
+  !$omp requires dynamic_allocators
+contains
+subroutine f ()
+  !$omp declare target
+  integer :: var
+  !$omp allocate(var)
+  var = 5
+end
+
+subroutine h ()
+  !$omp target
+   !$omp parallel
+    !$omp single
+      block
+       integer :: var2(5)
+       !$omp allocate(var2)
+       var2(1) = 7
+      end block
+    !$omp end single
+   !$omp end parallel
+  !$omp end target
+end
+end module

  reply	other threads:[~2023-10-18 10:56 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-10 16:46 [Patch] Fortran: Support OpenMP's 'allocate' directive for stack vars Tobias Burnus
2023-10-13 11:01 ` Jakub Jelinek
2023-10-13 13:29   ` Tobias Burnus
2023-10-18  9:12     ` Thomas Schwinge
2023-10-18  9:36       ` Jakub Jelinek
2023-10-18 10:56         ` Tobias Burnus [this message]
2023-10-18 11:01           ` [Patch] OpenMP: Avoid ICE with LTO and 'omp allocate (was: [Patch] Fortran: Support OpenMP's 'allocate' directive for stack vars) Jakub Jelinek

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=8d407d7b-c546-4454-92c1-707ef00f0ba0@codesourcery.com \
    --to=tobias@codesourcery.com \
    --cc=fortran@gcc.gnu.org \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jakub@redhat.com \
    --cc=thomas@codesourcery.com \
    /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).