Re: [Patch] Fortran: Avoid SAVE_EXPR for deferred-len char types
On Tue, Feb 21, 2023 at 08:30:50AM +0100, Richard Biener wrote: > > I think this mostly helps with access inside the FE of the type 'size = > > TYPE_SIZE_UNIT(type)', which is used surprisingly often and is often > > directly evaluated (i.e. assigned to a temporary). > > that's what I thought So, either we can do a temporary hack where we stick the non-SAVE_EXPR in there but somehow mark those types in type_lang_specific (if they aren't yet) and clear that when passing the type from FE to the middle-end. Or, stick some bogus value into TYPE_SIZE_UNIT (error_mark_node or something worse that triggers ICEs all around, say VOID_CST) and fix up what breaks, say add a short function which will replace TYPE_SIZE_UNIT (type) and do if (TYPE_LANG_SPECIFIC (type) && deferred_len (type)) return some_type_lang_specific_field (type); else return TYPE_SIZE_UNIT (type) and replace those. Or mass replace TYPE_SIZE_UNIT (type) in the FE with the new function. Though, there surely are spots for which deferred-len types may never appear... Jakub
[Patch] Fortran/OpenMP: Fix mapping of array descriptors and deferred-length strings
This patch moves some generic code for Fortran out of gimplify.cc
to trans-openmp.cc and fixes several issues related to mapping.
Tested with nvptx offloading.
OK for mainline?
Caveats:
Besides the issues shown in the comment-out code, there remains also an
issue with implicit mapping - at least for deferred-length strings,
but I wouldn't be surprised if - at least depending on the used
'defaultmap' value (e.g. 'alloc') - there are also issues with array
descriptors.
Note:
Regarding the declare target check for mapping: Without declare
target, my assumption is that the hidden length variable will
get implicitly mapped if needed. Independent of deferred-length
or not, there is probably an issue with 'defaultmap(none)' and
the hidden variable. - In any case, I prefer to defer all those
issues to later (by having them captured in one/several PR).
Tobias
PS: This patch is a follow up to
[Patch] Fortran/OpenMP: Fix DT struct-component with 'alloc' and array descr
https://gcc.gnu.org/pipermail/gcc-patches/2022-November/604887.html
which fixed part of the problems. But as discussed on IRC, it did treat 'alloc'
as special and missed some other map types. - In addition, this patch has a
much extended test coverage and fixes some more issues found that way.
-
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
Fortran/OpenMP: Fix mapping of array descriptors and deferred-length strings
Previously, array descriptors might have been mapped as 'alloc'
instead of 'to' for 'alloc', not updating the array bounds. The
'alloc' could also appear for 'data exit', failing with a libgomp
assert. In some cases, either array descriptors or deferred-length
string's length variable was not mapped. And, finally, some offset
calculations with array-sections mappings went wrong.
The testcases contain some comment-out tests which require follow-up
work and for which PR exist. Those mostly relate to deferred-length
strings which have several issues beyong OpenMP support.
gcc/fortran/ChangeLog:
* trans-decl.cc (gfc_get_symbol_decl): Add attributes
such as 'declare target' also to hidden artificial
variable for deferred-length character variables.
* trans-openmp.cc (gfc_trans_omp_array_section,
gfc_trans_omp_clauses, gfc_trans_omp_target_exit_data):
Improve mapping of array descriptors and deferred-length
string variables.
gcc/ChangeLog:
* gimplify.cc (gimplify_scan_omp_clauses): Remove Fortran
special case.
libgomp/ChangeLog:
* testsuite/libgomp.fortran/target-enter-data-3.f90: Uncomment
'target exit data'.
* testsuite/libgomp.fortran/target-enter-data-4.f90: New test.
* testsuite/libgomp.fortran/target-enter-data-5.f90: New test.
* testsuite/libgomp.fortran/target-enter-data-6.f90: New test.
* testsuite/libgomp.fortran/target-enter-data-7.f90: New test.
gcc/fortran/trans-decl.cc | 2 +
gcc/fortran/trans-openmp.cc| 323
gcc/gimplify.cc| 42 +-
.../libgomp.fortran/target-enter-data-3.f90| 2 +-
.../libgomp.fortran/target-enter-data-4.f90| 540 +
.../libgomp.fortran/target-enter-data-5.f90| 540 +
.../libgomp.fortran/target-enter-data-6.f90| 392 +++
.../libgomp.fortran/target-enter-data-7.f90| 78 +++
8 files changed, 1783 insertions(+), 136 deletions(-)
diff --git a/gcc/fortran/trans-decl.cc b/gcc/fortran/trans-decl.cc
index ff64588..c46ffc1 100644
--- a/gcc/fortran/trans-decl.cc
+++ b/gcc/fortran/trans-decl.cc
@@ -1820,6 +1820,8 @@ gfc_get_symbol_decl (gfc_symbol * sym)
/* Add attributes to variables. Functions are handled elsewhere. */
attributes = add_attributes_to_decl (sym->attr, NULL_TREE);
decl_attributes (&decl, attributes, 0);
+ if (sym->ts.deferred)
+decl_attributes (&length, attributes, 0);
/* Symbols from modules should have their assembler names mangled.
This is done here rather than in gfc_finish_var_decl because it
diff --git a/gcc/fortran/trans-openmp.cc b/gcc/fortran/trans-openmp.cc
index 2d16f3b..5cb2668 100644
--- a/gcc/fortran/trans-openmp.cc
+++ b/gcc/fortran/trans-openmp.cc
@@ -2403,33 +2403,50 @@ static vec *doacross_steps;
/* Translate an array section or array element. */
static void
-gfc_trans_omp_array_section (stmtblock_t *block, gfc_omp_namelist *n,
- tree decl, bool element, gomp_map_kind ptr_kind,
- tree &node, tree &node2, tree &node3, tree &node4)
+gfc_trans_omp_array_section (stmtblock_t *block, gfc_exec_op op,
+ gfc_omp_namelist *n, tree decl, bool element,
+ gomp_map_kind ptr_kind, tree &node, tree &node2,
+ tree &node3, tree &node4)
{
gfc_se se;
tree ptr, ptr2;
tree elemsz = NULL_TREE;
Re: [PATCH] Fortran: improve checking of character length specification [PR96025]
Hi Thomas, Am 21.02.23 um 08:19 schrieb Thomas Koenig via Gcc-patches: Hi Harald, the attached patch fixes an ICE on invalid (non-integer) specification expressions for character length in function declarations. It appears that the error handling was already in place (mostly) and we need to essentially prevent run-on errors. Regtested on x86_64-pc-linux-gnu. OK for mainline? As a very minor matter of style, you might want to write function_result_typed = check_function_result_typed (); instead of if (check_function_result_typed ()) function_result_typed = true; I was considering that too, but believed that the logic around these places (a loop and an if) would confuse readers. Thinking again and rechecking, I've changed the patch to follow your suggestion, including a minor style cleanup. Committed as: https://gcc.gnu.org/g:6c1b825b3d6499dfeacf7c79dcf4b56a393ac204 commit r13-6265-g6c1b825b3d6499dfeacf7c79dcf4b56a393ac204 Author: Harald Anlauf Date: Mon Feb 20 21:28:09 2023 +0100 OK either way. The PR is marked as a 10/11/12/13 regression, so I would like to backport this as far as it seems reasonable. Also OK. Thanks for the patch! Thanks for the review! Harald Best regards Thomas
[PATCH] Fortran: reject invalid CHARACTER length of derived type components [PR96024]
Dear all,
the attached simple patch detects and rejects CHARACTER components
of derived types whose length specification is non-integer.
Regtested on x86_64-pc-linux-gnu. OK for mainline?
This PR is also marked as a 10/11/12/13 regression, so I would
like to backport this as far as it seems reasonable.
Thanks,
Harald
From 0a392415cb5d5486e3e660880c81d6fdbbb47285 Mon Sep 17 00:00:00 2001
From: Harald Anlauf
Date: Tue, 21 Feb 2023 22:06:33 +0100
Subject: [PATCH] Fortran: reject invalid CHARACTER length of derived type
components [PR96024]
gcc/fortran/ChangeLog:
PR fortran/96024
* resolve.cc (resolve_component): The type of a CHARACTER length
expression must be INTEGER.
gcc/testsuite/ChangeLog:
PR fortran/96024
* gfortran.dg/pr96024.f90: New test.
---
gcc/fortran/resolve.cc| 13 +
gcc/testsuite/gfortran.dg/pr96024.f90 | 11 +++
2 files changed, 24 insertions(+)
create mode 100644 gcc/testsuite/gfortran.dg/pr96024.f90
diff --git a/gcc/fortran/resolve.cc b/gcc/fortran/resolve.cc
index 427f901a438..2780c82c798 100644
--- a/gcc/fortran/resolve.cc
+++ b/gcc/fortran/resolve.cc
@@ -14892,6 +14892,19 @@ resolve_component (gfc_component *c, gfc_symbol *sym)
c->ts.u.cl->length ? &c->ts.u.cl->length->where : &c->loc);
return false;
}
+
+ if (c->ts.u.cl->length && c->ts.u.cl->length->ts.type != BT_INTEGER)
+ {
+ if (!c->ts.u.cl->length->error)
+ {
+ gfc_error ("Character length expression of component %qs at %L "
+ "must be of INTEGER type, found %s",
+ c->name, &c->ts.u.cl->length->where,
+ gfc_basic_typename (c->ts.u.cl->length->ts.type));
+ c->ts.u.cl->length->error = 1;
+ }
+ return false;
+ }
}
if (c->ts.type == BT_CHARACTER && c->ts.deferred
diff --git a/gcc/testsuite/gfortran.dg/pr96024.f90 b/gcc/testsuite/gfortran.dg/pr96024.f90
new file mode 100644
index 000..2c914a997f2
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/pr96024.f90
@@ -0,0 +1,11 @@
+! { dg-do compile }
+! PR fortran/96024 - ICE in mio_name_expr_t
+! Contributed by G.Steinmetz
+
+module m
+ implicit none
+ type t
+ character(char(1)) :: a ! { dg-error "must be of INTEGER type" }
+ end type
+ type(t) :: z = t('a')
+end
--
2.35.3
Re: [PATCH] Fortran: reject invalid CHARACTER length of derived type components [PR96024]
On Tue, Feb 21, 2023 at 10:18:58PM +0100, Harald Anlauf via Fortran wrote: > Dear all, > > the attached simple patch detects and rejects CHARACTER components > of derived types whose length specification is non-integer. > > Regtested on x86_64-pc-linux-gnu. OK for mainline? > > This PR is also marked as a 10/11/12/13 regression, so I would > like to backport this as far as it seems reasonable. > OK. Backports are fine with me, but give others a chance to comment. Thanks for continuing to pursue the backlog of bug reports. -- Steve
