This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[Patch, fortran] PR36932,3 - unnecessary temporaries
- From: Paul Richard Thomas <paul dot richard dot thomas at gmail dot com>
- To: fortran at gcc dot gnu dot org, gcc-patches <gcc-patches at gcc dot gnu dot org>
- Date: Sat, 13 Feb 2010 18:36:27 +0100
- Subject: [Patch, fortran] PR36932,3 - unnecessary temporaries
Having committed the patch for PR41113,7 to trunk (r156749), I thought
that I should have a stab at PR36932 and 3, as suggested by Tobias.
In fact, PR36932 is half fixed by the patch just committed. The
attached patch attacks the other half by addressing the somewhat
over-conservative alias checking in
dependency.c(check_data_pointer_types).
The relaxation of the alias checking is done by testing if the type of
a potential target is the same as potential pointer references. For
example,
'structure_pointer%real_array_component' cannot alias with 'real_array'
whereas
'structure_pointer%real_array_pointer_component' can.
I have not attempted to relax the checking for nested derived types
nor for the case of derived type and derived type. These could be
done if it is thought to be useful; it's just somewhat tedious!
I would be grateful if the reviewer were to check the logic in
check_data_pointer_types just to make sure that no aliasing cases slip
through the net.
Bootstrapped and regtested on FC9/x86_64 - OK for trunk?
Paul
2010-02-13 Paul Thomas <pault@gcc.gnu.org>
PR fortran/36932
PR fortran/36933
* dependency.c (gfc_check_argument_var_dependency): Use enum
value instead of arithmetic vaue for 'elemental'.
(check_data_pointer_types): New function.
(gfc_check_dependency): Call check_data_pointer_types.
2010-02-13 Paul Thomas <pault@gcc.gnu.org>
PR fortran/36932
PR fortran/36933
* gfortran.dg/dependency_26.f90: New test.
Index: gcc/fortran/dependency.c
===================================================================
*** gcc/fortran/dependency.c (revision 156748)
--- gcc/fortran/dependency.c (working copy)
*************** gfc_check_argument_var_dependency (gfc_e
*** 467,473 ****
/* In case of elemental subroutines, there is no dependency
between two same-range array references. */
if (gfc_ref_needs_temporary_p (expr->ref)
! || gfc_check_dependency (var, expr, !elemental))
{
if (elemental == ELEM_DONT_CHECK_VARIABLE)
{
--- 467,473 ----
/* In case of elemental subroutines, there is no dependency
between two same-range array references. */
if (gfc_ref_needs_temporary_p (expr->ref)
! || gfc_check_dependency (var, expr, elemental == NOT_ELEMENTAL))
{
if (elemental == ELEM_DONT_CHECK_VARIABLE)
{
*************** gfc_are_equivalenced_arrays (gfc_expr *e
*** 677,682 ****
--- 677,747 ----
}
+ /* Return true if there is no possibility of aliasing because of a type
+ mismatch between all the possible pointer references and the
+ potential target. */
+ static bool
+ check_data_pointer_types (gfc_expr *expr1, gfc_expr *expr2)
+ {
+ gfc_component *cm1;
+ gfc_component *cm2;
+ gfc_symbol *sym1;
+ gfc_symbol *sym2;
+
+ if (expr1->expr_type != EXPR_VARIABLE
+ || expr1->expr_type != EXPR_VARIABLE)
+ return false;
+
+ sym1 = expr1->symtree->n.sym;
+ sym2 = expr2->symtree->n.sym;
+
+ /* Keep it simple for now. */
+ if (sym1->ts.type == BT_DERIVED && sym2->ts.type == BT_DERIVED)
+ return false;
+
+ if (sym1->attr.pointer)
+ {
+ if (gfc_compare_types (&sym1->ts, &expr2->symtree->n.sym->ts))
+ return false;
+ }
+
+ if (sym1->ts.type == BT_DERIVED)
+ {
+ for (cm1 = sym1->ts.u.derived->components; cm1; cm1 = cm1->next)
+ {
+ /* Again, keep it simple. */
+ if (cm1->ts.type == BT_DERIVED)
+ return false;
+
+ if ((sym2->attr.pointer || cm1->attr.pointer)
+ && gfc_compare_types (&cm1->ts, &sym2->ts))
+ return false;
+ }
+ }
+
+ if (sym2->attr.pointer)
+ {
+ if (gfc_compare_types (&sym2->ts, &expr1->symtree->n.sym->ts))
+ return false;
+ }
+
+ if (sym2->ts.type == BT_DERIVED)
+ {
+ for (cm2 = sym2->ts.u.derived->components; cm2; cm2 = cm2->next)
+ {
+ /* Again, keep it simple. */
+ if (cm2->ts.type == BT_DERIVED)
+ return false;
+
+ if ((sym1->attr.pointer || cm2->attr.pointer)
+ && gfc_compare_types (&cm2->ts, &sym1->ts))
+ return false;
+ }
+ }
+
+ return true;
+ }
+
/* Return true if the statement body redefines the condition. Returns
true if expr2 depends on expr1. expr1 should be a single term
suitable for the lhs of an assignment. The IDENTICAL flag indicates
*************** gfc_check_dependency (gfc_expr *expr1, g
*** 726,732 ****
/* If either variable is a pointer, assume the worst. */
/* TODO: -fassume-no-pointer-aliasing */
if (gfc_is_data_pointer (expr1) || gfc_is_data_pointer (expr2))
! return 1;
/* Otherwise distinct symbols have no dependencies. */
return 0;
--- 791,802 ----
/* If either variable is a pointer, assume the worst. */
/* TODO: -fassume-no-pointer-aliasing */
if (gfc_is_data_pointer (expr1) || gfc_is_data_pointer (expr2))
! {
! if (check_data_pointer_types (expr1, expr2))
! return 0;
!
! return 1;
! }
/* Otherwise distinct symbols have no dependencies. */
return 0;
Index: gcc/testsuite/gfortran.dg/dependency_26.f90
===================================================================
*** gcc/testsuite/gfortran.dg/dependency_26.f90 (revision 0)
--- gcc/testsuite/gfortran.dg/dependency_26.f90 (revision 0)
***************
*** 0 ****
--- 1,32 ----
+ ! { dg-do compile }
+ ! { dg-options "-fdump-tree-original" }
+ !
+ ! Test the fix for PR36932 and PR36933, in which unnecessary
+ ! temporaries were being generated.
+ !
+ ! Contributed by Joost VandeVondele <jv244@cam.ac.uk>
+ !
+ MODULE M1
+ IMPLICIT NONE
+ TYPE cell_type
+ REAL :: h(3,3)
+ END TYPE
+ CONTAINS
+ SUBROUTINE S1(cell)
+ TYPE(cell_type), POINTER :: cell
+ REAL :: a(3)
+ REAL :: b(3) = [1, 2, 3]
+ a=MATMUL(cell%h,b)
+ if (ANY (INT (a) .ne. [30, 36, 42])) call abort
+ END SUBROUTINE S1
+ END MODULE M1
+
+ use M1
+ TYPE(cell_type), POINTER :: cell
+ allocate (cell)
+ cell%h = reshape ([(real(i), i = 1, 9)], [3, 3])
+ call s1 (cell)
+ end
+ ! { dg-final { cleanup-modules "M1" } }
+ ! { dg-final { scan-tree-dump-times "&a" 1 "original" } }
+ ! { dg-final { cleanup-tree-dump "original" } }