[Bug libfortran/77278] Use LTO for libgfortran

2016-08-20 Thread tkoenig at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77278

Thomas Koenig  changed:

   What|Removed |Added

 Status|UNCONFIRMED |NEW
   Last reconfirmed||2016-08-20
 Ever confirmed|0   |1

--- Comment #1 from Thomas Koenig  ---
It is not easy to find a simple test case where an advantage
can be found.  With my hand-hacked LTO libgfortran, I get

ig25@linux-fd1f:~/Krempel/LTO> cat write.f90
program main
  implicit none
  integer :: i
  real(kind=8) :: a
  do i=1,10**7
call random_number(a)
write (10,'(E17.8," ")',advance="NO") a
  end do
end program main

With -static-libgfortran -lto I get for three consecutive runs

real0m16.463s
user0m15.979s
sys 0m0.449s

real0m17.839s
user0m17.440s
sys 0m0.388s

real0m16.824s
user0m16.378s
sys 0m0.436s

and with the normal shared library

real0m19.508s
user0m19.086s
sys 0m0.410s

real0m19.850s
user0m19.395s
sys 0m0.444s

real0m18.213s
user0m17.802s
sys 0m0.400s

[Bug libfortran/77278] Use LTO for libgfortran

2016-08-21 Thread tkoenig at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77278

--- Comment #2 from Thomas Koenig  ---
Here's a test case which shows a performance loss with LTO-enabled
libgfortran.

program main
  implicit none
  integer, parameter :: n=10**7
  integer :: i
  real, dimension(n) :: a
  real :: t1, t2, t3
  call random_number(a)
  call cpu_time(t1)
  write (20,'(E17.12)') a
  call cpu_time(t2)
  write (21,'(E17.12)') (a(i),i=1,n)
  call cpu_time(t3)
  write (*,*) 'array write: ', t2-t1
  write (*,*) 'implied write: ', t3-t2
end program main

Running three times with shared libraries:
 array write:12.0480003
 implied write:11.8389988
 array write:12.5640001
 implied write:12.5340004
 array write:11.8710003
 implied write:12.1729994  

Running three times with LTO:
 array write:14.9330006
 implied write:15.344
 array write:14.2670002
 implied write:14.8539991
 array write:13.7550001
 implied write:14.8719997  

So, it is not always all that clear.

[Bug libfortran/77278] Use LTO for libgfortran

2016-08-21 Thread tkoenig at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77278

--- Comment #3 from Thomas Koenig  ---
Some information about the type mismatch.

The first mismatch about gfortran_st_write is

lto1: warning: type of '_gfortran_st_write' does not match original declaration
[-Wlto-type-mismatch]

Breakpoint 1, warn_types_mismatch (t1=0x76a8d150, t2=0x76a39540,
loc1=27996512, loc2=0) at ../../trunk/gcc/ipa-devirt.c:1051
1051{
(gdb) up
#1  0x00654b39 in lto_symtab_merge_decls_2 (diagnosed_p=false,
first=) at ../../trunk/gcc/lto/lto-symtab.c:679
679DECL_SOURCE_LOCATION (decl));
(gdb) down
#0  warn_types_mismatch (t1=0x76a8d150, t2=0x76a39540, loc1=27996512,
loc2=0) at ../../trunk/gcc/ipa-devirt.c:1051
1051{
(gdb) call debug_tree(t1)
 >
QI
size  constant 8>
unit size  constant 1>
align 8 symtab 0 alias set -1 structural equality
arg-types 
unsigned DI
size 
unit size 
align 64 symtab 0 alias set 1 structural equality>
chain >>
pointer_to_this >
(gdb) call debug_tree(t2)
 >
QI
size  constant 8>
unit size  constant 1>
align 8 symtab 0 alias set -1 structural equality
attributes 
value >>
arg-types 
unsigned DI
size 
unit size 
align 64 symtab 0 alias set 6 structural equality>
chain >>
pointer_to_this >

which looks quite different, but I lack the tree-foo to really decypher this.

[Bug libfortran/77278] Use LTO for libgfortran

2016-08-21 Thread dominiq at lps dot ens.fr
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77278

--- Comment #4 from Dominique d'Humieres  ---
> This led to error messages like
>
> lto1: warning: type of '_gfortran_st_write' does not match original
> declaration [-Wlto-type-mismatch] 
> ../../../trunk/libgfortran/io/transfer.c:3746:1:
> note: 'st_write' was previously declared here
> ../../../trunk/libgfortran/io/transfer.c:3746:1: note: code may be
> misoptimized unless -fno-strict-aliasing is used
>
> so there is likely work to do on the library side and on the LTO side.

This looks like pr68649 and pr68717.

[Bug libfortran/77278] Use LTO for libgfortran

2019-04-02 Thread dominiq at lps dot ens.fr
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77278
Bug 77278 depends on bug 68717, which changed state.

Bug 68717 Summary: [7/8/9 Regression] New (bogus?) warnings when compiling some 
gfortran.dg tests with -flto after r231239
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68717

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution|--- |FIXED

[Bug libfortran/77278] Use LTO for libgfortran

2019-04-02 Thread dominiq at lps dot ens.fr
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77278
Bug 77278 depends on bug 68649, which changed state.

Bug 68649 Summary: [7/8/9 Regression] note: code may be misoptimized unless 
-fno-strict-aliasing is used
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68649

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution|--- |FIXED

[Bug libfortran/77278] Use LTO for libgfortran

2019-06-02 Thread tkoenig at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77278

--- Comment #5 from Thomas Koenig  ---
One thing we would also have to tackle is GFC_LOGICAL arguments.
C only has one bool type, which is (for gcc) equivalent to
logical(kind=1).  We might just get by with 

typedef enum { _zero=1, _one=1 } GFC_LOGICAL_4;

but what about arguments with other logical types?
We might actually need a C extension there, or disable
aliasing-based optimization.

[Bug libfortran/77278] Use LTO for libgfortran

2019-06-02 Thread tkoenig at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77278

--- Comment #6 from Thomas Koenig  ---
So, I played around with this a little. By putting in
-flto and -ffat-lto-binaries into CFLAGS, FFLAGS and LDFLAGS
into the Makefile in the libgfortran build directory, it is
possible to build an LTO-enabled libgfortran.

For the first test (write.f90 from comment#1 below), a small
patch

Index: io/open.c
===
--- io/open.c   (Revision 271843)
+++ io/open.c   (Arbeitskopie)
@@ -740,6 +740,7 @@ st_open (st_parameter_open *opp)
   GFC_INTEGER_4 cf = opp->common.flags;
   unit_convert conv;

+  memset (&flags, 0, sizeof(flags));
   library_start (&opp->common);

   /* Decode options.  */

led to a lot of conditional code not being pulled into the
main program. So far, so good - constant folding for open
was good.

The main function then became (in the optimized tree dump)

   [local count: 10737418]:
  open_parm.0.common.filename = &"write.f90"[1]{lb: 1 sz: 1};
  open_parm.0.common.line = 5;
  open_parm.0.file = &"foo.bar"[1]{lb: 1 sz: 1};
  open_parm.0.file_len = 7;
  open_parm.0.readonly = 0;
  MEM[(int *)&open_parm.0] = 42966450432;
  st_open.constprop (&open_parm.0);
  open_parm.0 ={v} {CLOBBER};
  # DEBUG i => 1

   [local count: 1063004407]:
  # ivtmp_27 = PHI <1000(2), ivtmp_1(3)>
  # DEBUG i => NULL
  _gfortran_random_r8 (&a);
  dt_parm.1.common.filename = &"write.f90"[1]{lb: 1 sz: 1};
  dt_parm.1.common.line = 8;
  dt_parm.1.advance = &"NO"[1]{lb: 1 sz: 1};
  dt_parm.1.format = &"(E17.8,\" \")"[1]{lb: 1 sz: 1};
  MEM[(long int *)&dt_parm.1 + 88B] = { 11, 2 };
  MEM[(int *)&dt_parm.1] = 42949685248;
  st_write.constprop (&dt_parm.1);
  transfer_real_write.constprop (&dt_parm.1, &a);
  st_write_done (&dt_parm.1);
  dt_parm.1 ={v} {CLOBBER};
  # DEBUG i => NULL
  ivtmp_1 = ivtmp_27 + 4294967295;
  if (ivtmp_1 == 0)
goto ; [1.01%]
  else
goto ; [98.99%]

   [local count: 10737418]:
  a ={v} {CLOBBER};
  return;

}

So, dt_parm_1 is still filled with information in the tight loop
(which the library does not change), and the call to
transfer_real_write.constprop does not do a lot of the things
that could be done in theory, for example keeping the unit
number cached, take a note that this is not asynchronous,
that we always use "NO" on advance in the loop, etc.

So, is it realistic to expect that LTO could do this kind
of thing with the very complex structure that libgfortran?

[Bug libfortran/77278] Use LTO for libgfortran

2019-06-03 Thread rguenth at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77278

--- Comment #7 from Richard Biener  ---
(In reply to Thomas Koenig from comment #5)
> One thing we would also have to tackle is GFC_LOGICAL arguments.
> C only has one bool type, which is (for gcc) equivalent to
> logical(kind=1).  We might just get by with 
> 
> typedef enum { _zero=1, _one=1 } GFC_LOGICAL_4;
> 
> but what about arguments with other logical types?
> We might actually need a C extension there, or disable
> aliasing-based optimization.

aliasing shouldn't be an issue here.  The other logical kinds need to
be mapped to short/int/long anyways for ABI reasons, no?

[Bug libfortran/77278] Use LTO for libgfortran

2019-06-03 Thread rguenther at suse dot de
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77278

--- Comment #8 from rguenther at suse dot de  ---
On Sun, 2 Jun 2019, tkoenig at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77278
> 
> --- Comment #6 from Thomas Koenig  ---
...
> So, dt_parm_1 is still filled with information in the tight loop
> (which the library does not change), and the call to
> transfer_real_write.constprop does not do a lot of the things
> that could be done in theory, for example keeping the unit
> number cached, take a note that this is not asynchronous,
> that we always use "NO" on advance in the loop, etc.
> 
> So, is it realistic to expect that LTO could do this kind
> of thing with the very complex structure that libgfortran?

Sure, why not.  Of course the original motivation wasn't so
much I/O but inlining/optimizing intrinsics.  The frontend
does a lot more inlining itself here today so the effect
might not be as big.

[Bug libfortran/77278] Use LTO for libgfortran

2019-06-03 Thread tkoenig at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77278

--- Comment #9 from Thomas Koenig  ---
Created attachment 46446
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=46446&action=edit
LTO tree dump from simple test case

So, I tried out a simple program:

$ cat minloc.f90
program main
  real, dimension(10,10) :: a
  integer, dimension(2) :: m1
  call random_number(a)
  m1 = minloc(a)
  print *,m1
end program main

Compiling this with the fat-object libgfortran results in the somewhat
humorous

$ gfortran -fdump-tree-optimized -O3 -flto -static-libgfortran  minloc.f90
minloc.f90:5: warning: type of '_gfortran_minloc0_4_r4' does not match original
declaration [-Wlto-type-mismatch]
5 |   m1 = minloc(a)
  | 
../../../ggdb3/libgfortran/generated/minloc0_4_r4.c:38:1: note: type mismatch
in parameter 3
../../../ggdb3/libgfortran/generated/minloc0_4_r4.c:38:1: note: 'minloc0_4_r4'
was previously declared here
../../../ggdb3/libgfortran/generated/minloc0_4_r4.c:38:1: note: code may be
misoptimized unless '-fno-strict-aliasing' is used

which looks like an LTO bug.

There is a lot of code pulled in for writing to standard output
which could be optimized away.

The good news: Inlining _gfortran_minloc0_4_r4 appears to work :-)

[Bug libfortran/77278] Use LTO for libgfortran

2019-06-03 Thread rguenther at suse dot de
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77278

--- Comment #10 from rguenther at suse dot de  ---
On June 3, 2019 6:54:10 PM GMT+02:00, "tkoenig at gcc dot gnu.org"
 wrote:
>https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77278
>
>--- Comment #9 from Thomas Koenig  ---
>Created attachment 46446
>  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=46446&action=edit
>LTO tree dump from simple test case
>
>So, I tried out a simple program:
>
>$ cat minloc.f90
>program main
>  real, dimension(10,10) :: a
>  integer, dimension(2) :: m1
>  call random_number(a)
>  m1 = minloc(a)
>  print *,m1
>end program main
>
>Compiling this with the fat-object libgfortran results in the somewhat
>humorous
>
>$ gfortran -fdump-tree-optimized -O3 -flto -static-libgfortran 
>minloc.f90
>minloc.f90:5: warning: type of '_gfortran_minloc0_4_r4' does not match
>original
>declaration [-Wlto-type-mismatch]
>5 |   m1 = minloc(a)
>  | 
>../../../ggdb3/libgfortran/generated/minloc0_4_r4.c:38:1: note: type
>mismatch
>in parameter 3
>../../../ggdb3/libgfortran/generated/minloc0_4_r4.c:38:1: note:
>'minloc0_4_r4'
>was previously declared here
>../../../ggdb3/libgfortran/generated/minloc0_4_r4.c:38:1: note: code
>may be
>misoptimized unless '-fno-strict-aliasing' is used
>
>which looks like an LTO bug.

It tells you the actual argument of the Fortran frontend generated call is not
compatible with the C prototype. It
Doesn't say which types are involved here and the leading sentence is
confusing. 

>There is a lot of code pulled in for writing to standard output
>which could be optimized away.
>
>The good news: Inlining  _gfortran_minloc0_4_r4 appears to work :-)

[Bug libfortran/77278] Use LTO for libgfortran

2019-06-03 Thread rguenth at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77278

Richard Biener  changed:

   What|Removed |Added

 CC||hubicka at gcc dot gnu.org

--- Comment #11 from Richard Biener  ---
Honza can probably suggest ways to make the warning more to the point and how
to show the missing information (the actual argument type vs. the declared
one).

[Bug libfortran/77278] Use LTO for libgfortran

2019-06-04 Thread tkoenig at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77278

--- Comment #12 from Thomas Koenig  ---
In libgfortran, we have

#define GFC_ARRAY_DESCRIPTOR(type) \
struct {\
  type *base_addr;\
  size_t offset;\
  dtype_type dtype;\
  index_type span;\
  descriptor_dimension dim[];\
}

and then later

typedef GFC_ARRAY_DESCRIPTOR (GFC_INTEGER_4) gfc_array_i4;

so the array descriptors expected by the libgfotran routines
have a flexible array members.

If, in the front end, we have the equivalent of (the type name
isn't exactly what the front end generates)

typedef struct {
  GFC_INTEGER_4 *base_addr;\
  size_t offset;\
  dtype_type dtype;\
  index_type span;\
  descriptor_dimension dim[3];\
} _array03_integer_4_descriptor;

_array03_integer_4_descriptor my_descriptor;

and a pointer type that corresponds to what the library
expects, we should then be able to call

minloc_... ((gfc_array_i4 *) &my_descriptor, ..)

right?

I think this should probably be restricted to calling the
library, I would feel nervous touching use code with this.

[Bug libfortran/77278] Use LTO for libgfortran

2019-06-04 Thread rguenther at suse dot de
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77278

--- Comment #13 from rguenther at suse dot de  ---
On Tue, 4 Jun 2019, tkoenig at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77278
> 
> --- Comment #12 from Thomas Koenig  ---
> In libgfortran, we have
> 
> #define GFC_ARRAY_DESCRIPTOR(type) \
> struct {\
>   type *base_addr;\
>   size_t offset;\
>   dtype_type dtype;\
>   index_type span;\
>   descriptor_dimension dim[];\
> }
> 
> and then later
> 
> typedef GFC_ARRAY_DESCRIPTOR (GFC_INTEGER_4) gfc_array_i4;
> 
> so the array descriptors expected by the libgfotran routines
> have a flexible array members.
> 
> If, in the front end, we have the equivalent of (the type name
> isn't exactly what the front end generates)
> 
> typedef struct {
>   GFC_INTEGER_4 *base_addr;\
>   size_t offset;\
>   dtype_type dtype;\
>   index_type span;\
>   descriptor_dimension dim[3];\
> } _array03_integer_4_descriptor;
> 
> _array03_integer_4_descriptor my_descriptor;

Yeah, I do remember this.  I think we settled on the above
(previously you had dim[7] in the library I think) to be
compatible.  Still a C simple testcase complains:

typedef struct { int ndim; int dim[]; } *descp;
void foo (descp d);

void bar()
{
  struct { int ndim; int dim[2]; } desc;
  desc.ndim = 2;
  foo (&desc);
}

t2.c: In function ‘bar’:
t2.c:8:8: warning: passing argument 1 of ‘foo’ from incompatible pointer 
type [-Wincompatible-pointer-types]
   foo (&desc);
^
t2.c:2:17: note: expected ‘descp’ {aka ‘struct  *’} but 
argument is of type ‘struct  *’
 void foo (descp d);
   ~~^

and we probably assign different alias sets to both.

Now to make aliasing happy both the Frontend and LTO have to
compute the same TYPE_CANONICAL for _all_ of the array
descriptor types.  You can either go and allocate
dim always to the max size statically or in the Fortran
FE use self-referential types (not sure if you then can
statically instantiate an object of such type...) or
rewrite all accesses to the fixed-dimension statically
allocated array descriptors to go via the dim[] type
(I think I suggested the latter elsewhere).

So instantiate my_descriptor and then store for further
use VIEW_CONVERT_EXPR  (my_descriptor).

I hope that doesn't defeat [IPA] optimization ...

> and a pointer type that corresponds to what the library
> expects, we should then be able to call
> 
> minloc_... ((gfc_array_i4 *) &my_descriptor, ..)
> 
> right?
> 
> I think this should probably be restricted to calling the
> library, I would feel nervous touching use code with this.

While that might silence the warning it doesn't solve the
TBAA issue that is indeed present -- the populating of
the descriptor on the fortran caller side would not
alias with reads from the passed descriptor done via
the C routine and its descriptor type.  In practice we
probably see the must-alias relationship and let the
TBAA disambiguation possibility unused but I guess we'll
quickly get less lucky...

[Bug libfortran/77278] Use LTO for libgfortran

2019-06-04 Thread hubicka at ucw dot cz
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77278

--- Comment #14 from Jan Hubicka  ---
> 
> Yeah, I do remember this.  I think we settled on the above
> (previously you had dim[7] in the library I think) to be
> compatible.  Still a C simple testcase complains:
> 
> typedef struct { int ndim; int dim[]; } *descp;
> void foo (descp d);
> 
> void bar()
> {
>   struct { int ndim; int dim[2]; } desc;
>   desc.ndim = 2;
>   foo (&desc);
> }
> 
> t2.c: In function ‘bar’:
> t2.c:8:8: warning: passing argument 1 of ‘foo’ from incompatible pointer 
> type [-Wincompatible-pointer-types]
>foo (&desc);
> ^
> t2.c:2:17: note: expected ‘descp’ {aka ‘struct  *’} but 
> argument is of type ‘struct  *’
>  void foo (descp d);
>~~^
> 
> and we probably assign different alias sets to both.

Curiously enough in C version of the LTO testcase I get no warning now
since we simplify function type and thus both pointers are turned into
incomplete pointers and considered TBAA compatible.
After lunch I will check why the warning triggers here.

Still the alias set is different, so I think we more or less get lucky
by using base+offset tests first since array descriptors are accessed
directly after constant propagation of poointers.
> 
> Now to make aliasing happy both the Frontend and LTO have to
> compute the same TYPE_CANONICAL for _all_ of the array
> descriptor types.  You can either go and allocate
> dim always to the max size statically or in the Fortran
> FE use self-referential types (not sure if you then can
> statically instantiate an object of such type...) or
> rewrite all accesses to the fixed-dimension statically
> allocated array descriptors to go via the dim[] type
> (I think I suggested the latter elsewhere).
> 
> So instantiate my_descriptor and then store for further
> use VIEW_CONVERT_EXPR  (my_descriptor).
> 
> I hope that doesn't defeat [IPA] optimization ...
I think Martin's code generally gives up on type mismaches so it
is quite possible it gives up already.

An option would be also to teach LTO that array descriptors are
special types aliasing with each other but not aliasing with
anything else.

Honza

[Bug libfortran/77278] Use LTO for libgfortran

2019-06-04 Thread rguenther at suse dot de
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77278

--- Comment #15 from rguenther at suse dot de  ---
On Tue, 4 Jun 2019, hubicka at ucw dot cz wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77278
> 
> --- Comment #14 from Jan Hubicka  ---
> > 
> > Yeah, I do remember this.  I think we settled on the above
> > (previously you had dim[7] in the library I think) to be
> > compatible.  Still a C simple testcase complains:
> > 
> > typedef struct { int ndim; int dim[]; } *descp;
> > void foo (descp d);
> > 
> > void bar()
> > {
> >   struct { int ndim; int dim[2]; } desc;
> >   desc.ndim = 2;
> >   foo (&desc);
> > }
> > 
> > t2.c: In function ‘bar’:
> > t2.c:8:8: warning: passing argument 1 of ‘foo’ from incompatible pointer 
> > type [-Wincompatible-pointer-types]
> >foo (&desc);
> > ^
> > t2.c:2:17: note: expected ‘descp’ {aka ‘struct  *’} but 
> > argument is of type ‘struct  *’
> >  void foo (descp d);
> >~~^
> > 
> > and we probably assign different alias sets to both.
> 
> Curiously enough in C version of the LTO testcase I get no warning now
> since we simplify function type and thus both pointers are turned into
> incomplete pointers and considered TBAA compatible.
> After lunch I will check why the warning triggers here.
> 
> Still the alias set is different, so I think we more or less get lucky
> by using base+offset tests first since array descriptors are accessed
> directly after constant propagation of poointers.
> > 
> > Now to make aliasing happy both the Frontend and LTO have to
> > compute the same TYPE_CANONICAL for _all_ of the array
> > descriptor types.  You can either go and allocate
> > dim always to the max size statically or in the Fortran
> > FE use self-referential types (not sure if you then can
> > statically instantiate an object of such type...) or
> > rewrite all accesses to the fixed-dimension statically
> > allocated array descriptors to go via the dim[] type
> > (I think I suggested the latter elsewhere).
> > 
> > So instantiate my_descriptor and then store for further
> > use VIEW_CONVERT_EXPR  (my_descriptor).
> > 
> > I hope that doesn't defeat [IPA] optimization ...
> I think Martin's code generally gives up on type mismaches so it
> is quite possible it gives up already.
> 
> An option would be also to teach LTO that array descriptors are
> special types aliasing with each other but not aliasing with
> anything else.

I'd rather not do that ;)  Btw, I wonder what happens at
the call boundary inside a single fortran module where
the caller passes a dim[2] array to a subroutine
handling arbitrary dimension arrays?  I suspect the
IL would have the very same TBAA issue.  Can you produce
a fortran testcase that exposes such a case so we can have a
look into the details?

[Bug libfortran/77278] Use LTO for libgfortran

2019-06-04 Thread mjambor at suse dot cz
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77278

--- Comment #16 from Martin Jambor  ---
Hi,

On Tue, Jun 04 2019, Jan Hubicka wrote:
>> 
>> Yeah, I do remember this.  I think we settled on the above
>> (previously you had dim[7] in the library I think) to be
>> compatible.  Still a C simple testcase complains:
>> 
>> typedef struct { int ndim; int dim[]; } *descp;
>> void foo (descp d);
>> 
>> void bar()
>> {
>>   struct { int ndim; int dim[2]; } desc;
>>   desc.ndim = 2;
>>   foo (&desc);
>> }
>> 
>> t2.c: In function ‘bar’:
>> t2.c:8:8: warning: passing argument 1 of ‘foo’ from incompatible pointer 
>> type [-Wincompatible-pointer-types]
>>foo (&desc);
>> ^
>> t2.c:2:17: note: expected ‘descp’ {aka ‘struct  *’} but 
>> argument is of type ‘struct  *’
>>  void foo (descp d);
>>~~^
>> 
>> and we probably assign different alias sets to both.
>
> Curiously enough in C version of the LTO testcase I get no warning now
> since we simplify function type and thus both pointers are turned into
> incomplete pointers and considered TBAA compatible.
> After lunch I will check why the warning triggers here.
>
> Still the alias set is different, so I think we more or less get lucky
> by using base+offset tests first since array descriptors are accessed
> directly after constant propagation of poointers.
>> 
>> Now to make aliasing happy both the Frontend and LTO have to
>> compute the same TYPE_CANONICAL for _all_ of the array
>> descriptor types.  You can either go and allocate
>> dim always to the max size statically or in the Fortran
>> FE use self-referential types (not sure if you then can
>> statically instantiate an object of such type...) or
>> rewrite all accesses to the fixed-dimension statically
>> allocated array descriptors to go via the dim[] type
>> (I think I suggested the latter elsewhere).
>> 
>> So instantiate my_descriptor and then store for further
>> use VIEW_CONVERT_EXPR  (my_descriptor).
>> 
>> I hope that doesn't defeat [IPA] optimization ...
> I think Martin's code generally gives up on type mismaches so it
> is quite possible it gives up already.

I guess I'm missing some context here (and the example above does not
contain t1.c?).  But I quickly grepped for typec_compatible_p and
useless_type_conversion_p in ipa-prop.c (and ipa-cp.c) and the code
tries to resolve issues with inserting V_C_E rather than giving up
straight away.

Martin

[Bug libfortran/77278] Use LTO for libgfortran

2019-06-04 Thread hubicka at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77278

--- Comment #17 from Jan Hubicka  ---
A small self-contained example would be welcome, I can take a look why aliasing
oracle does not mess things up.

Concerning the warning, those are quite hard to do - the line information
should point to mismatched declarations, but does not since libgfortran is no
longer on the expected path.

Dumping actual type will get it output in C-like syntax which is probably not
very useful except for debugging reasons...

[Bug libfortran/77278] Use LTO for libgfortran

2019-06-04 Thread tkoenig at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77278

--- Comment #18 from Thomas Koenig  ---
Created attachment 46451
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=46451&action=edit
Preprocessed source of library file for LTO mismatch

Hi,

here is a test case (preprocessed source from libgfortran).  To reproduce,
use the test program

$ cat minloc.f90
program main
  real, dimension(10,10) :: a
  integer, dimension(2) :: m1
  call random_number(a)
  m1 = minloc(a)
  print *,m1
end program main

and compile/link with

$ gfortran -static-libgfortran -flto -O3 minloc.f90 minloc0_4_r4.i 
minloc.f90:5: warning: type of '_gfortran_minloc0_4_r4' does not match original
declaration [-Wlto-type-mismatch]
5 |   m1 = minloc(a)
  | 
../../../ggdb3/libgfortran/generated/minloc0_4_r4.c:38:1: note: type mismatch
in parameter 3
../../../ggdb3/libgfortran/generated/minloc0_4_r4.c:38:1: note: 'minloc0_4_r4'
was previously declared here
../../../ggdb3/libgfortran/generated/minloc0_4_r4.c:38:1: note: code may be
misoptimized unless '-fno-strict-aliasing' is used

[Bug libfortran/77278] Use LTO for libgfortran

2019-06-04 Thread tkoenig at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77278

--- Comment #19 from Thomas Koenig  ---
(In reply to rguent...@suse.de from comment #15)
  Btw, I wonder what happens at
> the call boundary inside a single fortran module where
> the caller passes a dim[2] array to a subroutine
> handling arbitrary dimension arrays?  I suspect the
> IL would have the very same TBAA issue.  Can you produce
> a fortran testcase that exposes such a case so we can have a
> look into the details?

Here is a test case:

module x
  implicit none
contains
  subroutine foo(a)
real, dimension(..) :: a
print *,shape(a)
  end subroutine foo
  subroutine bar
real, dimension(2,2) :: a
real, dimension(3,3,3) :: b
call foo(a)
call foo(b)
  end subroutine bar
end module x

program main
  use x
  call bar
end program main

Looking at the *.original tree dump, we have

bar ()
{
  real(kind=4) a[4];
  real(kind=4) b[27];

  {
struct array02_real(kind=4) parm.0;

[...]

foo (&parm.0);
  }

  {
struct array03_real(kind=4) parm.1;

[...]

foo (&parm.1);
  }

and

foo (struct array15_real(kind=4) & restrict a)
{
  {
struct __st_parameter_dt dt_parm.2;

This does not really look very healthy (but it is not warned about).

[Bug libfortran/77278] Use LTO for libgfortran

2019-06-04 Thread hubicka at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77278

--- Comment #20 from Jan Hubicka  ---
OK, the mismatched declaration types are:
void  (struct array01_integer(kind=4) &, float & restrict,
logical(kind=4) *)
and
void  (struct gfc_array_i4 * restrict, struct gfc_array_r4 * restrict,
GFC_LOGICAL_4)

The mismatch happens in the last parameter that is logical(kind=4) and
GFC_LOGICAL_4.

 
unit-size 
align:32 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type
0x76616c78 precision:1 min  max 
pointer_to_this >
unsigned DI
size  constant 64>
unit-size  constant 8>
align:64 warn_if_not_align:0 symtab:0 alias-set 3 structural-equality>


  constant 32>
unit-size  constant 4>
align:32 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type
0x76821690 precision:32 min  max  context 
pointer_to_this >

So mixing up enum and pointer to enum.
Fixing C source to expect pointer to enum makes warning to go away, but looking
at the gimple produced, it really just seems in bug in fortran FE declaring the
function incorrectly? It seems to really just pass 0 instead of pointer to 0:
_gfortran_minloc0_4_r4 (&parm.1, _40, 0);

Honza

[Bug libfortran/77278] Use LTO for libgfortran

2019-06-04 Thread tkoenig at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77278

--- Comment #21 from Thomas Koenig  ---
(In reply to Jan Hubicka from comment #20)
> OK, the mismatched declaration types are:
> void  (struct array01_integer(kind=4) &, float & restrict,
> logical(kind=4) *)
> and
> void  (struct gfc_array_i4 * restrict, struct gfc_array_r4 * restrict,
> GFC_LOGICAL_4)
> 
> The mismatch happens in the last parameter that is logical(kind=4) and
> GFC_LOGICAL_4.

Thanks for looking into this.

One question: What do I have to do to get at this type of information,
and the following tree type dump?  What exactly would I have to
run?  (I looked around, but could not find any info).


> So mixing up enum and pointer to enum.

> Fixing C source to expect pointer to enum makes warning to go away, but
> looking at the gimple produced, it really just seems in bug in fortran FE
> declaring the function incorrectly? It seems to really just pass 0 instead
> of pointer to 0:
> _gfortran_minloc0_4_r4 (&parm.1, _40, 0);


It was certainly the inention to pass a value.

However, I tried doing this with this little patchlet

Index: intrinsic.c
===
--- intrinsic.c (Revision 271843)
+++ intrinsic.c (Arbeitskopie)
@@ -2614,6 +2614,8 @@ add_functions (void)
   msk, BT_LOGICAL, dl, OPTIONAL, kind, BT_INTEGER, di, OPTIONAL,
   bck, BT_LOGICAL, dl, OPTIONAL);

+  set_attr_value (5, false, false, false, false, true);
+
   make_generic ("minloc", GFC_ISYM_MINLOC, GFC_STD_F95);

   add_sym_3red ("minval", GFC_ISYM_MINVAL, CLASS_TRANSFORMATIONAL, ACTUAL_NO,
BT_REAL, dr, GFC_STD_F95,

and ended up with

minloc.f90:5: warning: type of '_gfortran_minloc0_4_r4' does not match original
declaration [-Wlto-type-mismatch]
5 |   m1 = minloc(a)
  | 
../../../ggdb3/libgfortran/generated/minloc0_4_r4.c:38:1: note: type mismatch
in parameter 3
../../../ggdb3/libgfortran/generated/minloc0_4_r4.c:38:1: note: type
'GFC_LOGICAL_4' should match type 'logical(kind=4)'
../../../ggdb3/libgfortran/generated/minloc0_4_r4.c:38:1: note: 'minloc0_4_r4'
was previously declared here
../../../ggdb3/libgfortran/generated/minloc0_4_r4.c:38:1: note: code may be
misoptimized unless '-fno-strict-aliasing' is used

Now, currently GFC_LOGICAL_4 is just a typedef for GFC_INTEGER_4, which
in turn is just a typedef for int.  Could this be the source of the
trouble here?  And what could we do about a GFC_LOGICAL_2 and so on?

[Bug libfortran/77278] Use LTO for libgfortran

2019-06-05 Thread rguenther at suse dot de
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77278

--- Comment #22 from rguenther at suse dot de  ---
On Tue, 4 Jun 2019, tkoenig at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77278
> 
> --- Comment #19 from Thomas Koenig  ---
> (In reply to rguent...@suse.de from comment #15)
>   Btw, I wonder what happens at
> > the call boundary inside a single fortran module where
> > the caller passes a dim[2] array to a subroutine
> > handling arbitrary dimension arrays?  I suspect the
> > IL would have the very same TBAA issue.  Can you produce
> > a fortran testcase that exposes such a case so we can have a
> > look into the details?
> 
> Here is a test case:
> 
> module x
>   implicit none
> contains
>   subroutine foo(a)
> real, dimension(..) :: a
> print *,shape(a)
>   end subroutine foo
>   subroutine bar
> real, dimension(2,2) :: a
> real, dimension(3,3,3) :: b
> call foo(a)
> call foo(b)
>   end subroutine bar
> end module x
> 
> program main
>   use x
>   call bar
> end program main
> 
> Looking at the *.original tree dump, we have
> 
> bar ()
> {
>   real(kind=4) a[4];
>   real(kind=4) b[27];
> 
>   {
> struct array02_real(kind=4) parm.0;
> 
> [...]
> 
> foo (&parm.0);
>   }
> 
>   {
> struct array03_real(kind=4) parm.1;
> 
> [...]
> 
> foo (&parm.1);
>   }
> 
> and
> 
> foo (struct array15_real(kind=4) & restrict a)
> {
>   {
> struct __st_parameter_dt dt_parm.2;
> 
> This does not really look very healthy (but it is not warned about).

OK, so changing the testcase to

module x
  implicit none
contains
  subroutine foo(a)
real, dimension(..) :: a
if (rank(a) < 2) STOP 3
  end subroutine foo
  subroutine bar
real, dimension(2,2) :: a
real, dimension(3,3,3) :: b
call foo(a)
call foo(b)
  end subroutine bar
end module x

program main
  use x
  call bar
end program main

makes us inline foo() where we then see

  struct array03_real(kind=4) parm.1;
  struct array02_real(kind=4) parm.0;
...
  parm.0.dtype.rank = 2;
...
  _15 = MEM[(struct array15_real(kind=4) &)&parm.0].dtype.rank;
  if (_15 <= 1)

which shows a possible wrong-code issue because the store
to rank is via array02 but the read uses array15.  We don't
miscompile this because we try to be forgiving if we see
a must-alias.

The frontend should create a descriptor type mimicing the one
in the library API and emit

  MEM[(struct arrayN_real(kind=4) &)¶m.0].dtype.rank = 2;
...
  _15 = MEM[(struct arrayN_real(kind=4) &)¶m.0].dtype.rank;
  if (_15 <= 1)

that is, all accesses to array descriptors should be as-if
they were indirect accesses through the assumed rank descriptor
type with the flex-array dim member.

Note this would no longer allow TBAA disambiguation of
different rank array descriptor accesses.  So alternatively
we would have to arrange accesses via structs with a flex
array member alias accesses via structs with fixed-array
members.  Still functions accessing assumed rank arrays
like foo() above then need to use the flexarray descriptor
type to access dtype.rank, not as it does at the moment
the max-rank descriptor type.  I suppose that would be
the first thing to fix (maybe also the easiest).

Honza and me can then think about ways to make
struct { int ndim; int dim[]; } accesses conflict with
struct { int ndim; int dim[N]; } with arbitrary N.
Like in record-component-aliases if it sees a trailing
fixed-size array member, build the corresponding flex-array
record and record a subset of that (ugh).

[Bug libfortran/77278] Use LTO for libgfortran

2019-06-05 Thread hubicka at ucw dot cz
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77278

--- Comment #23 from Jan Hubicka  ---
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77278
> 
> --- Comment #21 from Thomas Koenig  ---
> (In reply to Jan Hubicka from comment #20)
> > OK, the mismatched declaration types are:
> > void  (struct array01_integer(kind=4) &, float & restrict,
> > logical(kind=4) *)
> > and
> > void  (struct gfc_array_i4 * restrict, struct gfc_array_r4 * restrict,
> > GFC_LOGICAL_4)
> > 
> > The mismatch happens in the last parameter that is logical(kind=4) and
> > GFC_LOGICAL_4.
> 
> Thanks for looking into this.
> 
> One question: What do I have to do to get at this type of information,
> and the following tree type dump?  What exactly would I have to
> run?  (I looked around, but could not find any info).

I simply add debug_tree and debug_generic_stmt to the place warning is
output (in lto/lto-symtab.c)
It would be nice to make these warnings more understandable somehow.  I
did some work on the ODR warnings, but in this case it is even harder
(especially because the mismatch is cross-language and we have no way to
dump types in user understandable form at this stage).
> 
> 
> > So mixing up enum and pointer to enum.
> 
> > Fixing C source to expect pointer to enum makes warning to go away, but
> > looking at the gimple produced, it really just seems in bug in fortran FE
> > declaring the function incorrectly? It seems to really just pass 0 instead
> > of pointer to 0:
> > _gfortran_minloc0_4_r4 (&parm.1, _40, 0);
> 
> Now, currently GFC_LOGICAL_4 is just a typedef for GFC_INTEGER_4, which
> in turn is just a typedef for int.  Could this be the source of the
> trouble here?  And what could we do about a GFC_LOGICAL_2 and so on?

In the dump it seems that fortran FE produces boolean_type while the
other is enum type.
We turn enum type into integer type, but boolean remains boolean
so they are not considered same.

Does changing typedef to boolean help?

Honza
> 
> -- 
> You are receiving this mail because:
> You are on the CC list for the bug.

[Bug libfortran/77278] Use LTO for libgfortran

2019-06-05 Thread hubicka at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77278

--- Comment #24 from Jan Hubicka  ---
Hi,
actually it won't help since C has only one bool type and not bools in
different sizes (why would one need that?).
I guess it would be easiest to turn Fortran frontend to use integers here.

Honza

[Bug libfortran/77278] Use LTO for libgfortran

2019-06-05 Thread tkoenig at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77278

--- Comment #25 from Thomas Koenig  ---
(In reply to Jan Hubicka from comment #24)
> Hi,
> actually it won't help since C has only one bool type and not bools in
> different sizes (why would one need that?).

"Because it's in the Fortran language standard" is probably a good
answer as any.  For example, the standard specifies that default
logical and default integer have to have the same size, and
it specifies all the other logical kinds.

> I guess it would be easiest to turn Fortran frontend to use integers here.

Easiest for this case, yes.

However, this is far from the only case where integer vs. logical
is a problem in libgfortran - to reduce code size, we've been treating
them interchangeably for quite some time.

Really, it would be easiest if there was a way to tell the middle
end that logical(kind=4) and integer(kind=4) could alias.

There is another, possibly even worse, case.  To reduce combinatorical
explosion of some functions, we have been using this idiom

  mask_kind = GFC_DESCRIPTOR_SIZE (mask);

  if (mask_kind == 1 || mask_kind == 2 || mask_kind == 4 || mask_kind == 8
#ifdef HAVE_GFC_LOGICAL_16
  || mask_kind == 16
#endif
  )
{
  /*  Do not convert a NULL pointer as we use test for NULL below.  */
  if (mptr)
mptr = GFOR_POINTER_TO_L1 (mptr, mask_kind);
}
  else
runtime_error ("Funny sized logical array");

where GFOR_POINTER_TO_L1 computes an offset between little- and big-endian
systems, and we then access the value bytewise.

If we made this more cleanly, we would add 1505 more functions to the
libgfortran library (rough count).

On the plus side, all of the pointers involved are restrict.

[Bug libfortran/77278] Use LTO for libgfortran

2019-06-06 Thread rguenther at suse dot de
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77278

--- Comment #26 from rguenther at suse dot de  ---
On Thu, 6 Jun 2019, tkoenig at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77278
> 
> --- Comment #25 from Thomas Koenig  ---
> (In reply to Jan Hubicka from comment #24)
> > Hi,
> > actually it won't help since C has only one bool type and not bools in
> > different sizes (why would one need that?).
> 
> "Because it's in the Fortran language standard" is probably a good
> answer as any.  For example, the standard specifies that default
> logical and default integer have to have the same size, and
> it specifies all the other logical kinds.
> 
> > I guess it would be easiest to turn Fortran frontend to use integers here.
> 
> Easiest for this case, yes.
> 
> However, this is far from the only case where integer vs. logical
> is a problem in libgfortran - to reduce code size, we've been treating
> them interchangeably for quite some time.
> 
> Really, it would be easiest if there was a way to tell the middle
> end that logical(kind=4) and integer(kind=4) could alias.

I think that's reasonably easy to do for LTO.  We'd want to keep
the default boolean_type_node size BOOLEAN_TYPEs separate but
can glob larger ones with integer types in the canonical type
merging.  We can probably do the same in non-LTO but that might
not be required.

Honza?

> There is another, possibly even worse, case.  To reduce combinatorical
> explosion of some functions, we have been using this idiom
> 
>   mask_kind = GFC_DESCRIPTOR_SIZE (mask);
> 
>   if (mask_kind == 1 || mask_kind == 2 || mask_kind == 4 || mask_kind == 8
> #ifdef HAVE_GFC_LOGICAL_16
>   || mask_kind == 16
> #endif
>   )
> {
>   /*  Do not convert a NULL pointer as we use test for NULL below.  */
>   if (mptr)
> mptr = GFOR_POINTER_TO_L1 (mptr, mask_kind);
> }
>   else
> runtime_error ("Funny sized logical array");
> 
> where GFOR_POINTER_TO_L1 computes an offset between little- and big-endian
> systems, and we then access the value bytewise.
> 
> If we made this more cleanly, we would add 1505 more functions to the
> libgfortran library (rough count).

If the "bytewise" access uses character types (or uint8_t) then
TBAA wise that should be fine.  If the pointers are restrict
you don't lose optimization either.

[Bug libfortran/77278] Use LTO for libgfortran

2019-06-06 Thread hubicka at ucw dot cz
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77278

--- Comment #27 from Jan Hubicka  ---
> 
> I think that's reasonably easy to do for LTO.  We'd want to keep
> the default boolean_type_node size BOOLEAN_TYPEs separate but
> can glob larger ones with integer types in the canonical type
> merging.  We can probably do the same in non-LTO but that might
> not be required.

Yes, we can glob other sizes of bool into integers and that should
not affect non-fortran languages where truth comes in only one size.
It would be bit inconsistent in a way that logical sized same way
as C bool will bind to C _Bool type and others will bind to integer
types of corresponding sizes.

The Fortran 2003 language draft has section on interoperability of
C and Fortran language:
https://j3-fortran.org/doc/year/10/10-007.pdf
Which says that fortran language C_BOOL should interoperate with _Bool.
Why libgfortran API functions which dispatch into C code are not
declared with C_BOOL rather than the integer sized logical?

I wrote some testcases for LTO C and fortran types interoperatibility
(gfortran.dg/lto/bind_c*)
Since my Fortran-fu is limited, they may not be complete. It would be
very useful to look into them and see if everything important is
tested and also that we have testcase for all cases we want to support
in addition to stadnard (like this one it seems) with some rationale
in them for future reference.

Honza

[Bug libfortran/77278] Use LTO for libgfortran

2019-06-10 Thread tkoenig at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77278

--- Comment #28 from Thomas Koenig  ---
(In reply to Jan Hubicka from comment #27)
> > 
> > I think that's reasonably easy to do for LTO.  We'd want to keep
> > the default boolean_type_node size BOOLEAN_TYPEs separate but
> > can glob larger ones with integer types in the canonical type
> > merging.  We can probably do the same in non-LTO but that might
> > not be required.
> 
> Yes, we can glob other sizes of bool into integers and that should
> not affect non-fortran languages where truth comes in only one size.
> It would be bit inconsistent in a way that logical sized same way
> as C bool will bind to C _Bool type and others will bind to integer
> types of corresponding sizes.

That would be excellent.  It would also solve a problem with
non-standard interoperability (which people used before there
was the standard version, and which continues to be used
to this day, for example in LAPACK or BLAS).

The old code passes a standard LOGICAL to a C routine by reference,
but the C side can only use an int pointer.

> The Fortran 2003 language draft has section on interoperability of
> C and Fortran language:
> https://j3-fortran.org/doc/year/10/10-007.pdf
> Which says that fortran language C_BOOL should interoperate with _Bool.
> Why libgfortran API functions which dispatch into C code are not
> declared with C_BOOL rather than the integer sized logical?

History.  The library implementation was started long before the
standard C interface. And when we implemented the BACK intrinsic,
passing it as a C_BOOL was simply not discussed.

If we ever rewrite our ABI (again), I think it would make sense to
pass all scalar LOGICAL arguments via scalar.

> I wrote some testcases for LTO C and fortran types interoperatibility
> (gfortran.dg/lto/bind_c*)
> Since my Fortran-fu is limited, they may not be complete. It would be
> very useful to look into them and see if everything important is
> tested and also that we have testcase for all cases we want to support
> in addition to stadnard (like this one it seems) with some rationale
> in them for future reference.

One thing that should work is (according to the convention that
people use, and also to what we're using in libgfortran):

$ cat logical_f.f90
program main
  logical (kind=1) :: l1
  logical (kind=2) :: l2
  logical (kind=4) :: l4
  logical (kind=8) :: l8
  logical (kind=16) :: l16

  l1 = .true.
  l2 = .true.
  l4 = .true.
  l8 = .true.
  call logical_c (l1, l2, l4, l8, l16)
  if (l1 .or. l2 .or. l4 .or. l8 .or. l16) stop 1
end program main

subroutine foo(l1, l2, l4, l8, l16)
  logical (kind=1) :: l1
  logical (kind=2) :: l2
  logical (kind=4) :: l4
  logical (kind=8) :: l8
  logical (kind=16) :: l16

  l1 = .false.
  l2 = .false.
  l4 = .false.
  l8 = .false.
  l16 = .false.
end subroutine foo

$ cat logical_c.c
void logical_c_ (_Bool *l1, short *l2, int *l4, long *l8, long long *l16)
{
  foo_ (l1, l2, l4, l8, l16);
}

This would be a big first step in making things compatible.

Another test case which currently works, and where it would
be important that it keeps working, is

$ cat character_f.f90
program main
  character (len=10) :: a
  a = ''
  call foo(a)
  if (a /= '0123456789') stop 1
end program main

subroutine bar (a)
  character (len=*), intent(inout) :: a
  if (len(a) /= 10) stop 2
  if (a /= 'ABCDEFGHIJ') stop 3
  a = '0123456789'
end subroutine bar
$ cat character_c.c
#include 
#include 

void bar_ (char *a, size_t a_len);

void foo_ (char *a, size_t a_len)
{
  memcpy (a, "ABCDEFGHIJ", 10);
  bar_ (a, 10);
}

A really large positive effect would be that we could then really
tell people to use -flto to detect broken C / Fortran bindings.
Unfortunately, there are very many out there, see PR 90329.

[Bug libfortran/77278] Use LTO for libgfortran

2019-06-17 Thread tkoenig at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77278

--- Comment #29 from Thomas Koenig  ---
Created attachment 46493
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=46493&action=edit
Patch to put all libgfortran functions into a namespace

This is something we should be doing as part of making libgfortran
LTO-compatible.  It adds all resolved library function to the libgfortran
namespace.

This does not work (i.e. causes regressions) because of functions like
pack, unpack, cshift etc where we call the same function with
different types of arguments.  We should be calling this via
gfc_array_char.  For example, here's eoshift0.c:

static void
eoshift0 (gfc_array_char * ret, const gfc_array_char * array,
  index_type shift, const char * pbound, int which, index_type size,
  const char *filler, index_type filler_len)

[...]

[Bug libfortran/77278] Use LTO for libgfortran

2019-06-20 Thread hubicka at ucw dot cz
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77278

--- Comment #30 from Jan Hubicka  ---
Hi,
this patch makes Fortran logicals to become C unsigned types of
corresponding size.  I think it is better than making them signed
because the globbing will affect aliasing within Fortran programs as
well.  We still ignore sign on chars and size_t, so this is still
throwing away some precision (i.e. LOGICAL of size 1 will become INTEGER
if size 1 for TBAA purposes)

Index: lto/lto-common.c
===
--- lto/lto-common.c(revision 272506)
+++ lto/lto-common.c(working copy)
@@ -238,6 +238,8 @@ hash_canonical_type (tree type)
  types.  */
   gcc_checking_assert (type_with_alias_set_p (type));

+  type = type_for_canonical_type_merging (type);
+
   /* Combine a few common features of types so that types are grouped into
  smaller sets; when searching for existing matching types to merge,
  only existing types having the same features as the new type will be
Index: testsuite/gfortran.dg/lto/bind_c-7_0.f90
===
--- testsuite/gfortran.dg/lto/bind_c-7_0.f90(nonexistent)
+++ testsuite/gfortran.dg/lto/bind_c-7_0.f90(working copy)
@@ -0,0 +1,33 @@
+! { dg-lto-do run }
+! { dg-lto-options {{ -O3 -flto }} }
+! this testcase tests additional interoperability of Fortran logical
+! that is not part of the standard but needed by libgfortran.
+program main
+  logical (kind=1) :: l1
+  logical (kind=2) :: l2
+  logical (kind=4) :: l4
+  logical (kind=8) :: l8
+  logical (kind=16) :: l16
+
+  l1 = .true.
+  l2 = .true.
+  l4 = .true.
+  l8 = .true.
+  call logical_c (l1, l2, l4, l8, l16)
+  if (l1 .or. l2 .or. l4 .or. l8 .or. l16) stop 1
+end program main
+
+subroutine foo(l1, l2, l4, l8, l16)
+  logical (kind=1) :: l1
+  logical (kind=2) :: l2
+  logical (kind=4) :: l4
+  logical (kind=8) :: l8
+  logical (kind=16) :: l16
+
+  l1 = .false.
+  l2 = .false.
+  l4 = .false.
+  l8 = .false.
+  l16 = .false.
+end subroutine foo
+
Index: testsuite/gfortran.dg/lto/bind_c-7_1.c
===
--- testsuite/gfortran.dg/lto/bind_c-7_1.c  (nonexistent)
+++ testsuite/gfortran.dg/lto/bind_c-7_1.c  (working copy)
@@ -0,0 +1,6 @@
+
+extern void foo_ (_Bool *l1, unsigned short *l2, unsigned int *l4, unsigned
long long *l8, unsigned __int128 *l16);
+void logical_c_ (_Bool *l1, unsigned short *l2, unsigned int *l4, unsigned
long long *l8, unsigned __int128 *l16)
+{
+  foo_ (l1, l2, l4, l8, l16);
+}
Index: tree.c
===
--- tree.c  (revision 272506)
+++ tree.c  (working copy)
@@ -14047,6 +14053,25 @@ type_with_interoperable_signedness (cons
 || TYPE_PRECISION (type) == TYPE_PRECISION (size_type_node));
 }

+/* Return type replacing TYPE for canonical type merging.
+   
+   We translate BOOLEAN_TYPE of size different from C _Bool to
+   unsigned integers. This makes it possible to interoperate C
+   and Fortran on Fortran's LOGICAL types which come in different
+   sizes.  This is not required by the language standard but
+   used by libgfortran.  */
+
+extern tree
+type_for_canonical_type_merging (const_tree type)
+{
+  if (TREE_CODE (type) == BOOLEAN_TYPE
+  && !tree_int_cst_equal (TYPE_SIZE (type),
+ TYPE_SIZE (boolean_type_node)))
+return lang_hooks.types.type_for_mode (TYPE_MODE (type),
+  true);
+  return const_cast  (type);
+}
+
 /* Return true iff T1 and T2 are structurally identical for what
TBAA is concerned.  
This function is used both by lto.c canonical type merging and by the
@@ -14066,6 +14091,8 @@ gimple_canonical_types_compatible_p (con
   t1 = TYPE_MAIN_VARIANT (t1);
   t2 = TYPE_MAIN_VARIANT (t2);
 }
+  t1 = type_for_canonical_type_merging (t1);
+  t2 = type_for_canonical_type_merging (t2);

   /* Check first for the obvious case of pointer identity.  */
   if (t1 == t2)
Index: tree.h
===
--- tree.h  (revision 272506)
+++ tree.h  (working copy)
@@ -5132,9 +5141,10 @@ extern void DEBUG_FUNCTION verify_type (
 extern bool gimple_canonical_types_compatible_p (const_tree, const_tree,
 bool trust_type_canonical =
true);
 extern bool type_with_interoperable_signedness (const_tree);
+extern tree type_for_canonical_type_merging (const_tree type);
 extern bitmap get_nonnull_args (const_tree);
 extern int get_range_pos_neg (tree);

 /* Return simplified tree code of type that is used for canonical type
merging.  */
 inline enum tree_code

[Bug libfortran/77278] Use LTO for libgfortran

2019-06-20 Thread tkoenig at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77278

--- Comment #31 from Thomas Koenig  ---
(
> this patch makes Fortran logicals to become C unsigned types of
> corresponding size.  I think it is better than making them signed
> because the globbing will affect aliasing within Fortran programs as
> well.

I think you're right there, I was wondering about that problem
as well.

Do I understand correctly that this type merging only happens
if LTO is enabled, and does not affect performance otherwise?

We would then have to redefine the GFC_LOGICAL_* types
in libgfortran to unsigned, but that should be doable.

Regarding the test case: As it is, this will only work if
there is a 16-byte integer. It is probably better to
use only up to 8-byte logicals, and put in a separate
test case with the 16-byte logicals and put in

! { dg-require-effective-target fortran_integer_16 }

This is looking very good, thanks!