| Message ID | 961971c0-9b01-4a38-6ac7-243c29f7051e@codesourcery.com |
|---|---|
| State | New |
| Headers | show |
| Series | tree-nested: Update assert for Fortran module vars [PR97927] | expand |
On Fri, 5 Mar 2021, Tobias Burnus wrote: > Nested functions are permitted for C but not C++ as extension. > They are also permitted for Fortran, which generates DECL_CONTEXT > == NAMESPACE_DECL for module variables. > > That causes the gcc_assert (decl_function_context (decl) == info->context) > to fail in tree-nested.c's lookup_field_for_decl. > > The call happens in convert_local_reference_stmt for: > 2524 case GIMPLE_ASSIGN: > 2525 if (gimple_clobber_p (stmt)) > 2528 if (DECL_P (lhs) > 2529 && !use_pointer_in_frame (lhs) > 2530 && lookup_field_for_decl (info, lhs, NO_INSERT)) > > The latter runs into the assert mentioned above. And the > '= {CLOBBER}' occurs in gfortran due to the intent(out). > > As additional ingredient, a nested (internal) procedure involved, > obviously as otherwise tree-nested.c wouldn't be involved. > > The patch fixes the assert and in terms of the assert it makes > sense, but I am not sure whether there are assumptions > elsewhere which are wrong for NAMESPACE_DECL. I think the bug is elsewhere. We're not expecting non-local (non-auto) variables to be queried with lookup_field_for_decl. I think a reasonable fix would be sth like (also see convert_local_reference_op on how it processes decls). diff --git a/gcc/tree-nested.c b/gcc/tree-nested.c index addd6eef9ab..5619ebc85d4 100644 --- a/gcc/tree-nested.c +++ b/gcc/tree-nested.c @@ -2526,6 +2526,7 @@ convert_local_reference_stmt (gimple_stmt_iterator *gsi, bool *handled_ops_p, { tree lhs = gimple_assign_lhs (stmt); if (DECL_P (lhs) + && auto_var_p (lhs) && !use_pointer_in_frame (lhs) && lookup_field_for_decl (info, lhs, NO_INSERT)) { > OK for mainline? GCC 10? > > Tobias > > ----------------- > Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München > Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank > Thürauf > >
On 08.03.21 08:45, Richard Biener wrote: > On Fri, 5 Mar 2021, Tobias Burnus wrote: >> Nested functions are permitted for C but not C++ as extension. >> They are also permitted for Fortran, which generates DECL_CONTEXT >> == NAMESPACE_DECL for module variables. >> >> That causes the gcc_assert (decl_function_context (decl) == info->context) >> to fail in tree-nested.c's lookup_field_for_decl. [...] > I think the bug is elsewhere. We're not expecting non-local > (non-auto) variables to be queried with lookup_field_for_decl. [...] Now changed by doing the 'decl_function_context (decl) == info->context' check in the caller's condition, which matches the rest of the file. Thanks for the suggestion. OK for mainline? GCC 10? Tobias ----------------- Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank Thürauf
On Mon, 8 Mar 2021, Tobias Burnus wrote: > On 08.03.21 08:45, Richard Biener wrote: > > On Fri, 5 Mar 2021, Tobias Burnus wrote: > >> Nested functions are permitted for C but not C++ as extension. > >> They are also permitted for Fortran, which generates DECL_CONTEXT > >> == NAMESPACE_DECL for module variables. > >> > >> That causes the gcc_assert (decl_function_context (decl) == info->context) > >> to fail in tree-nested.c's lookup_field_for_decl. [...] > > I think the bug is elsewhere. We're not expecting non-local > > (non-auto) variables to be queried with lookup_field_for_decl. [...] > > Now changed by doing the > 'decl_function_context (decl) == info->context' > check in the caller's condition, which matches the rest of the file. > > Thanks for the suggestion. > > OK for mainline? GCC 10? OK for trunk and GCC 10 after a while. Richard. > Tobias > > ----------------- > Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München > Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank > Thürauf >
tree-nested: Update assert for Fortran module vars [PR97927] gcc/ChangeLog: PR fortran/97927 * tree-nested.c (lookup_field_for_decl): Also permit that the var is a Fortran module var (= namespace context). gcc/testsuite/ChangeLog: PR fortran/97927 * gfortran.dg/module_variable_3.f90: New test. gcc/testsuite/gfortran.dg/module_variable_3.f90 | 37 +++++++++++++++++++++++++ gcc/tree-nested.c | 3 +- 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/gcc/testsuite/gfortran.dg/module_variable_3.f90 b/gcc/testsuite/gfortran.dg/module_variable_3.f90 new file mode 100644 index 00000000000..0dae6d5bdd5 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/module_variable_3.f90 @@ -0,0 +1,37 @@ +! { dg-do compile } +! { dg-additional-options "-fdump-tree-original" } +! +! PR fortran/97927 +! +! Did ICE due to the in tree-nested.c due to {clobber} +! + +module mpi2 + interface + subroutine MPI_Allreduce(i) + implicit none + INTEGER, OPTIONAL, INTENT(OUT) :: i + end subroutine MPI_Allreduce + end interface +end module + +module modmpi + implicit none + integer ierror ! module variable = context NAMESPACE_DECL +end module + +subroutine exxengy + use modmpi + use mpi2, only: mpi_allreduce + implicit none + + ! intent(out) implies: ierror = {clobber} + call mpi_allreduce(ierror) + +contains + subroutine zrho2 + return + end subroutine +end subroutine + +! { dg-final { scan-tree-dump "ierror = {CLOBBER};" "original" } } diff --git a/gcc/tree-nested.c b/gcc/tree-nested.c index addd6eef9ab..cedeccac40b 100644 --- a/gcc/tree-nested.c +++ b/gcc/tree-nested.c @@ -382,7 +382,8 @@ static tree lookup_field_for_decl (struct nesting_info *info, tree decl, enum insert_option insert) { - gcc_checking_assert (decl_function_context (decl) == info->context); + gcc_checking_assert (decl_function_context (decl) == info->context + || TREE_CODE (DECL_CONTEXT (decl)) == NAMESPACE_DECL); if (insert == NO_INSERT) {
Nested functions are permitted for C but not C++ as extension. They are also permitted for Fortran, which generates DECL_CONTEXT == NAMESPACE_DECL for module variables. That causes the gcc_assert (decl_function_context (decl) == info->context) to fail in tree-nested.c's lookup_field_for_decl. The call happens in convert_local_reference_stmt for: 2524 case GIMPLE_ASSIGN: 2525 if (gimple_clobber_p (stmt)) 2528 if (DECL_P (lhs) 2529 && !use_pointer_in_frame (lhs) 2530 && lookup_field_for_decl (info, lhs, NO_INSERT)) The latter runs into the assert mentioned above. And the '= {CLOBBER}' occurs in gfortran due to the intent(out). As additional ingredient, a nested (internal) procedure involved, obviously as otherwise tree-nested.c wouldn't be involved. The patch fixes the assert and in terms of the assert it makes sense, but I am not sure whether there are assumptions elsewhere which are wrong for NAMESPACE_DECL. OK for mainline? GCC 10? Tobias ----------------- Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank Thürauf