public inbox for fortran@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH v2 0/9] fortran: clobber fixes [PR41453]
@ 2022-09-18 20:15 Mikael Morin
  2022-09-18 20:15 ` [PATCH v2 1/9] fortran: Move the clobber generation code Mikael Morin
                   ` (9 more replies)
  0 siblings, 10 replies; 14+ messages in thread
From: Mikael Morin @ 2022-09-18 20:15 UTC (permalink / raw)
  To: gcc-patches, fortran

Hello,

this is the second version of a set of changes around the clobber we
generate in the caller before a procedure call, for each actual argument
whose associated dummy has the INTENT(OUT) attribute.

The clobbering of partial variables (array elements, structure components) 
proved to be unsupported by the middle-end optimizers, even if it seemed
to work in practice.  So this version just removes it.

v1 of the series was posted here:
https://gcc.gnu.org/pipermail/gcc-patches/2022-September/601713.html
https://gcc.gnu.org/pipermail/fortran/2022-September/058165.html

Changes from v1:
 - patch 9/10 (clobber subreferences) has been dropped.
 - patch 10/10 (now 9/9): The test has been adjusted because some checks
   were failing without the dropped patch.  Basically there are less
   clobbers generated.
 - patch 5: In the test, an explicit kind has been added to integers,
   so that the dump match is not dependent on the
   -fdefault-integer-8 option.
 - patches 3, 4, 5, 8, and 10/10 (now 9/9): The tests have been renamed
   so that they are numbered in increasing order.

The first patch is a refactoring moving the clobber generation in
gfc_conv_procedure_call where it feels more appropriate.
The second patch is a fix for the ICE originally motivating my work
on this topic.
The third patch is a fix for some wrong code issue discovered with an
earlier version of this series.
The following patches are gradual condition loosenings to enable clobber 
generation in more and more cases.

Each patch has been tested through an incremental bootstrap and a
partial testsuite run on fortran *intent* tests, and the whole lot has
been run through the full fortran regression testsuite.
OK for master?


Harald Anlauf (1):
  fortran: Support clobbering with implicit interfaces [PR105012]

Mikael Morin (8):
  fortran: Move the clobber generation code
  fortran: Fix invalid function decl clobber ICE [PR105012]
  fortran: Move clobbers after evaluation of all arguments [PR106817]
  fortran: Support clobbering of reference variables [PR41453]
  fortran: Support clobbering of SAVE variables [PR87395]
  fortran: Support clobbering of ASSOCIATE variables [PR87397]
  fortran: Support clobbering of allocatables and pointers [PR41453]
  fortran: Support clobbering of derived types [PR41453]

 gcc/fortran/trans-expr.cc                     | 81 ++++++++++++-------
 gcc/fortran/trans.h                           |  3 +-
 .../gfortran.dg/intent_optimize_4.f90         | 43 ++++++++++
 .../gfortran.dg/intent_optimize_5.f90         | 24 ++++++
 .../gfortran.dg/intent_optimize_6.f90         | 34 ++++++++
 .../gfortran.dg/intent_optimize_7.f90         | 42 ++++++++++
 .../gfortran.dg/intent_optimize_8.f90         | 66 +++++++++++++++
 gcc/testsuite/gfortran.dg/intent_out_15.f90   | 27 +++++++
 8 files changed, 290 insertions(+), 30 deletions(-)
 create mode 100644 gcc/testsuite/gfortran.dg/intent_optimize_4.f90
 create mode 100644 gcc/testsuite/gfortran.dg/intent_optimize_5.f90
 create mode 100644 gcc/testsuite/gfortran.dg/intent_optimize_6.f90
 create mode 100644 gcc/testsuite/gfortran.dg/intent_optimize_7.f90
 create mode 100644 gcc/testsuite/gfortran.dg/intent_optimize_8.f90
 create mode 100644 gcc/testsuite/gfortran.dg/intent_out_15.f90

-- 
2.35.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2 1/9] fortran: Move the clobber generation code
  2022-09-18 20:15 [PATCH v2 0/9] fortran: clobber fixes [PR41453] Mikael Morin
@ 2022-09-18 20:15 ` Mikael Morin
  2022-09-18 20:15 ` [PATCH v2 2/9] fortran: Fix invalid function decl clobber ICE [PR105012] Mikael Morin
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Mikael Morin @ 2022-09-18 20:15 UTC (permalink / raw)
  To: gcc-patches, fortran

This change inlines the clobber generation code from
gfc_conv_expr_reference to the single caller from where the add_clobber
flag can be true, and removes the add_clobber argument.

What motivates this is the standard making the procedure call a cause
for a variable to become undefined, which translates to a clobber
generation, so clobber generation should be closely related to procedure
call generation, whereas it is rather orthogonal to variable reference
generation.  Thus the generation of the clobber feels more appropriate
in gfc_conv_procedure_call than in gfc_conv_expr_reference.

Behaviour remains unchanged.

gcc/fortran/ChangeLog:

	* trans.h (gfc_conv_expr_reference): Remove add_clobber
	argument.
	* trans-expr.cc (gfc_conv_expr_reference): Ditto. Inline code
	depending on add_clobber and conditions controlling it ...
	(gfc_conv_procedure_call): ... to here.
---
 gcc/fortran/trans-expr.cc | 58 +++++++++++++++++++++------------------
 gcc/fortran/trans.h       |  3 +-
 2 files changed, 32 insertions(+), 29 deletions(-)

diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc
index 850007fd2e1..7902b941c2d 100644
--- a/gcc/fortran/trans-expr.cc
+++ b/gcc/fortran/trans-expr.cc
@@ -6395,7 +6395,7 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym,
 				&& e->symtree->n.sym->attr.pointer))
 			&& fsym && fsym->attr.target)
 		/* Make sure the function only gets called once.  */
-		gfc_conv_expr_reference (&parmse, e, false);
+		gfc_conv_expr_reference (&parmse, e);
 	      else if (e->expr_type == EXPR_FUNCTION
 		       && e->symtree->n.sym->result
 		       && e->symtree->n.sym->result != e->symtree->n.sym
@@ -6502,22 +6502,36 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym,
 		    }
 		  else
 		    {
-		      bool add_clobber;
-		      add_clobber = fsym && fsym->attr.intent == INTENT_OUT
-			&& !fsym->attr.allocatable && !fsym->attr.pointer
-			&& e->symtree && e->symtree->n.sym
-			&& !e->symtree->n.sym->attr.dimension
-			&& !e->symtree->n.sym->attr.pointer
-			&& !e->symtree->n.sym->attr.allocatable
-			/* See PR 41453.  */
-			&& !e->symtree->n.sym->attr.dummy
-			/* FIXME - PR 87395 and PR 41453  */
-			&& e->symtree->n.sym->attr.save == SAVE_NONE
-			&& !e->symtree->n.sym->attr.associate_var
-			&& e->ts.type != BT_CHARACTER && e->ts.type != BT_DERIVED
-			&& e->ts.type != BT_CLASS && !sym->attr.elemental;
+		      gfc_conv_expr_reference (&parmse, e);
 
-		      gfc_conv_expr_reference (&parmse, e, add_clobber);
+		      if (fsym
+			  && fsym->attr.intent == INTENT_OUT
+			  && !fsym->attr.allocatable
+			  && !fsym->attr.pointer
+			  && e->expr_type == EXPR_VARIABLE
+			  && e->ref == NULL
+			  && e->symtree
+			  && e->symtree->n.sym
+			  && !e->symtree->n.sym->attr.dimension
+			  && !e->symtree->n.sym->attr.pointer
+			  && !e->symtree->n.sym->attr.allocatable
+			  /* See PR 41453.  */
+			  && !e->symtree->n.sym->attr.dummy
+			  /* FIXME - PR 87395 and PR 41453  */
+			  && e->symtree->n.sym->attr.save == SAVE_NONE
+			  && !e->symtree->n.sym->attr.associate_var
+			  && e->ts.type != BT_CHARACTER
+			  && e->ts.type != BT_DERIVED
+			  && e->ts.type != BT_CLASS
+			  && !sym->attr.elemental)
+			{
+			  tree var;
+			  /* FIXME: This fails if var is passed by reference, see PR
+			     41453.  */
+			  var = e->symtree->n.sym->backend_decl;
+			  tree clobber = build_clobber (TREE_TYPE (var));
+			  gfc_add_modify (&se->pre, var, clobber);
+			}
 		    }
 		  /* Catch base objects that are not variables.  */
 		  if (e->ts.type == BT_CLASS
@@ -9485,7 +9499,7 @@ gfc_conv_expr_type (gfc_se * se, gfc_expr * expr, tree type)
    values only.  */
 
 void
-gfc_conv_expr_reference (gfc_se * se, gfc_expr * expr, bool add_clobber)
+gfc_conv_expr_reference (gfc_se * se, gfc_expr * expr)
 {
   gfc_ss *ss;
   tree var;
@@ -9525,16 +9539,6 @@ gfc_conv_expr_reference (gfc_se * se, gfc_expr * expr, bool add_clobber)
 	  gfc_add_block_to_block (&se->pre, &se->post);
 	  se->expr = var;
 	}
-      else if (add_clobber && expr->ref == NULL)
-	{
-	  tree clobber;
-	  tree var;
-	  /* FIXME: This fails if var is passed by reference, see PR
-	     41453.  */
-	  var = expr->symtree->n.sym->backend_decl;
-	  clobber = build_clobber (TREE_TYPE (var));
-	  gfc_add_modify (&se->pre, var, clobber);
-	}
       return;
     }
 
diff --git a/gcc/fortran/trans.h b/gcc/fortran/trans.h
index 03d5288aad2..bc9035c1717 100644
--- a/gcc/fortran/trans.h
+++ b/gcc/fortran/trans.h
@@ -499,8 +499,7 @@ tree gfc_build_compare_string (tree, tree, tree, tree, int, enum tree_code);
 void gfc_conv_expr (gfc_se * se, gfc_expr * expr);
 void gfc_conv_expr_val (gfc_se * se, gfc_expr * expr);
 void gfc_conv_expr_lhs (gfc_se * se, gfc_expr * expr);
-void gfc_conv_expr_reference (gfc_se * se, gfc_expr * expr,
-			      bool add_clobber = false);
+void gfc_conv_expr_reference (gfc_se * se, gfc_expr * expr);
 void gfc_conv_expr_type (gfc_se * se, gfc_expr *, tree);
 
 
-- 
2.35.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2 2/9] fortran: Fix invalid function decl clobber ICE [PR105012]
  2022-09-18 20:15 [PATCH v2 0/9] fortran: clobber fixes [PR41453] Mikael Morin
  2022-09-18 20:15 ` [PATCH v2 1/9] fortran: Move the clobber generation code Mikael Morin
@ 2022-09-18 20:15 ` Mikael Morin
  2022-09-18 20:15 ` [PATCH v2 3/9] fortran: Move clobbers after evaluation of all arguments [PR106817] Mikael Morin
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Mikael Morin @ 2022-09-18 20:15 UTC (permalink / raw)
  To: gcc-patches, fortran

The fortran frontend, as result symbol for a function without
declared result symbol, uses the function symbol itself.  This caused
an invalid clobber of a function decl to be emitted, leading to an
ICE, whereas the intended behaviour was to clobber the function result
variable.  This change fixes the problem by getting the decl from the
just-retrieved variable reference after the call to
gfc_conv_expr_reference, instead of copying it from the frontend symbol.

	PR fortran/105012

gcc/fortran/ChangeLog:

	* trans-expr.cc (gfc_conv_procedure_call): Retrieve variable
	from the just calculated variable reference.

gcc/testsuite/ChangeLog:

	* gfortran.dg/intent_out_15.f90: New test.
---
 gcc/fortran/trans-expr.cc                   |  3 ++-
 gcc/testsuite/gfortran.dg/intent_out_15.f90 | 27 +++++++++++++++++++++
 2 files changed, 29 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gfortran.dg/intent_out_15.f90

diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc
index 7902b941c2d..76c587e3d9f 100644
--- a/gcc/fortran/trans-expr.cc
+++ b/gcc/fortran/trans-expr.cc
@@ -6528,7 +6528,8 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym,
 			  tree var;
 			  /* FIXME: This fails if var is passed by reference, see PR
 			     41453.  */
-			  var = e->symtree->n.sym->backend_decl;
+			  var = build_fold_indirect_ref_loc (input_location,
+							     parmse.expr);
 			  tree clobber = build_clobber (TREE_TYPE (var));
 			  gfc_add_modify (&se->pre, var, clobber);
 			}
diff --git a/gcc/testsuite/gfortran.dg/intent_out_15.f90 b/gcc/testsuite/gfortran.dg/intent_out_15.f90
new file mode 100644
index 00000000000..64334e6f038
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/intent_out_15.f90
@@ -0,0 +1,27 @@
+! { dg-do compile }
+! { dg-additional-options "-fdump-tree-original" }
+!
+! PR fortran/105012
+! The following case was triggering an ICE because of a clobber
+! on the DERFC function decl instead of its result.
+
+module error_function
+integer, parameter :: r8 = selected_real_kind(12) ! 8 byte real
+contains
+SUBROUTINE CALERF_r8(ARG, RESULT, JINT)
+   integer, parameter :: rk = r8
+   real(rk), intent(in)  :: arg
+   real(rk), intent(out) :: result
+   IF (Y .LE. THRESH) THEN
+   END IF
+end SUBROUTINE CALERF_r8
+FUNCTION DERFC(X)
+   integer, parameter :: rk = r8 ! 8 byte real
+   real(rk), intent(in) :: X
+   real(rk) :: DERFC
+   CALL CALERF_r8(X, DERFC, JINT)
+END FUNCTION DERFC
+end module error_function
+
+! { dg-final { scan-tree-dump-times "CLOBBER" 1 "original" } }
+! { dg-final { scan-tree-dump "__result_derfc = {CLOBBER};" "original" } }
-- 
2.35.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2 3/9] fortran: Move clobbers after evaluation of all arguments [PR106817]
  2022-09-18 20:15 [PATCH v2 0/9] fortran: clobber fixes [PR41453] Mikael Morin
  2022-09-18 20:15 ` [PATCH v2 1/9] fortran: Move the clobber generation code Mikael Morin
  2022-09-18 20:15 ` [PATCH v2 2/9] fortran: Fix invalid function decl clobber ICE [PR105012] Mikael Morin
@ 2022-09-18 20:15 ` Mikael Morin
  2022-09-18 20:15 ` [PATCH v2 4/9] fortran: Support clobbering with implicit interfaces [PR105012] Mikael Morin
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Mikael Morin @ 2022-09-18 20:15 UTC (permalink / raw)
  To: gcc-patches, fortran

For actual arguments whose dummy is INTENT(OUT), we used to generate
clobbers on them at the same time we generated the argument reference
for the function call.  This was wrong if for an argument coming
later, the value expression was depending on the value of the just-
clobbered argument, and we passed an undefined value in that case.

With this change, clobbers are collected separatedly and appended
to the procedure call preliminary code after all the arguments have been
evaluated.

	PR fortran/106817

gcc/fortran/ChangeLog:

	* trans-expr.cc (gfc_conv_procedure_call): Collect all clobbers
	to their own separate block.  Append the block of clobbers to
	the procedure preliminary block after the argument evaluation
	codes for all the arguments.

gcc/testsuite/ChangeLog:

	* gfortran.dg/intent_optimize_4.f90: New test.
---
 gcc/fortran/trans-expr.cc                     |  6 ++-
 .../gfortran.dg/intent_optimize_4.f90         | 43 +++++++++++++++++++
 2 files changed, 47 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/gfortran.dg/intent_optimize_4.f90

diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc
index 76c587e3d9f..a62a3bb642d 100644
--- a/gcc/fortran/trans-expr.cc
+++ b/gcc/fortran/trans-expr.cc
@@ -6018,7 +6018,6 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym,
   gfc_charlen cl;
   gfc_expr *e;
   gfc_symbol *fsym;
-  stmtblock_t post;
   enum {MISSING = 0, ELEMENTAL, SCALAR, SCALAR_POINTER, ARRAY};
   gfc_component *comp = NULL;
   int arglen;
@@ -6062,7 +6061,9 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym,
   else
     info = NULL;
 
+  stmtblock_t post, clobbers;
   gfc_init_block (&post);
+  gfc_init_block (&clobbers);
   gfc_init_interface_mapping (&mapping);
   if (!comp)
     {
@@ -6531,7 +6532,7 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym,
 			  var = build_fold_indirect_ref_loc (input_location,
 							     parmse.expr);
 			  tree clobber = build_clobber (TREE_TYPE (var));
-			  gfc_add_modify (&se->pre, var, clobber);
+			  gfc_add_modify (&clobbers, var, clobber);
 			}
 		    }
 		  /* Catch base objects that are not variables.  */
@@ -7400,6 +7401,7 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym,
 
       vec_safe_push (arglist, parmse.expr);
     }
+  gfc_add_block_to_block (&se->pre, &clobbers);
   gfc_finish_interface_mapping (&mapping, &se->pre, &se->post);
 
   if (comp)
diff --git a/gcc/testsuite/gfortran.dg/intent_optimize_4.f90 b/gcc/testsuite/gfortran.dg/intent_optimize_4.f90
new file mode 100644
index 00000000000..effbaa12a2d
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/intent_optimize_4.f90
@@ -0,0 +1,43 @@
+! { dg-do run }
+! { dg-additional-options "-fdump-tree-original" }
+! { dg-final { scan-tree-dump-times "CLOBBER" 2 "original" } }
+!
+! PR fortran/106817
+! Check that for an actual argument whose dummy is INTENT(OUT),
+! the clobber that is emitted in the caller before a procedure call
+! happens after any expression depending on the argument value has been
+! evaluated.
+! 
+
+module m
+  implicit none
+contains
+  subroutine copy1(out, in)
+    integer, intent(in) :: in
+    integer, intent(out) :: out
+    out = in
+  end subroutine copy1
+  subroutine copy2(in, out)
+    integer, intent(in) :: in
+    integer, intent(out) :: out
+    out = in
+  end subroutine copy2
+end module m
+
+program p
+  use m
+  implicit none
+  integer :: a, b
+
+  ! Clobbering of a should happen after a+1 has been evaluated.
+  a = 3
+  call copy1(a, a+1)
+  if (a /= 4) stop 1
+
+  ! Clobbering order does not depend on the order of arguments.
+  ! It should also come last with reversed arguments.
+  b = 12
+  call copy2(b+1, b)
+  if (b /= 13) stop 2
+
+end program p
-- 
2.35.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2 4/9] fortran: Support clobbering with implicit interfaces [PR105012]
  2022-09-18 20:15 [PATCH v2 0/9] fortran: clobber fixes [PR41453] Mikael Morin
                   ` (2 preceding siblings ...)
  2022-09-18 20:15 ` [PATCH v2 3/9] fortran: Move clobbers after evaluation of all arguments [PR106817] Mikael Morin
@ 2022-09-18 20:15 ` Mikael Morin
  2022-09-18 20:15 ` [PATCH v2 5/9] fortran: Support clobbering of reference variables [PR41453] Mikael Morin
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Mikael Morin @ 2022-09-18 20:15 UTC (permalink / raw)
  To: gcc-patches, fortran

From: Harald Anlauf <anlauf@gmx.de>

Before procedure calls, we clobber actual arguments whose associated
dummy is INTENT(OUT).  This only applies to procedures with explicit
interfaces, as the knowledge of the interface is necessary to know
whether an argument has the INTENT(OUT) attribute.

This change also enables clobber generation for procedure calls without
explicit interface, when the procedure has been defined in the same
file because we can use the dummy arguments' characteristics from the
procedure definition in that case.

The knowledge of the dummy characteristics is directly available through
gfc_actual_arglist’s associated_dummy pointers which have been populated
as a side effect of calling gfc_check_externals.

	PR fortran/105012

gcc/fortran/ChangeLog:

	* trans-expr.cc (gfc_conv_procedure_call): Use dummy
	information from associated_dummy if there is no information
	from the procedure interface.

gcc/testsuite/ChangeLog:

	* gfortran.dg/intent_optimize_5.f90: New test.

Co-Authored-By: Mikael Morin <mikael@gcc.gnu.org>
---
 gcc/fortran/trans-expr.cc                     | 19 +++++++++++----
 .../gfortran.dg/intent_optimize_5.f90         | 24 +++++++++++++++++++
 2 files changed, 39 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/gfortran.dg/intent_optimize_5.f90

diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc
index a62a3bb642d..2301724729f 100644
--- a/gcc/fortran/trans-expr.cc
+++ b/gcc/fortran/trans-expr.cc
@@ -6505,10 +6505,21 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym,
 		    {
 		      gfc_conv_expr_reference (&parmse, e);
 
-		      if (fsym
-			  && fsym->attr.intent == INTENT_OUT
-			  && !fsym->attr.allocatable
-			  && !fsym->attr.pointer
+		      gfc_symbol *dsym = fsym;
+		      gfc_dummy_arg *dummy;
+
+		      /* Use associated dummy as fallback for formal
+			 argument if there is no explicit interface.  */
+		      if (dsym == NULL
+			  && (dummy = arg->associated_dummy)
+			  && dummy->intrinsicness == GFC_NON_INTRINSIC_DUMMY_ARG
+			  && dummy->u.non_intrinsic->sym)
+			dsym = dummy->u.non_intrinsic->sym;
+
+		      if (dsym
+			  && dsym->attr.intent == INTENT_OUT
+			  && !dsym->attr.allocatable
+			  && !dsym->attr.pointer
 			  && e->expr_type == EXPR_VARIABLE
 			  && e->ref == NULL
 			  && e->symtree
diff --git a/gcc/testsuite/gfortran.dg/intent_optimize_5.f90 b/gcc/testsuite/gfortran.dg/intent_optimize_5.f90
new file mode 100644
index 00000000000..2f184bf84a8
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/intent_optimize_5.f90
@@ -0,0 +1,24 @@
+! { dg-do run }
+! { dg-additional-options "-fno-inline -fno-ipa-modref -fdump-tree-optimized -fdump-tree-original" }
+!
+! PR fortran/105012
+! Check that the INTENT(OUT) attribute causes one clobber to be emitted in
+! the caller before the call to Y in the *.original dump, and the
+! initialization constant to be optimized away in the *.optimized dump,
+! despite the non-explicit interface if the subroutine with the INTENT(OUT)
+! is declared in the same file.
+
+SUBROUTINE Y (Z)
+      integer, intent(out) :: Z
+      Z = 42
+END SUBROUTINE Y
+PROGRAM TEST
+    integer :: X
+    X = 123456789
+    CALL Y (X)
+    if (X.ne.42) STOP 1
+END PROGRAM
+
+! { dg-final { scan-tree-dump-times "CLOBBER" 1 "original" } }
+! { dg-final { scan-tree-dump "x = {CLOBBER};" "original" } }
+! { dg-final { scan-tree-dump-not "123456789" "optimized" { target __OPTIMIZE__ } } }
-- 
2.35.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2 5/9] fortran: Support clobbering of reference variables [PR41453]
  2022-09-18 20:15 [PATCH v2 0/9] fortran: clobber fixes [PR41453] Mikael Morin
                   ` (3 preceding siblings ...)
  2022-09-18 20:15 ` [PATCH v2 4/9] fortran: Support clobbering with implicit interfaces [PR105012] Mikael Morin
@ 2022-09-18 20:15 ` Mikael Morin
  2022-09-18 20:15 ` [PATCH v2 6/9] fortran: Support clobbering of SAVE variables [PR87395] Mikael Morin
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Mikael Morin @ 2022-09-18 20:15 UTC (permalink / raw)
  To: gcc-patches, fortran

This adds support for clobbering of variables passed by reference,
when the reference is forwarded to a subroutine as actual argument
whose associated dummy has the INTENT(OUT) attribute.
This was explicitly disabled and enabling it seems to work, as
demonstrated by the new testcase.

	PR fortran/41453

gcc/fortran/ChangeLog:

	* trans-expr.cc (gfc_conv_procedure_call): Remove condition
	disabling clobber generation for dummy variables.  Remove
	obsolete comment.

gcc/testsuite/ChangeLog:

	* gfortran.dg/intent_optimize_6.f90: New test.
---
 gcc/fortran/trans-expr.cc                     |  4 ---
 .../gfortran.dg/intent_optimize_6.f90         | 34 +++++++++++++++++++
 2 files changed, 34 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/gfortran.dg/intent_optimize_6.f90

diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc
index 2301724729f..9b2832bdb26 100644
--- a/gcc/fortran/trans-expr.cc
+++ b/gcc/fortran/trans-expr.cc
@@ -6527,8 +6527,6 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym,
 			  && !e->symtree->n.sym->attr.dimension
 			  && !e->symtree->n.sym->attr.pointer
 			  && !e->symtree->n.sym->attr.allocatable
-			  /* See PR 41453.  */
-			  && !e->symtree->n.sym->attr.dummy
 			  /* FIXME - PR 87395 and PR 41453  */
 			  && e->symtree->n.sym->attr.save == SAVE_NONE
 			  && !e->symtree->n.sym->attr.associate_var
@@ -6538,8 +6536,6 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym,
 			  && !sym->attr.elemental)
 			{
 			  tree var;
-			  /* FIXME: This fails if var is passed by reference, see PR
-			     41453.  */
 			  var = build_fold_indirect_ref_loc (input_location,
 							     parmse.expr);
 			  tree clobber = build_clobber (TREE_TYPE (var));
diff --git a/gcc/testsuite/gfortran.dg/intent_optimize_6.f90 b/gcc/testsuite/gfortran.dg/intent_optimize_6.f90
new file mode 100644
index 00000000000..72fec3db583
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/intent_optimize_6.f90
@@ -0,0 +1,34 @@
+! { dg-do run }
+! { dg-additional-options "-fno-inline -fno-ipa-modref -fdump-tree-optimized -fdump-tree-original" }
+!
+! PR fortran/41453
+! Check that the INTENT(OUT) attribute causes one clobber to be emitted in
+! the caller before each call to FOO in the *.original dump, and the
+! initialization constant to be optimized away in the *.optimized dump,
+! in the case of an argument passed by reference to the caller.
+
+module x
+implicit none
+contains
+  subroutine foo(a)
+    integer(kind=4), intent(out) :: a
+    a = 42
+  end subroutine foo
+  subroutine bar(b)
+    integer(kind=4) :: b
+    b = 123456789
+    call foo(b)
+  end subroutine bar
+end module x
+
+program main
+  use x
+  implicit none
+  integer(kind=4) :: c
+  call bar(c)
+  if (c /= 42) stop 1
+end program main
+
+! { dg-final { scan-tree-dump-times "CLOBBER" 1 "original" } }
+! { dg-final { scan-tree-dump "\\*\\\(integer\\\(kind=4\\\) \\*\\\) b = {CLOBBER};" "original" } }
+! { dg-final { scan-tree-dump-not "123456789" "optimized" { target __OPTIMIZE__ } } }
-- 
2.35.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2 6/9] fortran: Support clobbering of SAVE variables [PR87395]
  2022-09-18 20:15 [PATCH v2 0/9] fortran: clobber fixes [PR41453] Mikael Morin
                   ` (4 preceding siblings ...)
  2022-09-18 20:15 ` [PATCH v2 5/9] fortran: Support clobbering of reference variables [PR41453] Mikael Morin
@ 2022-09-18 20:15 ` Mikael Morin
  2022-09-18 20:15 ` [PATCH v2 7/9] fortran: Support clobbering of ASSOCIATE variables [PR87397] Mikael Morin
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Mikael Morin @ 2022-09-18 20:15 UTC (permalink / raw)
  To: gcc-patches, fortran

This is in spirit a revert of:
r9-3032-gee7fb0588c6361b4d77337ab0f7527be64fcdde2

That commit added a condition to avoid generating ICE with clobbers
of variables with the SAVE attribute.
The test added at that point continues to pass if we remove that
condition now.

	PR fortran/87395
	PR fortran/41453

gcc/fortran/ChangeLog:

	* trans-expr.cc (gfc_conv_procedure_call): Remove condition
	on SAVE attribute guarding clobber generation.
---
 gcc/fortran/trans-expr.cc | 2 --
 1 file changed, 2 deletions(-)

diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc
index 9b2832bdb26..d169df44a71 100644
--- a/gcc/fortran/trans-expr.cc
+++ b/gcc/fortran/trans-expr.cc
@@ -6527,8 +6527,6 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym,
 			  && !e->symtree->n.sym->attr.dimension
 			  && !e->symtree->n.sym->attr.pointer
 			  && !e->symtree->n.sym->attr.allocatable
-			  /* FIXME - PR 87395 and PR 41453  */
-			  && e->symtree->n.sym->attr.save == SAVE_NONE
 			  && !e->symtree->n.sym->attr.associate_var
 			  && e->ts.type != BT_CHARACTER
 			  && e->ts.type != BT_DERIVED
-- 
2.35.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2 7/9] fortran: Support clobbering of ASSOCIATE variables [PR87397]
  2022-09-18 20:15 [PATCH v2 0/9] fortran: clobber fixes [PR41453] Mikael Morin
                   ` (5 preceding siblings ...)
  2022-09-18 20:15 ` [PATCH v2 6/9] fortran: Support clobbering of SAVE variables [PR87395] Mikael Morin
@ 2022-09-18 20:15 ` Mikael Morin
  2022-09-18 20:15 ` [PATCH v2 8/9] fortran: Support clobbering of allocatables and pointers [PR41453] Mikael Morin
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Mikael Morin @ 2022-09-18 20:15 UTC (permalink / raw)
  To: gcc-patches, fortran

This is in spirit a revert of:
r9-3051-gc109362313623d83fe0a5194bceaf994cf0c6ce0

That commit added a condition to avoid generating ICE with clobbers
of ASSOCIATE variables.
The test added at that point continues to pass if we remove that
condition now.

	PR fortran/87397
	PR fortran/41453

gcc/fortran/ChangeLog:

	* trans-expr.cc (gfc_conv_procedure_call): Remove condition
	disabling clobber generation for ASSOCIATE variables.
---
 gcc/fortran/trans-expr.cc | 1 -
 1 file changed, 1 deletion(-)

diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc
index d169df44a71..4491465c4d6 100644
--- a/gcc/fortran/trans-expr.cc
+++ b/gcc/fortran/trans-expr.cc
@@ -6527,7 +6527,6 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym,
 			  && !e->symtree->n.sym->attr.dimension
 			  && !e->symtree->n.sym->attr.pointer
 			  && !e->symtree->n.sym->attr.allocatable
-			  && !e->symtree->n.sym->attr.associate_var
 			  && e->ts.type != BT_CHARACTER
 			  && e->ts.type != BT_DERIVED
 			  && e->ts.type != BT_CLASS
-- 
2.35.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2 8/9] fortran: Support clobbering of allocatables and pointers [PR41453]
  2022-09-18 20:15 [PATCH v2 0/9] fortran: clobber fixes [PR41453] Mikael Morin
                   ` (6 preceding siblings ...)
  2022-09-18 20:15 ` [PATCH v2 7/9] fortran: Support clobbering of ASSOCIATE variables [PR87397] Mikael Morin
@ 2022-09-18 20:15 ` Mikael Morin
  2022-09-18 20:15 ` [PATCH v2 9/9] fortran: Support clobbering of derived types [PR41453] Mikael Morin
  2022-09-22 20:42 ` [PATCH v2 0/9] fortran: clobber fixes [PR41453] Harald Anlauf
  9 siblings, 0 replies; 14+ messages in thread
From: Mikael Morin @ 2022-09-18 20:15 UTC (permalink / raw)
  To: gcc-patches, fortran

This adds support for clobbering of allocatable and pointer scalar
variables passed as actual argument to a subroutine when the associated
dummy has the INTENT(OUT) attribute.
Support was explicitly disabled, but the clobber generation code seems
to support it well, as demonstrated by the newly added testcase.

	PR fortran/41453

gcc/fortran/ChangeLog:

	* trans-expr.cc (gfc_conv_procedure_call): Remove conditions
	on ALLOCATABLE and POINTER attributes guarding clobber
	generation.

gcc/testsuite/ChangeLog:

	* gfortran.dg/intent_optimize_7.f90: New test.
---
 gcc/fortran/trans-expr.cc                     |  2 -
 .../gfortran.dg/intent_optimize_7.f90         | 42 +++++++++++++++++++
 2 files changed, 42 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/gfortran.dg/intent_optimize_7.f90

diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc
index 4491465c4d6..ae685157e22 100644
--- a/gcc/fortran/trans-expr.cc
+++ b/gcc/fortran/trans-expr.cc
@@ -6525,8 +6525,6 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym,
 			  && e->symtree
 			  && e->symtree->n.sym
 			  && !e->symtree->n.sym->attr.dimension
-			  && !e->symtree->n.sym->attr.pointer
-			  && !e->symtree->n.sym->attr.allocatable
 			  && e->ts.type != BT_CHARACTER
 			  && e->ts.type != BT_DERIVED
 			  && e->ts.type != BT_CLASS
diff --git a/gcc/testsuite/gfortran.dg/intent_optimize_7.f90 b/gcc/testsuite/gfortran.dg/intent_optimize_7.f90
new file mode 100644
index 00000000000..0146dff4e20
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/intent_optimize_7.f90
@@ -0,0 +1,42 @@
+! { dg-do run }
+! { dg-additional-options "-fno-inline -fno-ipa-modref -fdump-tree-optimized -fdump-tree-original" }
+!
+! PR fortran/41453
+! Check that the INTENT(OUT) attribute causes one clobber to be emitted in
+! the caller before each call to FOO in the *.original dump, and the
+! initialization constants to be optimized away in the *.optimized dump,
+! in the case of scalar allocatables and pointers.
+
+module x
+implicit none
+contains
+  subroutine foo(a)
+    integer, intent(out) :: a
+    a = 42
+  end subroutine foo
+end module x
+
+program main
+  use x
+  implicit none
+  integer, allocatable :: ca
+  integer, target :: ct
+  integer, pointer :: cp
+
+  allocate(ca)
+  ca = 123456789
+  call foo(ca)
+  if (ca /= 42) stop 1
+  deallocate(ca)
+
+  ct = 987654321
+  cp => ct
+  call foo(cp)
+  if (ct /= 42) stop 2
+end program main
+
+! { dg-final { scan-tree-dump-times "CLOBBER" 2 "original" } }
+! { dg-final { scan-tree-dump "\\*ca = {CLOBBER};" "original" } }
+! { dg-final { scan-tree-dump "\\*cp = {CLOBBER};" "original" } }
+! { dg-final { scan-tree-dump-not "123456789" "optimized" { target __OPTIMIZE__ } } }
+! { dg-final { scan-tree-dump-not "987654321" "optimized" { target __OPTIMIZE__ } } }
-- 
2.35.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2 9/9] fortran: Support clobbering of derived types [PR41453]
  2022-09-18 20:15 [PATCH v2 0/9] fortran: clobber fixes [PR41453] Mikael Morin
                   ` (7 preceding siblings ...)
  2022-09-18 20:15 ` [PATCH v2 8/9] fortran: Support clobbering of allocatables and pointers [PR41453] Mikael Morin
@ 2022-09-18 20:15 ` Mikael Morin
  2022-09-22 20:42 ` [PATCH v2 0/9] fortran: clobber fixes [PR41453] Harald Anlauf
  9 siblings, 0 replies; 14+ messages in thread
From: Mikael Morin @ 2022-09-18 20:15 UTC (permalink / raw)
  To: gcc-patches, fortran

This is probably the most risky patch in the series.

A previous version of this patch allowing all exactly matching derived
types showed two regressions.  One of them uncovered PR106817 for which
I added a fix in this series, and for the other I have excluded
types with allocatable components from clobbering.

I have additionnally excluded finalizable types for similar reasons, and
parameterized derived type because they may not be constant-sized.

I hope we are safe for all the rest.

-- >8 --

This adds support for clobbering of non-polymorphic derived type
variables, when they are passed as actual argument whose associated
dummy has the INTENT(OUT) attribute.

We avoid to play with non-constant type sizes or class descriptors by
requiring that the types are derived (not class) and strictly matching,
and by excluding parameterized derived types.

Types that are used in the callee are also excluded: they are types with
allocatable components (the components will be deallocated), and
finalizable types or types with finalizable components (they will be
passed to finalization routines).

	PR fortran/41453

gcc/fortran/ChangeLog:

	* trans-expr.cc (gfc_conv_procedure_call): Allow strictly
	matching derived types.

gcc/testsuite/ChangeLog:

	* gfortran.dg/intent_optimize_8.f90: New test.
---
 gcc/fortran/trans-expr.cc                     | 18 ++++-
 .../gfortran.dg/intent_optimize_8.f90         | 66 +++++++++++++++++++
 2 files changed, 83 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gfortran.dg/intent_optimize_8.f90

diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc
index ae685157e22..16f14554db3 100644
--- a/gcc/fortran/trans-expr.cc
+++ b/gcc/fortran/trans-expr.cc
@@ -6526,8 +6526,24 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym,
 			  && e->symtree->n.sym
 			  && !e->symtree->n.sym->attr.dimension
 			  && e->ts.type != BT_CHARACTER
-			  && e->ts.type != BT_DERIVED
 			  && e->ts.type != BT_CLASS
+			  && (e->ts.type != BT_DERIVED
+			      || (dsym->ts.type == BT_DERIVED
+				  && e->ts.u.derived == dsym->ts.u.derived
+				  /* Types with allocatable components are
+				     excluded from clobbering because we need
+				     the unclobbered pointers to free the
+				     allocatable components in the callee.
+				     Same goes for finalizable types or types
+				     with finalizable components, we need to
+				     pass the unclobbered values to the
+				     finalization routines.
+				     For parameterized types, it's less clear
+				     but they may not have a constant size
+				     so better exclude them in any case.  */
+				  && !e->ts.u.derived->attr.alloc_comp
+				  && !e->ts.u.derived->attr.pdt_type
+				  && !gfc_is_finalizable (e->ts.u.derived, NULL)))
 			  && !sym->attr.elemental)
 			{
 			  tree var;
diff --git a/gcc/testsuite/gfortran.dg/intent_optimize_8.f90 b/gcc/testsuite/gfortran.dg/intent_optimize_8.f90
new file mode 100644
index 00000000000..d8bc1bb3b7b
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/intent_optimize_8.f90
@@ -0,0 +1,66 @@
+! { dg-do run }
+! { dg-additional-options "-fno-inline -fno-ipa-modref -fdump-tree-optimized -fdump-tree-original" }
+!
+! PR fortran/41453
+! Check that the INTENT(OUT) attribute causes in the case of non-polymorphic derived type arguments:
+!  - one clobber to be emitted in the caller before calls to FOO in the *.original dump,
+!  - no clobber to be emitted in the caller before calls to BAR in the *.original dump,
+!  - the initialization constants to be optimized away in the *.optimized dump.
+
+module x
+  implicit none
+  type :: t
+    integer :: c
+  end type t
+  type, extends(t) :: u
+    integer :: d
+  end type u
+contains
+  subroutine foo(a)
+    type(t), intent(out) :: a
+    a = t(42)
+  end subroutine foo
+  subroutine bar(b)
+    class(t), intent(out) :: b
+    b%c = 24
+  end subroutine bar
+end module x
+
+program main
+  use x
+  implicit none
+  type(t) :: tc
+  type(u) :: uc, ud
+  class(t), allocatable :: te, tf
+
+  tc = t(123456789)
+  call foo(tc)
+  if (tc%c /= 42) stop 1
+
+  uc = u(987654321, 0)
+  call foo(uc%t)
+  if (uc%c /= 42) stop 2
+  if (uc%d /= 0) stop 3
+
+  ud = u(11223344, 0)
+  call bar(ud)
+  if (ud%c /= 24) stop 4
+
+  te = t(55667788)
+  call foo(te)
+  if (te%c /= 42) stop 5
+
+  tf = t(99887766)
+  call bar(tf)
+  if (tf%c /= 24) stop 6
+
+end program main
+
+! We don't support class descriptors, neither derived type components, so there is a clobber for tc only;
+! no clobber for uc, ud, te, tf.
+! { dg-final { scan-tree-dump-times "CLOBBER" 1 "original" } }
+! { dg-final { scan-tree-dump "tc = {CLOBBER};" "original" } }
+
+! There is a clobber for tc, so we should manage to optimize away the associated initialization constant (but not other
+! initialization constants).
+! { dg-final { scan-tree-dump-not "123456789" "optimized" { target __OPTIMIZE__ } } }
-- 
2.35.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 0/9] fortran: clobber fixes [PR41453]
  2022-09-18 20:15 [PATCH v2 0/9] fortran: clobber fixes [PR41453] Mikael Morin
                   ` (8 preceding siblings ...)
  2022-09-18 20:15 ` [PATCH v2 9/9] fortran: Support clobbering of derived types [PR41453] Mikael Morin
@ 2022-09-22 20:42 ` Harald Anlauf
  2022-09-23  7:54   ` Mikael Morin
  9 siblings, 1 reply; 14+ messages in thread
From: Harald Anlauf @ 2022-09-22 20:42 UTC (permalink / raw)
  To: fortran; +Cc: gcc-patches

Hi Mikael,

thanks for this impressive series of patches.

Am 18.09.22 um 22:15 schrieb Mikael Morin via Fortran:
> Changes from v1:
>   - patch 9/10 (clobber subreferences) has been dropped.
>   - patch 10/10 (now 9/9): The test has been adjusted because some checks
>     were failing without the dropped patch.  Basically there are less
>     clobbers generated.
>   - patch 5: In the test, an explicit kind has been added to integers,
>     so that the dump match is not dependent on the
>     -fdefault-integer-8 option.
>   - patches 3, 4, 5, 8, and 10/10 (now 9/9): The tests have been renamed
>     so that they are numbered in increasing order.
> 
> The first patch is a refactoring moving the clobber generation in
> gfc_conv_procedure_call where it feels more appropriate.
> The second patch is a fix for the ICE originally motivating my work
> on this topic.
> The third patch is a fix for some wrong code issue discovered with an
> earlier version of this series.

This LGTM.  It also fixes a regression introduced with r9-3030 :-)
If you think that this set (1-3) is backportable, feel free to do so.

> The following patches are gradual condition loosenings to enable clobber
> generation in more and more cases.

Patches 4-8 all look fine.  Since they address missed optimizations,
they are probably something for mainline.

I was wondering if you could add a test for the change in patch 7
addressing the clobber generation for an associate-name, e.g. by
adding to testcase intent_optimize_7.f90 near the end:

   associate (av => ct)
     av = 111222333
     call foo(av)
   end associate
   if (ct /= 42) stop 3

plus the adjustments in the patterns.

Regarding patch 9, I do not see anything wrong with it, but then
independent eyes might see more.  I think it is ok for mainline
as is.

> Each patch has been tested through an incremental bootstrap and a
> partial testsuite run on fortran *intent* tests, and the whole lot has
> been run through the full fortran regression testsuite.
> OK for master?

Yes.

Thanks,
Harald

> 
> Harald Anlauf (1):
>    fortran: Support clobbering with implicit interfaces [PR105012]
> 
> Mikael Morin (8):
>    fortran: Move the clobber generation code
>    fortran: Fix invalid function decl clobber ICE [PR105012]
>    fortran: Move clobbers after evaluation of all arguments [PR106817]
>    fortran: Support clobbering of reference variables [PR41453]
>    fortran: Support clobbering of SAVE variables [PR87395]
>    fortran: Support clobbering of ASSOCIATE variables [PR87397]
>    fortran: Support clobbering of allocatables and pointers [PR41453]
>    fortran: Support clobbering of derived types [PR41453]
> 
>   gcc/fortran/trans-expr.cc                     | 81 ++++++++++++-------
>   gcc/fortran/trans.h                           |  3 +-
>   .../gfortran.dg/intent_optimize_4.f90         | 43 ++++++++++
>   .../gfortran.dg/intent_optimize_5.f90         | 24 ++++++
>   .../gfortran.dg/intent_optimize_6.f90         | 34 ++++++++
>   .../gfortran.dg/intent_optimize_7.f90         | 42 ++++++++++
>   .../gfortran.dg/intent_optimize_8.f90         | 66 +++++++++++++++
>   gcc/testsuite/gfortran.dg/intent_out_15.f90   | 27 +++++++
>   8 files changed, 290 insertions(+), 30 deletions(-)
>   create mode 100644 gcc/testsuite/gfortran.dg/intent_optimize_4.f90
>   create mode 100644 gcc/testsuite/gfortran.dg/intent_optimize_5.f90
>   create mode 100644 gcc/testsuite/gfortran.dg/intent_optimize_6.f90
>   create mode 100644 gcc/testsuite/gfortran.dg/intent_optimize_7.f90
>   create mode 100644 gcc/testsuite/gfortran.dg/intent_optimize_8.f90
>   create mode 100644 gcc/testsuite/gfortran.dg/intent_out_15.f90
> 




^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 0/9] fortran: clobber fixes [PR41453]
  2022-09-22 20:42 ` [PATCH v2 0/9] fortran: clobber fixes [PR41453] Harald Anlauf
@ 2022-09-23  7:54   ` Mikael Morin
  2022-09-25 12:53     ` Mikael Morin
  2022-10-10 18:58     ` Mikael Morin
  0 siblings, 2 replies; 14+ messages in thread
From: Mikael Morin @ 2022-09-23  7:54 UTC (permalink / raw)
  To: fortran; +Cc: gcc-patches

Le 22/09/2022 à 22:42, Harald Anlauf via Fortran a écrit :
> Hi Mikael,
> 
> thanks for this impressive series of patches.
> 
> Am 18.09.22 um 22:15 schrieb Mikael Morin via Fortran:
>> The first patch is a refactoring moving the clobber generation in
>> gfc_conv_procedure_call where it feels more appropriate.
>> The second patch is a fix for the ICE originally motivating my work
>> on this topic.
>> The third patch is a fix for some wrong code issue discovered with an
>> earlier version of this series.
> 
> This LGTM.  It also fixes a regression introduced with r9-3030 :-)
> If you think that this set (1-3) is backportable, feel free to do so.
> 
Yes, 2 and 3 are worth backporting, I will see how dependent they are on 1.

>> The following patches are gradual condition loosenings to enable clobber
>> generation in more and more cases.
> 
> Patches 4-8 all look fine.  Since they address missed optimizations,
> they are probably something for mainline.
> 
> I was wondering if you could add a test for the change in patch 7
> addressing the clobber generation for an associate-name, e.g. by
> adding to testcase intent_optimize_7.f90 near the end:
> 
>    associate (av => ct)
>      av = 111222333
>      call foo(av)
>    end associate
>    if (ct /= 42) stop 3
> 
> plus the adjustments in the patterns.
> 
Indeed, I didn't add a test because there was one already, but the 
existing test hasn't the check for clobber generation and store removal.
I prefer to create a new test though, so that the patch and the test 
come together, and the test for patch 8 is not encumbered with unrelated 
stuff.

By the way, the same could be said about patch 6.
I will create a test for that one as well.

> Regarding patch 9, I do not see anything wrong with it, but then
> independent eyes might see more.  I think it is ok for mainline
> as is.
> 
>> Each patch has been tested through an incremental bootstrap and a
>> partial testsuite run on fortran *intent* tests, and the whole lot has
>> been run through the full fortran regression testsuite.
>> OK for master?
> 
> Yes.
> 
Thanks for the review.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 0/9] fortran: clobber fixes [PR41453]
  2022-09-23  7:54   ` Mikael Morin
@ 2022-09-25 12:53     ` Mikael Morin
  2022-10-10 18:58     ` Mikael Morin
  1 sibling, 0 replies; 14+ messages in thread
From: Mikael Morin @ 2022-09-25 12:53 UTC (permalink / raw)
  To: fortran; +Cc: gcc-patches

Le 23/09/2022 à 09:54, Mikael Morin a écrit :
> Le 22/09/2022 à 22:42, Harald Anlauf via Fortran a écrit :
>> I was wondering if you could add a test for the change in patch 7
>> addressing the clobber generation for an associate-name, e.g. by
>> adding to testcase intent_optimize_7.f90 near the end:
>>
>>    associate (av => ct)
>>      av = 111222333
>>      call foo(av)
>>    end associate
>>    if (ct /= 42) stop 3
>>
>> plus the adjustments in the patterns.
>>
> Indeed, I didn't add a test because there was one already, but the 
> existing test hasn't the check for clobber generation and store removal.
> I prefer to create a new test though, so that the patch and the test 
> come together, and the test for patch 8 is not encumbered with unrelated 
> stuff.
> 
> By the way, the same could be said about patch 6.
> I will create a test for that one as well.
> 
Patches pushed:
https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=77bbf69d2981dafc2ef3e59bfbefb645d88bab9d

Changes from v2:
  - patches 6 and 7: A test for each has been added.
  - patches 8 and 9: The tests have been renumbered.
  - patches 6 and 7: The PR number used in the subject line has been
      changed, from the different regression PRs to the one optimization PR.
  - patches 5 and 8: The commit message has been modified: the commit 
the patch
      partly reverts is mentioned, and the associated PR number as well.
  - patch 7: The regression PR number this refers to has been changed.


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 0/9] fortran: clobber fixes [PR41453]
  2022-09-23  7:54   ` Mikael Morin
  2022-09-25 12:53     ` Mikael Morin
@ 2022-10-10 18:58     ` Mikael Morin
  1 sibling, 0 replies; 14+ messages in thread
From: Mikael Morin @ 2022-10-10 18:58 UTC (permalink / raw)
  To: fortran; +Cc: gcc-patches

Le 23/09/2022 à 09:54, Mikael Morin a écrit :
> Le 22/09/2022 à 22:42, Harald Anlauf via Fortran a écrit :
>> This LGTM.  It also fixes a regression introduced with r9-3030 :-)
>> If you think that this set (1-3) is backportable, feel free to do so.
>>
> Yes, 2 and 3 are worth backporting, I will see how dependent they are on 1.
> 
I'm going to backport 1-3 as you suggested, it's so much easier.

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2022-10-10 18:58 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-18 20:15 [PATCH v2 0/9] fortran: clobber fixes [PR41453] Mikael Morin
2022-09-18 20:15 ` [PATCH v2 1/9] fortran: Move the clobber generation code Mikael Morin
2022-09-18 20:15 ` [PATCH v2 2/9] fortran: Fix invalid function decl clobber ICE [PR105012] Mikael Morin
2022-09-18 20:15 ` [PATCH v2 3/9] fortran: Move clobbers after evaluation of all arguments [PR106817] Mikael Morin
2022-09-18 20:15 ` [PATCH v2 4/9] fortran: Support clobbering with implicit interfaces [PR105012] Mikael Morin
2022-09-18 20:15 ` [PATCH v2 5/9] fortran: Support clobbering of reference variables [PR41453] Mikael Morin
2022-09-18 20:15 ` [PATCH v2 6/9] fortran: Support clobbering of SAVE variables [PR87395] Mikael Morin
2022-09-18 20:15 ` [PATCH v2 7/9] fortran: Support clobbering of ASSOCIATE variables [PR87397] Mikael Morin
2022-09-18 20:15 ` [PATCH v2 8/9] fortran: Support clobbering of allocatables and pointers [PR41453] Mikael Morin
2022-09-18 20:15 ` [PATCH v2 9/9] fortran: Support clobbering of derived types [PR41453] Mikael Morin
2022-09-22 20:42 ` [PATCH v2 0/9] fortran: clobber fixes [PR41453] Harald Anlauf
2022-09-23  7:54   ` Mikael Morin
2022-09-25 12:53     ` Mikael Morin
2022-10-10 18:58     ` Mikael Morin

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).