In order to aid optimization, it could be beneficial to build libgfortran as an lto-enabled library. There could be a big win for functions being called via constant propagation, especially for I/O and array operations. With I/O, checking for large number of options at runtime could be avoided. Array operations could profit from cases where arrays are contiguous, when the library needs to take non-contiguous cases into account. Some discussion starts here. https://gcc.gnu.org/ml/fortran/2016-08/msg00069.html What has been done so far to identify potential problems was to manually add -flto -ffat-lto-objects to the Makefile in libgfortran and compile a simple program with -flto. 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.
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 real 0m16.463s user 0m15.979s sys 0m0.449s real 0m17.839s user 0m17.440s sys 0m0.388s real 0m16.824s user 0m16.378s sys 0m0.436s and with the normal shared library real 0m19.508s user 0m19.086s sys 0m0.410s real 0m19.850s user 0m19.395s sys 0m0.444s real 0m18.213s user 0m17.802s sys 0m0.400s
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.3499994 array write: 14.2670002 implied write: 14.8539991 array write: 13.7550001 implied write: 14.8719997 So, it is not always all that clear.
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=0x7ffff6a8d150, t2=0x7ffff6a39540, loc1=27996512, loc2=0) at ../../trunk/gcc/ipa-devirt.c:1051 1051 { (gdb) up #1 0x0000000000654b39 in lto_symtab_merge_decls_2 (diagnosed_p=false, first=<optimized out>) at ../../trunk/gcc/lto/lto-symtab.c:679 679 DECL_SOURCE_LOCATION (decl)); (gdb) down #0 warn_types_mismatch (t1=0x7ffff6a8d150, t2=0x7ffff6a39540, loc1=27996512, loc2=0) at ../../trunk/gcc/ipa-devirt.c:1051 1051 { (gdb) call debug_tree(t1) <function_type 0x7ffff6a8d150 type <void_type 0x7ffff6c50150 void VOID align 8 symtab 0 alias set -1 structural equality pointer_to_this <pointer_type 0x7ffff6c502a0>> QI size <integer_cst 0x7ffff6c37ca8 type <integer_type 0x7ffff6c3b2a0 bitsizetype> constant 8> unit size <integer_cst 0x7ffff6c37cc0 type <integer_type 0x7ffff6c3b1f8 sizetype> constant 1> align 8 symtab 0 alias set -1 structural equality arg-types <tree_list 0x7ffff6a873e8 value <pointer_type 0x7ffff6a86738 type <record_type 0x7ffff6a867e0 st_parameter_dt> unsigned DI size <integer_cst 0x7ffff6c37bb8 constant 64> unit size <integer_cst 0x7ffff6c37bd0 constant 8> align 64 symtab 0 alias set 1 structural equality> chain <tree_list 0x7ffff6c499b0 value <void_type 0x7ffff6c50150 void>>> pointer_to_this <pointer_type 0x7ffff6aa19d8>> (gdb) call debug_tree(t2) <function_type 0x7ffff6a39540 type <void_type 0x7ffff6c50150 void VOID align 8 symtab 0 alias set -1 structural equality pointer_to_this <pointer_type 0x7ffff6c502a0>> QI size <integer_cst 0x7ffff6c37ca8 type <integer_type 0x7ffff6c3b2a0 bitsizetype> constant 8> unit size <integer_cst 0x7ffff6c37cc0 type <integer_type 0x7ffff6c3b1f8 sizetype> constant 1> align 8 symtab 0 alias set -1 structural equality attributes <tree_list 0x7ffff6a3baf0 purpose <identifier_node 0x7ffff6c69af0 fn spec> value <tree_list 0x7ffff6a3bac8 value <string_cst 0x7ffff6e26dc8 constant ".w">>> arg-types <tree_list 0x7ffff6a3baa0 value <pointer_type 0x7ffff6a39498 type <record_type 0x7ffff6a393f0 __st_parameter_dt> unsigned DI size <integer_cst 0x7ffff6c37bb8 constant 64> unit size <integer_cst 0x7ffff6c37bd0 constant 8> align 64 symtab 0 alias set 6 structural equality> chain <tree_list 0x7ffff6c499b0 value <void_type 0x7ffff6c50150 void>>> pointer_to_this <pointer_type 0x7ffff6a44150>> which looks quite different, but I lack the tree-foo to really decypher this.
> 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.
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.
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) <bb 2> [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 <bb 3> [local count: 1063004407]: # ivtmp_27 = PHI <10000000(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 <bb 4>; [1.01%] else goto <bb 3>; [98.99%] <bb 4> [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?
(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?
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 <tkoenig at gcc dot gnu.org> --- ... > 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.
Created attachment 46446 [details] 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 :-)
On June 3, 2019 6:54:10 PM GMT+02:00, "tkoenig at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org> wrote: >https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77278 > >--- Comment #9 from Thomas Koenig <tkoenig at gcc dot gnu.org> --- >Created attachment 46446 [details] > --> 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 :-)
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).
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.
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 <tkoenig at gcc dot gnu.org> --- > 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 <anonymous> *’} but argument is of type ‘struct <anonymous> *’ 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 <generic-descriptor-type> (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...
> > 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 <anonymous> *’} but > argument is of type ‘struct <anonymous> *’ > 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 <generic-descriptor-type> (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
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 <hubicka at ucw dot cz> --- > > > > 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 <anonymous> *’} but > > argument is of type ‘struct <anonymous> *’ > > 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 <generic-descriptor-type> (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?
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 <anonymous> *’} but >> argument is of type ‘struct <anonymous> *’ >> 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 <generic-descriptor-type> (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
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...
Created attachment 46451 [details] 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
(In reply to rguenther@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, the mismatched declaration types are: void <T67d> (struct array01_integer(kind=4) &, float & restrict, logical(kind=4) *) and void <T621> (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. <pointer_type 0x7ffff6616d20 type <boolean_type 0x7ffff6616c78 logical(kind=4) unsigned SI size <integer_cst 0x7ffff680cdb0 constant 32> unit-size <integer_cst 0x7ffff680cdc8 constant 4> align:32 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type 0x7ffff6616c78 precision:1 min <integer_cst 0x7ffff6a088e8 0> max <integer_cst 0x7ffff6a08900 1> pointer_to_this <pointer_type 0x7ffff6616d20>> unsigned DI size <integer_cst 0x7ffff680cb70 type <integer_type 0x7ffff68210a8 bitsizetype> constant 64> unit-size <integer_cst 0x7ffff680cb88 type <integer_type 0x7ffff6821000 sizetype> constant 8> align:64 warn_if_not_align:0 symtab:0 alias-set 3 structural-equality> <enumeral_type 0x7ffff6a06f18 GFC_LOGICAL_4 unsigned SI size <integer_cst 0x7ffff680cdb0 type <integer_type 0x7ffff68210a8 bitsizetype> constant 32> unit-size <integer_cst 0x7ffff680cdc8 type <integer_type 0x7ffff6821000 sizetype> constant 4> align:32 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type 0x7ffff6821690 precision:32 min <integer_cst 0x7ffff680cde0 0> max <integer_cst 0x7ffff680cd98 4294967295> context <translation_unit_decl 0x7ffff68161e0 ../../../ggdb3/libgfortran/generated/minloc0_4_r4.c> pointer_to_this <pointer_type 0x7ffff6616690>> 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
(In reply to Jan Hubicka from comment #20) > OK, the mismatched declaration types are: > void <T67d> (struct array01_integer(kind=4) &, float & restrict, > logical(kind=4) *) > and > void <T621> (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?
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 <tkoenig at gcc dot gnu.org> --- > (In reply to rguenther@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).
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77278 > > --- Comment #21 from Thomas Koenig <tkoenig at gcc dot gnu.org> --- > (In reply to Jan Hubicka from comment #20) > > OK, the mismatched declaration types are: > > void <T67d> (struct array01_integer(kind=4) &, float & restrict, > > logical(kind=4) *) > > and > > void <T621> (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.
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
(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.
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 <tkoenig at gcc dot gnu.org> --- > (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.
> > 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
(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 <stddef.h> #include <string.h> 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.
Created attachment 46493 [details] 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) [...]
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 <tree> (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
( > 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!