Re: [PATCH] Fortran: fixes for procedures with ALLOCATABLE,INTENT(OUT) arguments [PR92178]

2023-07-05 Thread Mikael Morin

Le 04/07/2023 à 21:37, Mikael Morin a écrit :

Le 04/07/2023 à 21:00, Harald Anlauf a écrit :

Hi Mikael, all,

I think I've found it: there is a call to gfc_conv_class_to_class
that - according to a comment - does a repackaging to a class array.
Deferring that repackaging along with the deallocation not only fixes
the regression, but also the cases I tested.

Attached is a "sneak preview", hoping that the experts (Paul, Mikael,
...) can tell if I am going down the wrong road.


I think that's it mostly.  There is one last thing that I am not sure...


diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc
index 16e8f037cfc..a68c8d33acc 100644
--- a/gcc/fortran/trans-expr.cc
+++ b/gcc/fortran/trans-expr.cc
@@ -6858,6 +6860,10 @@ gfc_conv_procedure_call (gfc_se * se, 
gfc_symbol * sym,

  && e->symtree->n.sym->attr.optional,
  CLASS_DATA (fsym)->attr.class_pointer
  || CLASS_DATA (fsym)->attr.allocatable);
+
+  /* Defer repackaging after deallocation.  */
+  if (defer_repackage)
+    gfc_add_block_to_block (&dealloc_blk, &parmse.pre);
 }
   else
 {


... whether you will not be deferring too much here.  That is parmse.pre 
contains both the argument evaluation and the class container setup from 
gfc_conv_class_to_class.  If it's safe to defer both, that's fine, 
otherwise a separate gfc_se struct should be passed to 
gfc_conv_class_to_class so that only the latter part can be deferred.

Need to think of an example...


Here is an example, admittedly artificial.  Fails with the above change, 
but fails with master as well.


program p
  implicit none
  type t
integer :: i
  end type t
  type u
class(t), allocatable :: ta(:)
  end type u
  type(u), allocatable, target :: c(:)
  c = [u([t(1), t(3)]), u([t(4), t(9)])]
  call bar (allocated (c(c(1)%ta(1)%i)%ta), c(c(1)%ta(1)%i)%ta, 
allocated (c(c(1)%ta(1)%i)%ta))

  if (allocated(c(1)%ta)) stop 11
  if (.not. allocated(c(2)%ta)) stop 12
contains
  subroutine bar (alloc, x, alloc2)
logical :: alloc, alloc2
class(t), allocatable, intent(out) :: x(:)
if (allocated (x)) stop 1
if (.not. alloc)   stop 2
if (.not. alloc2)  stop 3
  end subroutine bar
end



Re: [PATCH] Fortran: fixes for procedures with ALLOCATABLE,INTENT(OUT) arguments [PR92178]

2023-07-05 Thread Harald Anlauf via Fortran

Hi Mikael,

Am 05.07.23 um 16:54 schrieb Mikael Morin:

Here is an example, admittedly artificial.  Fails with the above change,
but fails with master as well.

program p
   implicit none
   type t
     integer :: i
   end type t
   type u
     class(t), allocatable :: ta(:)
   end type u
   type(u), allocatable, target :: c(:)
   c = [u([t(1), t(3)]), u([t(4), t(9)])]
   call bar (allocated (c(c(1)%ta(1)%i)%ta), c(c(1)%ta(1)%i)%ta,
allocated (c(c(1)%ta(1)%i)%ta))
   if (allocated(c(1)%ta)) stop 11
   if (.not. allocated(c(2)%ta)) stop 12
contains
   subroutine bar (alloc, x, alloc2)
     logical :: alloc, alloc2
     class(t), allocatable, intent(out) :: x(:)
     if (allocated (x)) stop 1
     if (.not. alloc)   stop 2
     if (.not. alloc2)  stop 3
   end subroutine bar
end


while it looks artificial, it is valid, and IMHO it is a beast...

I've played around and added another argument gfc_se *convse to
gfc_conv_class_to_class in an attempt to implement what I thought
you suggested (to get the .pre/.post separately), but in the end
this did not lead to working code.  And the tree-dump for your
example above is beyond what I can grasp.

I've noticed that my attempt does not properly handle the
parmse.post; at least this is what the above example shows:
there is a small part after the call to bar that should have
been executed before that call, which I attribute to .post.
But my attempts in moving that part regresses on a couple
of testcases with class and intent(out).  I am at a loss now.

I am attaching the latest version of my patch to give you or
Paul or others the opportunity to see what is wrong or add the
missing pieces.

Thanks for your help so far.

Harald

From 989030fc04eacf97a034ab1f7ed85b932669f82d Mon Sep 17 00:00:00 2001
From: Harald Anlauf 
Date: Wed, 5 Jul 2023 22:21:09 +0200
Subject: [PATCH] Fortran: fixes for procedures with ALLOCATABLE,INTENT(OUT)
 arguments [PR92178]

gcc/fortran/ChangeLog:

	PR fortran/92178
	* trans-expr.cc (gfc_conv_procedure_call): Check procedures for
	allocatable dummy arguments with INTENT(OUT) and move deallocation
	of actual arguments after evaluation of argument expressions before
	the procedure is executed.

gcc/testsuite/ChangeLog:

	PR fortran/92178
	* gfortran.dg/intent_out_16.f90: New test.
	* gfortran.dg/intent_out_17.f90: New test.
	* gfortran.dg/intent_out_18.f90: New test.

Co-authored-by: Steven G. Kargl 
---
 gcc/fortran/trans-expr.cc   | 54 +++--
 gcc/testsuite/gfortran.dg/intent_out_16.f90 | 89 +
 gcc/testsuite/gfortran.dg/intent_out_17.f90 | 46 +++
 gcc/testsuite/gfortran.dg/intent_out_18.f90 | 31 +++
 4 files changed, 215 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/gfortran.dg/intent_out_16.f90
 create mode 100644 gcc/testsuite/gfortran.dg/intent_out_17.f90
 create mode 100644 gcc/testsuite/gfortran.dg/intent_out_18.f90

diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc
index 30946ba3f63..7017b652d6e 100644
--- a/gcc/fortran/trans-expr.cc
+++ b/gcc/fortran/trans-expr.cc
@@ -6085,9 +6085,10 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym,
   else
 info = NULL;
 
-  stmtblock_t post, clobbers;
+  stmtblock_t post, clobbers, dealloc_blk;
   gfc_init_block (&post);
   gfc_init_block (&clobbers);
+  gfc_init_block (&dealloc_blk);
   gfc_init_interface_mapping (&mapping);
   if (!comp)
 {
@@ -6117,6 +6118,32 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym,
 	   && UNLIMITED_POLY (sym)
 	   && comp && (strcmp ("_copy", comp->name) == 0);
 
+  /* Scan for allocatable actual arguments passed to allocatable dummy
+ arguments with INTENT(OUT).  As the corresponding actual arguments are
+ deallocated before execution of the procedure, we evaluate actual
+ argument expressions to avoid problems with possible dependencies.  */
+  bool force_eval_args = false;
+  gfc_formal_arglist *tmp_formal;
+  for (arg = args, tmp_formal = formal; arg != NULL;
+   arg = arg->next, tmp_formal = tmp_formal ? tmp_formal->next : NULL)
+{
+  e = arg->expr;
+  fsym = tmp_formal ? tmp_formal->sym : NULL;
+  if (e && fsym
+	  && e->expr_type == EXPR_VARIABLE
+	  && fsym->attr.intent == INTENT_OUT
+	  && (fsym->ts.type == BT_CLASS && fsym->attr.class_ok
+	  ? CLASS_DATA (fsym)->attr.allocatable
+	  : fsym->attr.allocatable)
+	  && e->symtree
+	  && e->symtree->n.sym
+	  && gfc_variable_attr (e, NULL).allocatable)
+	{
+	  force_eval_args = true;
+	  break;
+	}
+}
+
   /* Evaluate the arguments.  */
   for (arg = args, argc = 0; arg != NULL;
arg = arg->next, formal = formal ? formal->next : NULL, ++argc)
@@ -6680,7 +6707,7 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym,
 		  else
 			tmp = gfc_finish_block (&block);
 
-		  gfc_add_expr_to_block (&se->pre, tmp);
+		  gfc_add_expr_to_block (&dealloc_blk, tmp);
 		}
 
 		  /* A class array element needs converting back to be a
@