Bug 38913 - Fortran does not set TYPE_CANONICAL properly
Summary: Fortran does not set TYPE_CANONICAL properly
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: fortran (show other bugs)
Version: 4.4.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: wrong-code
: 38965 (view as bug list)
Depends on:
Blocks: 42361 40011
  Show dependency treegraph
 
Reported: 2009-01-19 15:37 UTC by Richard Biener
Modified: 2011-07-24 18:47 UTC (History)
6 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2009-12-09 11:49:44


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Richard Biener 2009-01-19 15:37:10 UTC
TYPE_CANONICAL should specify a canonical type node (equivalent for type comparison for example during TBAA) for each type declaration.  Failing to do
so causes TBAA pessimizations.

The frontend also does not properly identify equivalent types, which is
a correctness issue.  Consider (reduced from polyhedron protein)

subroutine check (string1, string2)
character (len=1) :: string1, string2
if (string1(1:1) /= string2(1:1)) call abort
end subroutine check
program foo
  character (len=1) :: str, str2
  str(1:1) = ' '
  str2(1:1) = convert_lower_case(str(1:1))
  call check(str, str2)

  contains
          function convert_lower_case (input_string) result (output_string)
          character (len=1) :: input_string, output_string
          output_string = input_string
          end function convert_lower_case

end program

the character array type used in the inline function convert_lower_case is
not connected to the character array type used in the main function.  This
causes the alias-oracle on the alias-improvements branch to consider
str.293[0] and (*str.327_132)[1]{lb: 1 sz: 1} to be non-aliasing even
if str.327_132 is initialized from str.293 via
str.327_132 = (character(kind=1)[1:1] *) &str.293; because the type used
in that conversion is from the inline function and is not equal to that
of std.293.

I believe that boils down to a similar issue as the one-decl for each
function thing.  Either GFortran has to use -fno-strict-aliasing or
needs to properly use canonical types.
Comment 2 Paul Thomas 2009-01-21 20:22:13 UTC
(In reply to comment #1)
> http://gcc.gnu.org/ml/gcc-patches/2009-01/msg00937.html
> 

Richard,

I am not sure that any of the gfortran developers have the skills to deal with this.  I read the words about canonical types and more or less understand them but think that it would require more time than I have to implement what is needed.

You have set the PR as P3/blocker - does that imply that you intend to attack the problem or should we approach Jakub to do the business?

Cheers

Paul  
Comment 3 Richard Biener 2009-01-21 20:50:06 UTC
I was able to work around the issue sofar, but more analysis is still required.
I will see if I can find some time for that.
Comment 4 Daniel Franke 2009-01-25 15:25:22 UTC
*** Bug 38965 has been marked as a duplicate of this bug. ***
Comment 5 Paul Thomas 2009-02-06 13:44:58 UTC
I guess that since Richard says that it's a problem, we had better confirm it:-)

Paul
Comment 6 Joost VandeVondele 2009-02-07 15:32:44 UTC
(In reply to comment #5)
> I guess that since Richard says that it's a problem, we had better confirm
> it:-)

Do we need a bugzilla field 'confirmatio ad verecundiam' ;-)
Comment 7 rguenther@suse.de 2009-02-07 18:49:57 UTC
Subject: Re:  Fortran does not set TYPE_CANONICAL
 properly

On Sat, 7 Feb 2009, jv244 at cam dot ac dot uk wrote:

> ------- Comment #6 from jv244 at cam dot ac dot uk  2009-02-07 15:32 -------
> (In reply to comment #5)
> > I guess that since Richard says that it's a problem, we had better confirm
> > it:-)
> 
> Do we need a bugzilla field 'confirmatio ad verecundiam' ;-)

Haha ;)

OTOH I am no longer convinced it is a real problem, but I also only have
some F77 skills, so no testcases with whatever Fortran calls "structs"
from me ;)

Richard.
Comment 8 Richard Biener 2009-06-04 10:55:21 UTC
The whole-file patches now expose this problem.

! { dg-do run }
! Test the fix for PR34438, in which default initializers
! forced the derived type to be static; ie. initialized once
! during the lifetime of the programme.  Instead, they should
! be initialized each time they come into scope.
!
module demo
   type myint
     integer :: bar = 42
   end type myint
end module demo

! ...who came up with this one too.
subroutine func (retval2)
  use demo
  type(myint) :: foo2 = myint (77)
  type(myint) :: retval2
  retval2 = foo2
  foo2%bar = 999
end subroutine func

subroutine other
  use demo
  interface
    subroutine func(rv2)
      use demo
      type(myint) :: rv2
   end subroutine func
  end interface
  type(myint) :: val2
  call func (val2)
  if ((val2%bar .ne. 77_4)) call abort ()

end subroutine other

! Run both tests.
  call other
 end
! { dg-final { cleanup-modules "demo M1" } }


with -O2 -fwhole-file we inline func into other and get

other ()
{
  static struct myint foo2D.1539 = {.barD.1534=77};
  static struct myint foo2D.1539 = {.barD.1534=77};
  struct myint & retval2D.1572;
  struct myint val2D.1544;
  integer(kind=4)D.8 D.1547;

<bb 2>:
  val2D.1544.barD.1542 = 42;
  retval2D.1572_6 = (struct myint &) &val2D.1544;
  *retval2D.1572_6 = foo2D.1539;
  foo2D.1539.barD.1534 = 999;
  D.1547_1 = val2D.1544.barD.1542;
  if (D.1547_1 != 77)

which looks good sofar.  But the store *retval2D.1572_6 = foo2D.1539 is
through a different struct myint than the load from val2D.1544.barD.1542
so we optimize the load to 42 -- the only visible aliasing store.

 <var_decl 0x7ffff7fcbc80 val2
    type <record_type 0x7ffff7fd1180 myint SI
        size <integer_cst 0x7ffff7ed4a50 constant 32>
        unit size <integer_cst 0x7ffff7ed46c0 constant 4>
        align 32 symtab 0 alias set 2 canonical type 0x7ffff7fd1180
        fields <field_decl 0x7ffff7fcbbe0 bar type <integer_type 0x7ffff7ee1540 integer(kind=4)>
            SI file t.f90 line 23 col 0 size <integer_cst 0x7ffff7ed4a50 32> unit size <integer_cst 0x7ffff7ed46c0 4>
            align 32 offset_align 128
            offset <integer_cst 0x7ffff7ed46f0 constant 0>
            bit offset <integer_cst 0x7ffff7ed4f30 constant 0> context <record_type 0x7ffff7fd1180 myint>>
        pointer_to_this <pointer_type 0x7ffff7fd1300> chain <type_decl 0x7ffff7fd1240 D.1543>>
    addressable used SI file t.f90 line 30 col 0 size <integer_cst 0x7ffff7ed4a50 32> unit size <integer_cst 0x7ffff7ed46c0 4>
    align 32 context <function_decl 0x7ffff5f3f200 other>>

and

 <indirect_ref 0x7ffff7ff9dc0
    type <record_type 0x7ffff7fced80 myint SI
        size <integer_cst 0x7ffff7ed4a50 constant 32>
        unit size <integer_cst 0x7ffff7ed46c0 constant 4>
        align 32 symtab 0 alias set 4 canonical type 0x7ffff7fced80
        fields <field_decl 0x7ffff7fcbaa0 bar type <integer_type 0x7ffff7ee1540 integer(kind=4)>
            SI file t.f90 line 15 col 0 size <integer_cst 0x7ffff7ed4a50 32> unit size <integer_cst 0x7ffff7ed46c0 4>
            align 32 offset_align 128
            offset <integer_cst 0x7ffff7ed46f0 constant 0>
            bit offset <integer_cst 0x7ffff7ed4f30 constant 0> context <record_type 0x7ffff7fced80 myint>>
        reference_to_this <reference_type 0x7ffff7fcef00> chain <type_decl 0x7ffff7fcee40 D.1535>>
   
    arg 0 <ssa_name 0x7ffff5f4a060
        type <reference_type 0x7ffff7fcef00 type <record_type 0x7ffff7fced80 myint>
            public unsigned DI
            size <integer_cst 0x7ffff7ed4b40 constant 64>
            unit size <integer_cst 0x7ffff7ed4b70 constant 8>
            align 64 symtab 0 alias set -1 canonical type 0x7ffff7fcef00>
        visited var <var_decl 0x7ffff5f46780 retval2>def_stmt retval2_6 = (struct myint &) &val2;

        version 6
        ptr-info 0x7ffff7f9ac10>>

so the Frontend misses proper type unification.
Comment 9 Tobias Burnus 2009-06-04 11:47:33 UTC
Cf. also thread at http://gcc.gnu.org/ml/fortran/2009-06/msg00057.html

(Maybe if -fwhole-file is the permanent default and this problem is fixed, the hack at http://gcc.gnu.org/ml/gcc-patches/2009-01/msg00937.html can be removed ...)
Comment 10 rguenther@suse.de 2009-06-04 11:49:34 UTC
Subject: Re:  Fortran does not set TYPE_CANONICAL
 properly

On Thu, 4 Jun 2009, burnus at gcc dot gnu dot org wrote:

> ------- Comment #9 from burnus at gcc dot gnu dot org  2009-06-04 11:47 -------
> Cf. also thread at http://gcc.gnu.org/ml/fortran/2009-06/msg00057.html
> 
> (Maybe if -fwhole-file is the permanent default and this problem is fixed, the
> hack at http://gcc.gnu.org/ml/gcc-patches/2009-01/msg00937.html can be removed
> ...)

That hack is already gone ... ;)

Richard.
Comment 11 Tobias Burnus 2009-06-04 12:51:56 UTC
(In reply to comment #10)
> That hack is already gone ... ;)
The truck hack yes, the question is whether one can also do something about the following? Or is this a wider problem?

  /* ???  Array types are not properly unified in all cases as we have
     spurious changes in the index types for example.  Removing this
     causes all sorts of problems with the Fortran frontend.  */
  if (TREE_CODE (type1) == ARRAY_TYPE
      && TREE_CODE (type2) == ARRAY_TYPE)
    return -1;
Comment 12 rguenther@suse.de 2009-06-04 13:39:33 UTC
Subject: Re:  Fortran does not set TYPE_CANONICAL
 properly

On Thu, 4 Jun 2009, burnus at gcc dot gnu dot org wrote:

> ------- Comment #11 from burnus at gcc dot gnu dot org  2009-06-04 12:51 -------
> (In reply to comment #10)
> > That hack is already gone ... ;)
> The truck hack yes, the question is whether one can also do something about the
> following? Or is this a wider problem?
> 
>   /* ???  Array types are not properly unified in all cases as we have
>      spurious changes in the index types for example.  Removing this
>      causes all sorts of problems with the Fortran frontend.  */
>   if (TREE_CODE (type1) == ARRAY_TYPE
>       && TREE_CODE (type2) == ARRAY_TYPE)
>     return -1;

It's a wider problem.  It's char[1:1] vs. char where the FE is
very inconsistent, also char[1:] vs. char[1:1], etc.

Probably not worth fixing.  Maybe after everything else is ...

Richard.
Comment 13 Richard Biener 2009-06-28 16:56:13 UTC
Similar case, from reduced import.f90:

subroutine bar(x)
  type myType
    sequence
    integer :: i
  end type myType
  type(myType) :: x
  x%i = 5
end subroutine bar

program foo
  integer, parameter :: dp = 8
  type myType
    sequence
    integer :: i
  end type myType
  interface
    subroutine bar(x)
      import
      type(myType) :: x
    end subroutine bar
  end interface

  type(myType) :: y
  y%i = 2
  call bar(y)
  if(y%i /= 5) call abort()
end program foo
Comment 14 Richard Biener 2009-06-28 20:43:51 UTC
Another one, reduced from function_module_1.f90:

module M1

INTEGER p

CONTAINS
subroutine AA ()
   implicit NONE
   p = 1
end subroutine
end module

program P1
  USE M1
  implicit none
  p = 0
  call AA ()
  if (p /= 1) call abort
end

where the issue is that p is not properly unified in the use in P1 and AA.
After inlining we see:

p1 ()
{
  integer(kind=4)D.3 p.0D.1516;

<bb 2>:
  pD.1514 = 0;
  pD.1509 = 1;
  p.0D.1516_1 = pD.1514;
  if (p.0D.1516_1 != 1)

so we store to / read from different variables.
Comment 15 Joost VandeVondele 2009-08-03 10:11:35 UTC
testcases in comment #13 and comment #14 pass with current trunk. The testcase in comment #8 still fails.
Comment 16 Richard Biener 2009-09-22 15:44:58 UTC
Re-confirmed with current trunk, testcase from (comment #8).
Comment 17 Joost VandeVondele 2009-10-11 12:49:38 UTC
FYI, the testcase in comment #8 fails for '-O2 -fwhole-file' but not with '-O2 -flto', even though the latter option implies the first. 
Comment 18 Joost VandeVondele 2009-12-09 11:49:44 UTC
still fails with current trunk
Comment 19 Francois-Xavier Coudert 2010-06-09 22:11:22 UTC
(In reply to comment #16)
> Re-confirmed with current trunk, testcase from (comment #8).

I think it now passes.
Comment 20 Joost VandeVondele 2010-07-24 18:12:30 UTC
is this now fixed, all test cases seem to be passing ?
Comment 21 Thomas Koenig 2010-12-30 17:31:51 UTC
Is this still an issue?
Comment 22 Daniel Franke 2011-07-24 18:47:19 UTC
Closing according to comments #19 and #20.