Re: [Patch] OpenMP: Avoid ICE with LTO and 'omp allocate (was: [Patch] Fortran: Support OpenMP's 'allocate' directive for stack vars)
On Wed, Oct 18, 2023 at 12:56:01PM +0200, Tobias Burnus wrote: > 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 > 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. LGTM. Jakub
[Patch] OpenMP: Avoid ICE with LTO and 'omp allocate (was: [Patch] Fortran: Support OpenMP's 'allocate' directive for stack vars)
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
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 000..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
Re: [Patch] Fortran: Support OpenMP's 'allocate' directive for stack vars
On Wed, Oct 18, 2023 at 11:12:44AM +0200, Thomas Schwinge wrote: > Hi Tobias! > > On 2023-10-13T15:29:52+0200, Tobias Burnus wrote: > > => Updated patch attached > > When cherry-picking this commit 2d3dbf0eff668bed5f5f168b3cafd8590c54 > "Fortran: Support OpenMP's 'allocate' directive for stack vars" on top of > slightly older GCC sources (mentioning that just in case that's > relevant), in a configuration with offloading enabled (only), I see: > > +FAIL: gfortran.dg/gomp/allocate-13.f90 -O (internal compiler error: > tree code 'statement_list' is not supported in LTO streams) > +FAIL: gfortran.dg/gomp/allocate-13.f90 -O (test for excess errors) 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. Jakub
Re: [Patch] Fortran: Support OpenMP's 'allocate' directive for stack vars
Hi Tobias! On 2023-10-13T15:29:52+0200, Tobias Burnus wrote: > => Updated patch attached When cherry-picking this commit 2d3dbf0eff668bed5f5f168b3cafd8590c54 "Fortran: Support OpenMP's 'allocate' directive for stack vars" on top of slightly older GCC sources (mentioning that just in case that's relevant), in a configuration with offloading enabled (only), I see: +FAIL: gfortran.dg/gomp/allocate-13.f90 -O (internal compiler error: tree code 'statement_list' is not supported in LTO streams) +FAIL: gfortran.dg/gomp/allocate-13.f90 -O (test for excess errors) during IPA pass: modref [...]/gcc/testsuite/gfortran.dg/gomp/allocate-13.f90:10:3: internal compiler error: tree code 'statement_list' is not supported in LTO streams 0x13374fd lto_write_tree [...]/gcc/lto-streamer-out.cc:561 0x13374fd lto_output_tree_1 [...]/gcc/lto-streamer-out.cc:599 0x133f55b DFS::DFS(output_block*, tree_node*, bool, bool, bool) [...]/gcc/lto-streamer-out.cc:899 0x1340287 lto_output_tree(output_block*, tree_node*, bool, bool) [...]/gcc/lto-streamer-out.cc:1865 0x134197a output_function [...]/gcc/lto-streamer-out.cc:2436 0x134197a lto_output() [...]/gcc/lto-streamer-out.cc:2807 0x13d0551 write_lto [...]/gcc/passes.cc:2774 0x13d0551 ipa_write_summaries_1 [...]/gcc/passes.cc:2838 0x13d0551 ipa_write_summaries() [...]/gcc/passes.cc:2894 0x1002f2c ipa_passes [...]/gcc/cgraphunit.cc:2251 0x1002f2c symbol_table::compile() [...]/gcc/cgraphunit.cc:2331 0x10056b7 symbol_table::compile() [...]/gcc/cgraphunit.cc:2311 0x10056b7 symbol_table::finalize_compilation_unit() [...]/gcc/cgraphunit.cc:2583 Similarly: +FAIL: libgomp.fortran/allocate-6.f90 -O (internal compiler error: tree code 'statement_list' is not supported in LTO streams) +FAIL: libgomp.fortran/allocate-7.f90 -O (internal compiler error: tree code 'statement_list' is not supported in LTO streams) Grüße Thomas > Fortran: Support OpenMP's 'allocate' directive for stack vars > > gcc/fortran/ChangeLog: > > * gfortran.h (ext_attr_t): Add omp_allocate flag. > * match.cc (gfc_free_omp_namelist): Void deleting same > u2.allocator multiple times now that a sequence can use > the same one. > * openmp.cc (gfc_match_omp_clauses, gfc_match_omp_allocate): Use > same allocator expr multiple times. > (is_predefined_allocator): Make static. > (gfc_resolve_omp_allocate): Update/extend restriction checks; > remove sorry message. > (resolve_omp_clauses): Reject corarrays in allocate/allocators > directive. > * parse.cc (check_omp_allocate_stmt): Permit procedure pointers > here (rejected later) for less misleading diagnostic. > * trans-array.cc (gfc_trans_auto_array_allocation): Propagate > size for GOMP_alloc and location to which it should be added to. > * trans-decl.cc (gfc_trans_deferred_vars): Handle 'omp allocate' > for stack variables; sorry for static variables/common blocks. > * trans-openmp.cc (gfc_trans_omp_clauses): Evaluate 'allocate' > clause's allocator only once; fix adding expressions to the > block. > (gfc_trans_omp_single): Pass a block to gfc_trans_omp_clauses. > > gcc/ChangeLog: > > * gimplify.cc (gimplify_bind_expr): Handle Fortran's > 'omp allocate' for stack variables. > > libgomp/ChangeLog: > > * libgomp.texi (OpenMP Impl. Status): Mention that Fortran now > supports the allocate directive for stack variables. > * testsuite/libgomp.fortran/allocate-5.f90: New test. > * testsuite/libgomp.fortran/allocate-6.f90: New test. > * testsuite/libgomp.fortran/allocate-7.f90: New test. > * testsuite/libgomp.fortran/allocate-8.f90: New test. > > gcc/testsuite/ChangeLog: > > * c-c++-common/gomp/allocate-14.c: Fix directive name. > * c-c++-common/gomp/allocate-15.c: Likewise. > * c-c++-common/gomp/allocate-9.c: Fix comment typo. > * gfortran.dg/gomp/allocate-4.f90: Remove sorry dg-error. > * gfortran.dg/gomp/allocate-7.f90: Likewise. > * gfortran.dg/gomp/allocate-10.f90: New test. > * gfortran.dg/gomp/allocate-11.f90: New test. > * gfortran.dg/gomp/allocate-12.f90: New test. > * gfortran.dg/gomp/allocate-13.f90: New test. > * gfortran.dg/gomp/allocate-14.f90: New test. > * gfortran.dg/gomp/allocate-15.f90: New test. > * gfortran.dg/gomp/allocate-8.f90: New test. > * gfortran.dg/gomp/allocate-9.f90: New test. > > gcc/fortran/gfortran.h | 1 + > gcc/fortran/match.cc | 9 +- > gcc/fortran/openmp.cc| 62 +++- > gcc/fortran/parse.cc | 8 +- > gcc/fortran/trans-array.cc
