public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [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).