[Patch, fortran] PR4773 - [4.6 Regression] Unnecessary temporaries increase the runtime for channel.f90 by ~70%

[Patch, fortran] PR4773 - [4.6 Regression] Unnecessary temporaries increase the runtime for channel.f90 by ~70%

[Paul Richard Thomas]

[-- Attachment #1: Type: text/plain, Size: 1341 bytes --]

Revision 161670 affected an optimization used for

array_lhs = array_valued_function (args...).

In order to prevent aliasing with the result, which is passed by
reference, a fairly conservative test was applied to generate a
temporary, if needed.  Unfortunately, this reduced the performance of
gfortran with some code; most notably with channel.f90.

This patch tests if the 'array_lhs' has ever been host associated, on
condition that 'array_valued_function' is contained, in addition to
checking that the result is not use-associated, a pointer or a target.
 If this is not the case, no temporary is generated.

Clearly some further optimization would be possible, if the
'array_lhs' were specifically not host associated to
'array_valued_function' OR ANY OF IT'S CALLEEs.   This latter
condition is rather difficult to implement, which is why I have not
even tried it.

Bootstrapped and regtested on RHEL5.3/i686 - OK for trunk and.....?

Cheers

Paul

2010-07-09  Paul Thomas  <pault@gcc.gnu.org>

	PR fortran/44773
	* trans-expr.c (arrayfunc_assign_needs_temporary): No temporary
	if the lhs has never been host associated, as well as not being
	use associated, a pointer or a target.
	* resolve.c (resolve_variable): Mark variables that are host
	associated.
	* gfortran.h: Add the host_assoc bit to the symbol_attribute
	structure.

[-- Attachment #2: submit.diff --]
[-- Type: text/x-patch, Size: 2041 bytes --]

Index: gcc/fortran/trans-expr.c
===================================================================
--- gcc/fortran/trans-expr.c	(revision 161983)
+++ gcc/fortran/trans-expr.c	(working copy)
@@ -4978,6 +4978,11 @@
       if (!expr2->value.function.esym->attr.contained)
 	return false;
 
+      /* A temporary is not needed if the lhs has never been host
+	 associated and the procedure is contained.  */
+      else if (!sym->attr.host_assoc)
+	return false;
+
       /* A temporary is not needed if the variable is local and not
 	 a pointer, a target or a result.  */
       if (sym->ns->parent
Index: gcc/fortran/gfortran.h
===================================================================
--- gcc/fortran/gfortran.h	(revision 161983)
+++ gcc/fortran/gfortran.h	(working copy)
@@ -682,7 +682,8 @@
     use_assoc:1,		/* Symbol has been use-associated.  */
     use_only:1,			/* Symbol has been use-associated, with ONLY.  */
     use_rename:1,		/* Symbol has been use-associated and renamed.  */
-    imported:1;			/* Symbol has been associated by IMPORT.  */
+    imported:1,			/* Symbol has been associated by IMPORT.  */
+    host_assoc:1;		/* Symbol has been host associated.  */ 
 
   unsigned in_namelist:1, in_common:1, in_equivalence:1;
   unsigned function:1, subroutine:1, procedure:1;
Index: gcc/fortran/resolve.c
===================================================================
--- gcc/fortran/resolve.c	(revision 161983)
+++ gcc/fortran/resolve.c	(working copy)
@@ -4772,6 +4772,15 @@
 	sym->entry_id = current_entry_id + 1;
     }
 
+  /* If a symbol has been host_associated mark it.  This is used latter,
+     to identify if aliasing is possible via host association.  */
+  if (sym->attr.flavor == FL_VARIABLE
+	&& gfc_current_ns->parent
+	&& (gfc_current_ns->parent == sym->ns
+	      || (gfc_current_ns->parent->parent
+		    && gfc_current_ns->parent->parent == sym->ns)))
+    sym->attr.host_assoc = 1;
+
 resolve_procedure:
   if (t == SUCCESS && resolve_procedure_expression (e) == FAILURE)
     t = FAILURE;

Re: [Patch, fortran] PR4773 - [4.6 Regression] Unnecessary temporaries increase the runtime for channel.f90 by ~70%

[Tobias Burnus]

On 07/09/2010 05:04 PM, Paul Richard Thomas wrote:
> Bootstrapped and regtested on RHEL5.3/i686 - OK for trunk and.....?
>   
I was wondering whether one should add a test case (-Warray-temporaries).

OK for the trunk - and for the branches, for which the array-temporary
fix has been applied to.

Tobias

> 2010-07-09  Paul Thomas  <pault@gcc.gnu.org>
>
> 	PR fortran/44773
> 	* trans-expr.c (arrayfunc_assign_needs_temporary): No temporary
> 	if the lhs has never been host associated, as well as not being
> 	use associated, a pointer or a target.
> 	* resolve.c (resolve_variable): Mark variables that are host
> 	associated.
> 	* gfortran.h: Add the host_assoc bit to the symbol_attribute
> 	structure.
>   

Re: [Patch, fortran] PR4773 - [4.6 Regression] Unnecessary temporaries increase the runtime for channel.f90 by ~70%

[Paul Richard Thomas]

Dear Tobias,

> I was wondering whether one should add a test case (-Warray-temporaries).

That's not a bad idea.  I was very reluctant to add any more testcases
that test the tree-dump-original output.  They tend to be a complete
pain in the behind from the maintenance point of view.  Will do!

>
> OK for the trunk - and for the branches, for which the array-temporary
> fix has been applied to.

I have gone back to 4.4.  In the latter, I applied the array-temporary
fix as well.  For some reason, applying the overall patch to 4.3
throws up a number of very strange looking regressions.  I am inclined
to close the book on this if I cannot see a quick fix tonight.  As I
say elsewhere, the underlying bug has been present in gfortran for
about ten years and it has only emerged now!

Thanks

Paul