Patchwork [fortran] PR 40628, front-end optimization pass

login
register
mail settings
Submitter Thomas Koenig
Date July 20, 2010, 10:01 p.m.
Message ID <1279663274.29693.4.camel@linux-fd1f.site>
Download mbox | patch
Permalink /patch/59377/
State New
Headers show

Comments

Thomas Koenig - July 20, 2010, 10:01 p.m.
Well, here is an updated version of the patch.

I have called the new file (mostly unchanged) frontend-passes.c, because
my gdb gets confused about having two files called passes.c.

I have also changed the place where the gfc_run_passes is called to
resolve.c, as pault had suggested on IRC.

Regression-tested, only allocate_with_typespec.f90 failed (which I also
saw on gcc-testresults).

OK?

	Thomas


2010-07-20  Thomas Koenig  <tkoenig@gcc.gnu.org>

	* Make-lang.in:  Add fortran/frontend-passes.o.
	* gfortran.h:  Add prototype for gfc_run_passes.
	* resolve.c (gfc_resolve):  Call gfc_run_passes.
	* frontend-passes.c:  New file.

2010-0717  Thomas Koenig  <tkoenig@gcc.gnu.org>

	* trim_optimize_1.f90:  New test.
	* character_comparision_1.f90:  New test.
Tobias Burnus - July 25, 2010, 3:52 p.m.
Thomas Koenig wrote:
> Well, here is an updated version of the patch.
>
> I have also changed the place where the gfc_run_passes is called to
> resolve.c, as pault had suggested on IRC.
>
> Regression-tested, only allocate_with_typespec.f90 failed (which I also
> saw on gcc-testresults).
> OK?
>   
OK. Thanks for the patch - and sorry for the late review.

Nits:

Index: gfortran.h
+/* passes.c */
+
+void gfc_run_passes (gfc_namespace *);


Update the filename in the comment.

! { dg-final { scan-tree-dump-times "memmove" 2 "original" } }

Can you add:
! { dg-final { scan-tree-dump-times "string_trim" 0 "original" } }


  /* Check for direct comparison between identical variables.
     TODO: Handle cases with identical refs.  */
  if (op1->expr_type == EXPR_VARIABLE
      && op2->expr_type == EXPR_VARIABLE
      && op1->symtree == op2->symtree
      && op1->ref == NULL && op2->ref == NULL
      && op1->ts.type != BT_REAL && op2->ts.type != BT_REAL
      && op1->ts.type != BT_COMPLEX && op2->ts.type !=BT_COMPLEX)


Is there a reason that you do not include REAL and COMPLEX variables,
but everything else? (characters, derived types, polymorphic types
(class), integer, Hollerith, ...). Especially, as derived types can also
contains real/complex variables ;-)

It seems to be save to do this comparison also for REAL and COMPLEX.

Tobias

> 2010-07-20  Thomas Koenig  <tkoenig@gcc.gnu.org>
>
> 	* Make-lang.in:  Add fortran/frontend-passes.o.
> 	* gfortran.h:  Add prototype for gfc_run_passes.
> 	* resolve.c (gfc_resolve):  Call gfc_run_passes.
> 	* frontend-passes.c:  New file.
>
> 2010-0717  Thomas Koenig  <tkoenig@gcc.gnu.org>
>
> 	* trim_optimize_1.f90:  New test.
> 	* character_comparision_1.f90:  New test.
>
Thomas Koenig - July 25, 2010, 7:35 p.m.
Tobias Burnus wrote:
> Thomas Koenig wrote:
> > Well, here is an updated version of the patch.
> >
> > I have also changed the place where the gfc_run_passes is called to
> > resolve.c, as pault had suggested on IRC.
> >
> > Regression-tested, only allocate_with_typespec.f90 failed (which I also
> > saw on gcc-testresults).
> > OK?
> >   
> OK. Thanks for the patch - and sorry for the late review.

Thanks a lot!

> Nits:
> 
> Index: gfortran.h
> +/* passes.c */
> +
> +void gfc_run_passes (gfc_namespace *);
> 
> 
> Update the filename in the comment.

Done.

> ! { dg-final { scan-tree-dump-times "memmove" 2 "original" } }
> 
> Can you add:
> ! { dg-final { scan-tree-dump-times "string_trim" 0 "original" } }

Done.
> 
>   /* Check for direct comparison between identical variables.
>      TODO: Handle cases with identical refs.  */
>   if (op1->expr_type == EXPR_VARIABLE
>       && op2->expr_type == EXPR_VARIABLE
>       && op1->symtree == op2->symtree
>       && op1->ref == NULL && op2->ref == NULL
>       && op1->ts.type != BT_REAL && op2->ts.type != BT_REAL
>       && op1->ts.type != BT_COMPLEX && op2->ts.type !=BT_COMPLEX)

> Is there a reason that you do not include REAL and COMPLEX variables,
> but everything else? (characters, derived types, polymorphic types
> (class), integer, Hollerith, ...). Especially, as derived types can also
> contains real/complex variables ;-)

The main reason is that I don't want to invalidate the

  if (a /= a)

idiom for checking for NANs.  This would also cause a few testsuite
failures, where we do exactly this.

Again, thanks for the review!

├ťbertrage Daten ........
Revision 162519 ├╝bertragen.

	Thomas
Paolo Bonzini - July 25, 2010, 9:15 p.m.
On 07/25/2010 09:35 PM, Thomas Koenig wrote:
>>    /* Check for direct comparison between identical variables.
>>       TODO: Handle cases with identical refs.  */
>>    if (op1->expr_type == EXPR_VARIABLE
>>        &&  op2->expr_type == EXPR_VARIABLE
>>        &&  op1->symtree == op2->symtree
>>        &&  op1->ref == NULL&&  op2->ref == NULL
>>        &&  op1->ts.type != BT_REAL&&  op2->ts.type != BT_REAL
>>        &&  op1->ts.type != BT_COMPLEX&&  op2->ts.type !=BT_COMPLEX)
>
>> Is there a reason that you do not include REAL and COMPLEX variables,
>> but everything else? (characters, derived types, polymorphic types
>> (class), integer, Hollerith, ...). Especially, as derived types can also
>> contains real/complex variables ;-)
>
> The main reason is that I don't want to invalidate the
>
>    if (a /= a)
>
> idiom for checking for NANs.  This would also cause a few testsuite
> failures, where we do exactly this.

You could test HONOR_NANS for that, though I don't know whether modes 
are already available in the Fortran front-end (failing that, there is 
flag_finite_math_only).

What is the default definition of equality for derived types, if any? 
Would "if (d /= d)" change meaning if d contains a single real?

Paolo
Thomas Koenig - July 26, 2010, 5:14 p.m.
Paolo Bonzini wrote:

> You could test HONOR_NANS for that, though I don't know whether modes 
> are already available in the Fortran front-end (failing that, there
> is 
> flag_finite_math_only).

That's right, I'll probably do this in a future patch.

> What is the default definition of equality for derived types, if any? 

There is no intrinsic equality operator for derived types.  You can
define your own, but this will end up as the function actually being
invoked in the gfc_* structures.

	Thomas

Patch

Index: Make-lang.in
===================================================================
--- Make-lang.in	(Revision 162346)
+++ Make-lang.in	(Arbeitskopie)
@@ -66,7 +66,7 @@ 
     fortran/trans.o fortran/trans-array.o fortran/trans-common.o \
     fortran/trans-const.o fortran/trans-decl.o fortran/trans-expr.o \
     fortran/trans-intrinsic.o fortran/trans-io.o fortran/trans-openmp.o \
-    fortran/trans-stmt.o fortran/trans-types.o
+    fortran/trans-stmt.o fortran/trans-types.o fortran/frontend-passes.o
 
 fortran_OBJS = $(F95_OBJS) gfortranspec.o
 
Index: gfortran.h
===================================================================
--- gfortran.h	(Revision 162346)
+++ gfortran.h	(Arbeitskopie)
@@ -2831,4 +2831,8 @@ 
 
 #define CLASS_DATA(sym) sym->ts.u.derived->components
 
+/* passes.c */
+
+void gfc_run_passes (gfc_namespace *);
+
 #endif /* GCC_GFORTRAN_H  */
Index: resolve.c
===================================================================
--- resolve.c	(Revision 162346)
+++ resolve.c	(Arbeitskopie)
@@ -13068,4 +13068,6 @@ 
   gfc_current_ns = old_ns;
   cs_base = old_cs_base;
   ns->resolved = 1;
+
+  gfc_run_passes (ns);
 }