* [Patch] tree-nested: Update assert for Fortran module vars [PR97927]
@ 2021-03-05 22:07 Tobias Burnus
2021-03-08 7:45 ` Richard Biener
0 siblings, 1 reply; 4+ messages in thread
From: Tobias Burnus @ 2021-03-05 22:07 UTC (permalink / raw)
To: gcc-patches, Richard Biener, Jakub Jelinek
[-- Attachment #1: Type: text/plain, Size: 1230 bytes --]
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
[-- Attachment #2: tree-nest.diff --]
[-- Type: text/x-patch, Size: 2015 bytes --]
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)
{
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Patch] tree-nested: Update assert for Fortran module vars [PR97927]
2021-03-05 22:07 [Patch] tree-nested: Update assert for Fortran module vars [PR97927] Tobias Burnus
@ 2021-03-08 7:45 ` Richard Biener
2021-03-08 11:38 ` Tobias Burnus
0 siblings, 1 reply; 4+ messages in thread
From: Richard Biener @ 2021-03-08 7:45 UTC (permalink / raw)
To: Tobias Burnus; +Cc: gcc-patches, Jakub Jelinek
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
>
>
--
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Patch] tree-nested: Update assert for Fortran module vars [PR97927]
2021-03-08 7:45 ` Richard Biener
@ 2021-03-08 11:38 ` Tobias Burnus
2021-03-08 12:03 ` Richard Biener
0 siblings, 1 reply; 4+ messages in thread
From: Tobias Burnus @ 2021-03-08 11:38 UTC (permalink / raw)
To: Richard Biener, Tobias Burnus; +Cc: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 943 bytes --]
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
[-- Attachment #2: tree-nest-v2.diff --]
[-- Type: text/x-patch, Size: 1960 bytes --]
tree-nested: Update assert for Fortran module vars [PR97927]
gcc/ChangeLog:
PR fortran/97927
* tree-nested.c (convert_local_reference_stmt): Avoid calling
lookup_field_for_decl for Fortran module (= 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 | 1 +
2 files changed, 38 insertions(+)
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..cea917a4d58 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)
+ && decl_function_context (lhs) == info->context
&& !use_pointer_in_frame (lhs)
&& lookup_field_for_decl (info, lhs, NO_INSERT))
{
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Patch] tree-nested: Update assert for Fortran module vars [PR97927]
2021-03-08 11:38 ` Tobias Burnus
@ 2021-03-08 12:03 ` Richard Biener
0 siblings, 0 replies; 4+ messages in thread
From: Richard Biener @ 2021-03-08 12:03 UTC (permalink / raw)
To: Tobias Burnus; +Cc: gcc-patches
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
>
--
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-03-08 12:03 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-05 22:07 [Patch] tree-nested: Update assert for Fortran module vars [PR97927] Tobias Burnus
2021-03-08 7:45 ` Richard Biener
2021-03-08 11:38 ` Tobias Burnus
2021-03-08 12:03 ` Richard Biener
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).