| Message ID | 1279663274.29693.4.camel@linux-fd1f.site |
|---|---|
| State | New |
| Headers | show |
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. >
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
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
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
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); }