Bug 45777 - Alias analysis broken for arrays where LHS or RHS is a component ref
Summary: Alias analysis broken for arrays where LHS or RHS is a component ref
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: fortran (show other bugs)
Version: 4.6.0
: P3 normal
Target Milestone: ---
Assignee: Thomas Koenig
URL: http://gcc.gnu.org/ml/fortran/2011-01...
Keywords: wrong-code
Depends on:
Blocks: 32834 45586
  Show dependency treegraph
 
Reported: 2010-09-24 10:43 UTC by Joost VandeVondele
Modified: 2011-01-16 11:47 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work: 4.6.0
Known to fail: 4.3.0, 4.4.0, 4.5.2
Last reconfirmed: 2010-09-24 16:14:05


Attachments
Tentative patch (1.66 KB, patch)
2011-01-03 23:43 UTC, Thomas Koenig
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joost VandeVondele 2010-09-24 10:43:17 UTC
this one seems like a missing temporary:

MODULE M1
 TYPE T1
   INTEGER, DIMENSION(:), ALLOCATABLE :: data
 END TYPE T1
CONTAINS
 SUBROUTINE S1(T,d)
   INTEGER, DIMENSION(:), POINTER :: d
   TYPE(T1), POINTER :: T
    d(1:5)=T%data(3:7)
 END SUBROUTINE
END MODULE

USE M1
TYPE(T1), POINTER :: T
INTEGER, DIMENSION(:), POINTER :: d
ALLOCATE(T)
ALLOCATE(T%data(10))
T%data=(/(i,i=1,10)/)
d=>T%data(5:9)
CALL S1(T,d)
IF (ANY(d.NE.(/3,4,5,6,7/))) CALL ABORT()
DEALLOCATE(T%data)
DEALLOCATE(T)
END
Comment 1 Joost VandeVondele 2010-09-24 10:45:28 UTC
This was inspired by PR45586 (what does the restrict attribute on a component of a type mean?)
Comment 2 Tobias Burnus 2010-09-24 11:42:33 UTC
For what it is worth: gfortran 4.6 and 4.3.2, ifort 11.1, and pgf90 10.1 print
   3           4           3           4           3
and thus call abort().
Comment 3 Tobias Burnus 2010-09-24 16:14:05 UTC
With Crayftn, there is no abort.

Thinking a bit about the program, I believe it is valid, i.e. gfortran has a wrong-code bug.
Comment 4 Joost VandeVondele 2010-09-24 17:59:40 UTC
(In reply to comment #3)
> With Crayftn, there is no abort.

that also holds for NAG and g95, BTW.
 
> Thinking a bit about the program, I believe it is valid, i.e. gfortran has a
> wrong-code bug.

yes, I also think this testcase is valid. 

One little bit more tricky (in my mind) is the case where one declares T as 'TYPE(T1) :: T' in S1. It could be that that is not valid.
Comment 5 Tobias Burnus 2010-09-25 14:30:55 UTC
The issue seems to be how alias checking is implemented in trans-array.c:

gfc_could_be_alias (gfc_ss * lss, gfc_ss * rss)
[...]
  for (rref = rss->expr->ref; rref != rss->data.info.ref; rref = rref->next)
    {
      if (gfc_symbols_could_alias (rref->u.c.sym, lsym))
        return 1;
    }

While "rss->expr->symtree->n.sym" (= "rsym") has the pointer and target attribute, the element "rref->u.c.sym" usually has not.

Thus, the gfc_symbols_could_alias check succeeds (first argument is neither a pointer nor a target thus it cannot alias with the second argument, unless both symbol are the same); cf. symbol.c's

gfc_symbols_could_alias (gfc_symbol *lsym, gfc_symbol *rsym)
[...]
  if (lsym->attr.pointer
      && (rsym->attr.pointer || rsym->attr.allocatable || rsym->attr.target))
    return 1;
  if (lsym->attr.target && rsym->attr.pointer)
    return 1;
  if (lsym->attr.allocatable && rsym->attr.pointer)
    return 1;
[...]
  return 0;
Comment 6 Thomas Koenig 2010-11-30 16:40:10 UTC
At least one problem occurs because the typespec of rref->u.c.sym is not
filled out correctly when chasing  the refs on the rhs:

(gdb) p rref                                                                                        
No symbol "rref" in current context.                                                                
(gdb) up                                                                                            
#1  0x000000000054ceaa in gfc_could_be_alias (lss=<value optimized out>, rss=<value optimized out>) 
    at ../../trunk/gcc/fortran/trans-array.c:3501                                                   
3501          if (gfc_symbols_could_alias (rref->u.c.sym, lsym))                                    
(gdb) p rref
$6 = (gfc_ref *) 0x1458b90
(gdb) p rref->u.c.sym
$7 = (gfc_symbol *) 0x1454230
(gdb) p *(rref->u.c.sym)
$8 = {name = 0x7ffff5d11f98 "t1", module = 0x7ffff5d11f88 "m1", declared_at = {nextc = 0x14507f0,
    lb = 0x14507b0}, ts = {type = BT_UNKNOWN, kind = 0, u = {derived = 0x0, cl = 0x0, pad = 0},

(gdb) p rref->u.c.sym.ts
$10 = {type = BT_UNKNOWN, kind = 0, u = {derived = 0x0, cl = 0x0, pad = 0}, interface = 0x0,
  is_c_interop = 0, is_iso_c = 0, f90_type = BT_UNKNOWN, deferred = 0 '\000'}

Thus, gfc_symbols_could_alias has no chance of checking for type equivalence.
Comment 7 Thomas Koenig 2010-12-29 22:03:47 UTC
I think we should be doing the checking against the
typespec of the component.  The component looks reasonable:

p *(rref->u.c.component)
$19 = {name = 0x7ffff5d11fa8 "data", ts = {type = BT_INTEGER, kind = 4, u = {derived = 0x0, cl = 0x0,
      pad = 0},

but the sym just has

(gdb) p *(rref->u.c.sym)
$20 = {name = 0x7ffff5d11f98 "t1", module = 0x7ffff5d11f88 "m1", declared_at = {nextc = 0x148a8e0,
    lb = 0x148a8a0}, ts = {type = BT_UNKNOWN, kind = 0, u = {derived = 0x0, cl = 0x0, pad = 0},
    interface = 0x0, is_c_interop = 0, is_iso_c = 0, f90_type = BT_UNKNOWN, deferred = 0 '\000'}, attr = {
    allocatable = 0, dimension = 0, codimension = 0, external = 0, intrinsic = 0, optional = 0,
Comment 8 Mikael Morin 2010-12-30 14:56:27 UTC
(In reply to comment #7)
> I think we should be doing the checking against the
> typespec of the component.  The component looks reasonable:
> 
[...]
> 
> but the sym just has
> 
[...]

According to how it is used, it seems that u.c.sym is the component's derived type. And the dump you show doesn't contradict that (name="t1", empty type specification, ...). What's wrong ?
Comment 9 Thomas Koenig 2010-12-30 16:15:36 UTC
(In reply to comment #8)
> (In reply to comment #7)
> > I think we should be doing the checking against the
> > typespec of the component.  The component looks reasonable:
> > 
> [...]
> > 
> > but the sym just has
> > 
> [...]
> 
> According to how it is used, it seems that u.c.sym is the component's derived
> type. And the dump you show doesn't contradict that (name="t1", empty type
> specification, ...). What's wrong ?

The way we use it is wrong.

In gfc_could_be_alias, we call gfc_symbols_could_alias to check
possible aliases, like this:

  for (rref = rss->expr->ref; rref != rss->data.info.ref; rref = rref->next)
    {
      if (rref->type != REF_COMPONENT)
	break;

      if (gfc_symbols_could_alias (rref->u.c.sym, lsym))
	return 1;
    }

gfc_symbols_could_alias then checks, based on type, if the two
symbols could alias.  Unfortunately, the relevant type information is
lacking in rref->u.c.sym, so this check is a no-op.

For this test case, we need to perform a check on the type information
in the component, instead.
Comment 10 Thomas Koenig 2011-01-03 11:49:53 UTC
I'm working on it.
Comment 11 Thomas Koenig 2011-01-03 23:43:29 UTC
Created attachment 22887 [details]
Tentative patch

This could to the trick.
Comment 12 Thomas Koenig 2011-01-04 12:33:39 UTC
Patch here:

http://gcc.gnu.org/ml/fortran/2011-01/msg00014.html
Comment 13 Thomas Koenig 2011-01-08 09:38:17 UTC
Author: tkoenig
Date: Sat Jan  8 09:38:13 2011
New Revision: 168596

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=168596
Log:
2011-01-08  Thomas Koenig  <tkoenig@gcc.gnu.org>

	PR fortran/45777
	* symbol.c (gfc_symbols_could_alias):  Strip gfc_ prefix,
	make static and move in front of its only caller, to ...
	* trans-array.c (symbols_could_alias): ... here.
	Pass information about pointer and target status as
	arguments.  Allocatable arrays don't alias anything
	unless they have the POINTER attribute.
	(gfc_could_be_alias):  Keep track of pointer and target
	status when following references.  Also check if typespecs
	of components match those of other components or symbols.

2011-01-08  Thomas Koenig  <tkoenig@gcc.gnu.org>

	PR fortran/45777
	* gfortran.dg/dependency_39.f90:  New test.


Added:
    trunk/gcc/testsuite/gfortran.dg/dependency_39.f90
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/gfortran.h
    trunk/gcc/fortran/symbol.c
    trunk/gcc/fortran/trans-array.c
    trunk/gcc/testsuite/ChangeLog
Comment 14 Thomas Koenig 2011-01-08 10:04:00 UTC
Fixed on trunk, backport to 4.5 pending.
Comment 15 Thomas Koenig 2011-01-16 11:47:00 UTC
Author: tkoenig
Date: Sun Jan 16 11:46:55 2011
New Revision: 168851

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=168851
Log:
2011-01-16  Thomas Koenig  <tkoenig@gcc.gnu.org>

	Backport from trunk
	PR fortran/45777
	* symbol.c (gfc_symbols_could_alias):  Strip gfc_ prefix,
	make static and move in front of its only caller, to ...
	* trans-array.c (symbols_could_alias): ... here.
	Pass information about pointer and target status as
	arguments.  Allocatable arrays don't alias anything
	unless they have the POINTER attribute.
	(gfc_could_be_alias):  Keep track of pointer and target
	status when following references.  Also check if typespecs
	of components match those of other components or symbols.
	* gfortran.h:  Remove prototype for gfc_symbols_could_alias.

2011-01-16  Thomas Koenig  <tkoenig@gcc.gnu.org>

	Backport from trunk
	PR fortran/45777
	* gfortran.dg/dependency_39.f90:  New test.


Added:
    branches/gcc-4_5-branch/gcc/testsuite/gfortran.dg/dependency_39.f90
Modified:
    branches/gcc-4_5-branch/gcc/fortran/ChangeLog
    branches/gcc-4_5-branch/gcc/fortran/gfortran.h
    branches/gcc-4_5-branch/gcc/fortran/symbol.c
    branches/gcc-4_5-branch/gcc/fortran/trans-array.c
Comment 16 Thomas Koenig 2011-01-16 11:47:31 UTC
Fixed on 4.5 as well, closing.