public inbox for fortran@gcc.gnu.org
 help / color / mirror / Atom feed
* Test with an lto-build of libgfortran.
@ 2023-09-27 18:21 Toon Moene
  2023-09-27 21:48 ` Jeff Law
  2023-09-28  5:33 ` Thomas Koenig
  0 siblings, 2 replies; 15+ messages in thread
From: Toon Moene @ 2023-09-27 18:21 UTC (permalink / raw)
  To: gcc mailing list; +Cc: gfortran

Hi all,

During the GNU Tools Cauldron we discussed (at the BoF: IPA & LTO) the 
possibility (and hazards) of building the run time libraries for various 
compilers with -flto, enabling an -flto -static linking of programs with 
the run time library available during link time optimizations.

Today I tried that on my (AMD Ryzen 7 5800U) laptop with

gcc version 14.0.0 20230926 (experimental) [master 
r14-4282-g53daf67fd55] (GCC)

with the following "quick hack":

diff --git a/libgfortran/configure b/libgfortran/configure
index cd176b04a14..69a2b4a8881 100755
--- a/libgfortran/configure
+++ b/libgfortran/configure
@@ -5959,11 +5959,11 @@ fi
  # Add -Wall -fno-repack-arrays -fno-underscoring if we are using GCC.
  have_real_17=no
  if test "x$GCC" = "xyes"; then
-  AM_FCFLAGS="-I . -Wall -Werror -fimplicit-none -fno-repack-arrays 
-fno-underscoring"
+  AM_FCFLAGS="-I . -Wall -Werror -fimplicit-none -fno-repack-arrays 
-fno-underscoring -flto"
    ## We like to use C11 and C99 routines when available.  This makes
    ## sure that
    ## __STDC_VERSION__ is set such that libc includes make them available.
-  AM_CFLAGS="-std=gnu11 -Wall -Wstrict-prototypes -Wmissing-prototypes 
-Wold-style-definition -Wextra -Wwrite-strings 
-Werror=implicit-function-declaration -Werror=vla"
+  AM_CFLAGS="-std=gnu11 -Wall -Wstrict-prototypes -Wmissing-prototypes 
-Wold-style-definition -Wextra -Wwrite-strings 
-Werror=implicit-function-declaration -Werror=vla -flto"
    ## Compile the following tests with the same system header contents
    ## that we'll encounter when compiling our own source files.
    CFLAGS="-std=gnu11 $CFLAGS"

The build of this compiler (languages=fortran) completed without 
problems (no test results - not enough time).

I then proceeded to build LAPACK with the following build options:

CFLAGS = -O3 -flto -flto-partition=none -static
and
FFLAGS = -O3 -flto -flto-partition=none -static

This gave the same test results of the LAPACK test suite as the build 
with the same compiler, but without an lto'd libgfortran.

The lto-ing of libgfortran did succeed, because I did get a new warning:

gfortran -O3 -flto -flto-partition=none -static  -o xlintstrfz zchkrfp.o 
zdrvrfp.o zdrvrf1.o zdrvrf2.o zdrvrf3.o zdrvrf4.o zerrrfp.o zlatb4.o 
zlaipd.o zlarhs.o zsbmv.o zget04.o zpot01.o zpot03.o zpot02.o chkxer.o 
xerbla.o alaerh.o aladhd.o alahd.o alasvm.o ../../libtmglib.a 
../../liblapack.a ../../librefblas.a
In function 'xtoa_big',
     inlined from 'write_z' at 
/home/toon/compilers/gcc/libgfortran/io/write.c:1296:11,
     inlined from 'formatted_transfer_scalar_write' at 
/home/toon/compilers/gcc/libgfortran/io/transfer.c:2136:4:
/home/toon/compilers/gcc/libgfortran/io/write.c:1222:6: warning: writing 
1 byte into a region of size 0 [-Wstringop-overflow=]
  1222 |   *q = '\0';
       |      ^
/home/toon/compilers/gcc/libgfortran/io/write.c: In function 
'formatted_transfer_scalar_write':
/home/toon/compilers/gcc/libgfortran/io/write.c:1291:8: note: at offset 
[34, 4294967294] into destination object 'itoa_buf' of size 33
  1291 |   char itoa_buf[GFC_XTOA_BUF_SIZE];
       |        ^

which was (of course) not given with a non-lto libgfortran.

The full question of "lto-ing" run time libraries is more complicated 
than just "whether it works" as those who attended the BoF will recall.

Hope this helps,

-- 
Toon Moene - e-mail: toon@moene.org - phone: +31 346 214290
Saturnushof 14, 3738 XG  Maartensdijk, The Netherlands

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Test with an lto-build of libgfortran.
  2023-09-27 18:21 Test with an lto-build of libgfortran Toon Moene
@ 2023-09-27 21:48 ` Jeff Law
  2023-09-28  6:25   ` Richard Biener
  2023-09-28  5:33 ` Thomas Koenig
  1 sibling, 1 reply; 15+ messages in thread
From: Jeff Law @ 2023-09-27 21:48 UTC (permalink / raw)
  To: Toon Moene, gcc mailing list; +Cc: gfortran



On 9/27/23 12:21, Toon Moene wrote:

> 
> The lto-ing of libgfortran did succeed, because I did get a new warning:
> 
> gfortran -O3 -flto -flto-partition=none -static  -o xlintstrfz zchkrfp.o 
> zdrvrfp.o zdrvrf1.o zdrvrf2.o zdrvrf3.o zdrvrf4.o zerrrfp.o zlatb4.o 
> zlaipd.o zlarhs.o zsbmv.o zget04.o zpot01.o zpot03.o zpot02.o chkxer.o 
> xerbla.o alaerh.o aladhd.o alahd.o alasvm.o ../../libtmglib.a 
> ../../liblapack.a ../../librefblas.a
> In function 'xtoa_big',
>      inlined from 'write_z' at 
> /home/toon/compilers/gcc/libgfortran/io/write.c:1296:11,
>      inlined from 'formatted_transfer_scalar_write' at 
> /home/toon/compilers/gcc/libgfortran/io/transfer.c:2136:4:
> /home/toon/compilers/gcc/libgfortran/io/write.c:1222:6: warning: writing 
> 1 byte into a region of size 0 [-Wstringop-overflow=]
>   1222 |   *q = '\0';
>        |      ^
> /home/toon/compilers/gcc/libgfortran/io/write.c: In function 
> 'formatted_transfer_scalar_write':
> /home/toon/compilers/gcc/libgfortran/io/write.c:1291:8: note: at offset 
> [34, 4294967294] into destination object 'itoa_buf' of size 33
>   1291 |   char itoa_buf[GFC_XTOA_BUF_SIZE];
>        |        ^
> 
> which was (of course) not given with a non-lto libgfortran.
Yea.  This certainly can happen with LTO.  These warnings would 
definitely be something worth investigating.

Essentially the inlining enabled by LTO can expose a different set of 
diagnostics.

Jeff

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Test with an lto-build of libgfortran.
  2023-09-27 18:21 Test with an lto-build of libgfortran Toon Moene
  2023-09-27 21:48 ` Jeff Law
@ 2023-09-28  5:33 ` Thomas Koenig
  2023-09-28 19:03   ` Toon Moene
  1 sibling, 1 reply; 15+ messages in thread
From: Thomas Koenig @ 2023-09-28  5:33 UTC (permalink / raw)
  To: Toon Moene, gcc mailing list; +Cc: gfortran

Hi Toon,

> During the GNU Tools Cauldron we discussed (at the BoF: IPA & LTO) the 
> possibility (and hazards) of building the run time libraries for various 
> compilers with -flto, enabling an -flto -static linking of programs with 
> the run time library available during link time optimizations.

This would be a big win for libgfortran, especially the array functions,
knowing that stride==1 can be a _big_ win for optimization.  This is
why LTO is such an excellent idea for Fortran in general, and for the
library in particular.

There is a PR on this with quite some discussion already,
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77278 .

I've put you in CC of that bug, maybe we can discuss there more
in detail.

One point about the array functions: In the library, we use sort of a
ripple carry algorithm to step through the arrays.  This saves space
an is general, but the performance (esp for the most common one-and
two-dimensional arrays) can suffer.

[...]

> The full question of "lto-ing" run time libraries is more complicated 
> than just "whether it works" as those who attended the BoF will recall.

I didn't attend the Cauldron (but that discussion would have been
very interesting).  I think for libgfortran, a first step would be
additional work to get declarations on both sides to agree (which is
worth doing anyway).

Best regards

	Thomas



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Test with an lto-build of libgfortran.
  2023-09-27 21:48 ` Jeff Law
@ 2023-09-28  6:25   ` Richard Biener
  2023-09-28  6:29     ` Andrew Pinski
  2023-09-28  7:29     ` Tobias Burnus
  0 siblings, 2 replies; 15+ messages in thread
From: Richard Biener @ 2023-09-28  6:25 UTC (permalink / raw)
  To: Jeff Law; +Cc: Toon Moene, gcc mailing list, gfortran

On Wed, Sep 27, 2023 at 11:48 PM Jeff Law via Fortran
<fortran@gcc.gnu.org> wrote:
>
>
>
> On 9/27/23 12:21, Toon Moene wrote:
>
> >
> > The lto-ing of libgfortran did succeed, because I did get a new warning:
> >
> > gfortran -O3 -flto -flto-partition=none -static  -o xlintstrfz zchkrfp.o
> > zdrvrfp.o zdrvrf1.o zdrvrf2.o zdrvrf3.o zdrvrf4.o zerrrfp.o zlatb4.o
> > zlaipd.o zlarhs.o zsbmv.o zget04.o zpot01.o zpot03.o zpot02.o chkxer.o
> > xerbla.o alaerh.o aladhd.o alahd.o alasvm.o ../../libtmglib.a
> > ../../liblapack.a ../../librefblas.a
> > In function 'xtoa_big',
> >      inlined from 'write_z' at
> > /home/toon/compilers/gcc/libgfortran/io/write.c:1296:11,
> >      inlined from 'formatted_transfer_scalar_write' at
> > /home/toon/compilers/gcc/libgfortran/io/transfer.c:2136:4:
> > /home/toon/compilers/gcc/libgfortran/io/write.c:1222:6: warning: writing
> > 1 byte into a region of size 0 [-Wstringop-overflow=]
> >   1222 |   *q = '\0';
> >        |      ^
> > /home/toon/compilers/gcc/libgfortran/io/write.c: In function
> > 'formatted_transfer_scalar_write':
> > /home/toon/compilers/gcc/libgfortran/io/write.c:1291:8: note: at offset
> > [34, 4294967294] into destination object 'itoa_buf' of size 33
> >   1291 |   char itoa_buf[GFC_XTOA_BUF_SIZE];
> >        |        ^
> >
> > which was (of course) not given with a non-lto libgfortran.
> Yea.  This certainly can happen with LTO.  These warnings would
> definitely be something worth investigating.
>
> Essentially the inlining enabled by LTO can expose a different set of
> diagnostics.

This particular place in libgfortran has

  /* write_z, which calls xtoa_big, is called from transfer.c,
     formatted_transfer_scalar_write.  There it is passed the kind as
     argument, which means a maximum of 16.  The buffer is large
     enough, but the compiler does not know that, so shut up the
     warning here.  */
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wstringop-overflow"
  *q = '\0';
#pragma GCC diagnostic pop

so obviously the #pragma doesn't survive through LTO.  Somehow I think
this is a known bug, but maybe I misremember (I think we are not streaming
any of the ad-hoc location parts).

Richard.

>
> Jeff

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Test with an lto-build of libgfortran.
  2023-09-28  6:25   ` Richard Biener
@ 2023-09-28  6:29     ` Andrew Pinski
  2023-09-28  7:29     ` Tobias Burnus
  1 sibling, 0 replies; 15+ messages in thread
From: Andrew Pinski @ 2023-09-28  6:29 UTC (permalink / raw)
  To: Richard Biener; +Cc: Jeff Law, gcc mailing list, gfortran

On Wed, Sep 27, 2023 at 11:28 PM Richard Biener via Fortran
<fortran@gcc.gnu.org> wrote:
>
> On Wed, Sep 27, 2023 at 11:48 PM Jeff Law via Fortran
> <fortran@gcc.gnu.org> wrote:
> >
> >
> >
> > On 9/27/23 12:21, Toon Moene wrote:
> >
> > >
> > > The lto-ing of libgfortran did succeed, because I did get a new warning:
> > >
> > > gfortran -O3 -flto -flto-partition=none -static  -o xlintstrfz zchkrfp.o
> > > zdrvrfp.o zdrvrf1.o zdrvrf2.o zdrvrf3.o zdrvrf4.o zerrrfp.o zlatb4.o
> > > zlaipd.o zlarhs.o zsbmv.o zget04.o zpot01.o zpot03.o zpot02.o chkxer.o
> > > xerbla.o alaerh.o aladhd.o alahd.o alasvm.o ../../libtmglib.a
> > > ../../liblapack.a ../../librefblas.a
> > > In function 'xtoa_big',
> > >      inlined from 'write_z' at
> > > /home/toon/compilers/gcc/libgfortran/io/write.c:1296:11,
> > >      inlined from 'formatted_transfer_scalar_write' at
> > > /home/toon/compilers/gcc/libgfortran/io/transfer.c:2136:4:
> > > /home/toon/compilers/gcc/libgfortran/io/write.c:1222:6: warning: writing
> > > 1 byte into a region of size 0 [-Wstringop-overflow=]
> > >   1222 |   *q = '\0';
> > >        |      ^
> > > /home/toon/compilers/gcc/libgfortran/io/write.c: In function
> > > 'formatted_transfer_scalar_write':
> > > /home/toon/compilers/gcc/libgfortran/io/write.c:1291:8: note: at offset
> > > [34, 4294967294] into destination object 'itoa_buf' of size 33
> > >   1291 |   char itoa_buf[GFC_XTOA_BUF_SIZE];
> > >        |        ^
> > >
> > > which was (of course) not given with a non-lto libgfortran.
> > Yea.  This certainly can happen with LTO.  These warnings would
> > definitely be something worth investigating.
> >
> > Essentially the inlining enabled by LTO can expose a different set of
> > diagnostics.
>
> This particular place in libgfortran has
>
>   /* write_z, which calls xtoa_big, is called from transfer.c,
>      formatted_transfer_scalar_write.  There it is passed the kind as
>      argument, which means a maximum of 16.  The buffer is large
>      enough, but the compiler does not know that, so shut up the
>      warning here.  */
> #pragma GCC diagnostic push
> #pragma GCC diagnostic ignored "-Wstringop-overflow"
>   *q = '\0';
> #pragma GCC diagnostic pop
>
> so obviously the #pragma doesn't survive through LTO.  Somehow I think
> this is a known bug, but maybe I misremember (I think we are not streaming
> any of the ad-hoc location parts).

Yes it is a known bug.
See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80922 .

Thanks,
Andrew


>
> Richard.
>
> >
> > Jeff

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Test with an lto-build of libgfortran.
  2023-09-28  6:25   ` Richard Biener
  2023-09-28  6:29     ` Andrew Pinski
@ 2023-09-28  7:29     ` Tobias Burnus
  2023-09-28  9:51       ` Jakub Jelinek
  1 sibling, 1 reply; 15+ messages in thread
From: Tobias Burnus @ 2023-09-28  7:29 UTC (permalink / raw)
  To: Richard Biener, Thomas Koenig, Toon Moene
  Cc: gcc mailing list, gfortran, Jeff Law

[-- Attachment #1: Type: text/plain, Size: 1399 bytes --]

Hi all,

the following works for me. I have only tried a normal build (where it
does silence the same warning) and not an LTO build and I just believed
the comment - see attached patch. Comments?

On 28.09.23 08:25, Richard Biener via Fortran wrote:

> This particular place in libgfortran has
>
>    /* write_z, which calls xtoa_big, is called from transfer.c,
>       formatted_transfer_scalar_write.  There it is passed the kind as
>       argument, which means a maximum of 16.  The buffer is large
>       enough, but the compiler does not know that, so shut up the
>       warning here.  */
> #pragma GCC diagnostic push
> #pragma GCC diagnostic ignored "-Wstringop-overflow"
>    *q = '\0';
> #pragma GCC diagnostic pop
>
> so obviously the #pragma doesn't survive through LTO.  Somehow I think
> this is a known bug, but maybe I misremember (I think we are not streaming
> any of the ad-hoc location parts).

I have replaced it now by the assert that "len <= 16", i.e.

+  if (len > 16)
+    __builtin_unreachable ();

Build + tested on x86-64-gnu-linux
Comment? OK for mainline?

Tobias
-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955

[-- Attachment #2: xtoa_big-silence-warning.diff --]
[-- Type: text/x-patch, Size: 1583 bytes --]

libgfortran: Use __builtin_unreachable() not -Wno-stringop-overflow to silence warning

libgfortran/
	* io/write.c (xtoa_big): Change a 'GCC diagnostic ignored
	"-Wstringop-overflow"' to an assumption (via __builtin_unreachable).

 libgfortran/io/write.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/libgfortran/io/write.c b/libgfortran/io/write.c
index 5d47a6d25f7..00c8fd2e288 100644
--- a/libgfortran/io/write.c
+++ b/libgfortran/io/write.c
@@ -1179,6 +1179,15 @@ xtoa_big (const char *s, char *buffer, int len, GFC_UINTEGER_LARGEST *n)
   uint8_t h, l;
   int i;
 
+  /* write_z, which calls xtoa_big, is called from transfer.c,
+     formatted_transfer_scalar_write.  There it is passed the kind as
+     'len' argument, which means a maximum of 16.  The buffer is large
+     enough, but the compiler does not know that, so shut up the
+     warning here.  */
+
+  if (len > 16)
+    __builtin_unreachable ();
+
   q = buffer;
 
   if (__BYTE_ORDER__ == __ORDER_BIG_ENDIAN__)
@@ -1212,15 +1221,7 @@ xtoa_big (const char *s, char *buffer, int len, GFC_UINTEGER_LARGEST *n)
 	}
     }
 
-  /* write_z, which calls xtoa_big, is called from transfer.c,
-     formatted_transfer_scalar_write.  There it is passed the kind as
-     argument, which means a maximum of 16.  The buffer is large
-     enough, but the compiler does not know that, so shut up the
-     warning here.  */
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wstringop-overflow"
   *q = '\0';
-#pragma GCC diagnostic pop
 
   if (*n == 0)
     return "0";

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Test with an lto-build of libgfortran.
  2023-09-28  7:29     ` Tobias Burnus
@ 2023-09-28  9:51       ` Jakub Jelinek
  2023-09-28 11:00         ` Tobias Burnus
  0 siblings, 1 reply; 15+ messages in thread
From: Jakub Jelinek @ 2023-09-28  9:51 UTC (permalink / raw)
  To: Tobias Burnus
  Cc: Richard Biener, Thomas Koenig, Toon Moene, gcc mailing list,
	gfortran, Jeff Law

On Thu, Sep 28, 2023 at 09:29:02AM +0200, Tobias Burnus wrote:
> the following works for me. I have only tried a normal build (where it
> does silence the same warning) and not an LTO build and I just believed
> the comment - see attached patch. Comments?
> 
> On 28.09.23 08:25, Richard Biener via Fortran wrote:
> 
> > This particular place in libgfortran has
> > 
> >    /* write_z, which calls xtoa_big, is called from transfer.c,
> >       formatted_transfer_scalar_write.  There it is passed the kind as
> >       argument, which means a maximum of 16.  The buffer is large
> >       enough, but the compiler does not know that, so shut up the
> >       warning here.  */
> > #pragma GCC diagnostic push
> > #pragma GCC diagnostic ignored "-Wstringop-overflow"
> >    *q = '\0';
> > #pragma GCC diagnostic pop
> > 
> > so obviously the #pragma doesn't survive through LTO.  Somehow I think
> > this is a known bug, but maybe I misremember (I think we are not streaming
> > any of the ad-hoc location parts).
> 
> I have replaced it now by the assert that "len <= 16", i.e.
> 
> +  if (len > 16)
> +    __builtin_unreachable ();
> 
> Build + tested on x86-64-gnu-linux
> Comment? OK for mainline?

Is it just that in correct programs len can't be > 16, or that it is really
impossible for it being > 16?  I mean, we have that artificial kind 17 for
powerpc which better should be turned into length of 16, but isn't e.g.
_gfortran_transfer_integer etc. just called with a kind argument?  Does
anything error earlier if it is larger?  I mean, say user calling
_gfortan_transfer_integer by hand with kind 1024?

Sure, we could still say it is UB to do that kind of thing and
__builtin_unreachable () would be a way to turn that UB into manifestly
reproducable UB.

	Jakub


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Test with an lto-build of libgfortran.
  2023-09-28  9:51       ` Jakub Jelinek
@ 2023-09-28 11:00         ` Tobias Burnus
  2023-09-28 11:02           ` Jakub Jelinek
  0 siblings, 1 reply; 15+ messages in thread
From: Tobias Burnus @ 2023-09-28 11:00 UTC (permalink / raw)
  To: Jakub Jelinek
  Cc: Richard Biener, Thomas Koenig, Toon Moene, gfortran, Jeff Law,
	gcc-patches

(replace gcc@ by gcc-patches@; see
https://gcc.gnu.org/pipermail/gcc/2023-September/242591.html
and other emails in that thread)

On 28.09.23 11:51, Jakub Jelinek wrote:
> On Thu, Sep 28, 2023 at 09:29:02AM +0200, Tobias Burnus wrote:
>> On 28.09.23 08:25, Richard Biener via Fortran wrote:
>>
>>> This particular place in libgfortran has
>>>
>>>     /* write_z, which calls xtoa_big, is called from transfer.c,
>>>        formatted_transfer_scalar_write.  There it is passed the kind as
>>>        argument, which means a maximum of 16.  The buffer is large
>>>        enough, but the compiler does not know that, so shut up the
>>>        warning here.  */
...
>> I have replaced it now by the assert that "len <= 16", i.e.
>> +  if (len > 16)
>> +    __builtin_unreachable ();
> Is it just that in correct programs len can't be > 16, or that it is really
> impossible for it being > 16?  I mean, we have that artificial kind 17 for
> powerpc which better should be turned into length of 16, but isn't e.g.
> _gfortran_transfer_integer etc.

My understanding is that kind=17 only pops up on PowerPC
for REAL variables as they represent __float128 in multiple ways.

Having said that, the current call tree is:

* xtoa_big: that's where the warning suppression
   was replaced by the unreachable.

* Only caller is 'write_z' with calls it by passing its
   last argument ('len') as last argument ('len')

* "internal_proto(write_z)" implies that it is not called from
   outside libgfortran. The internal only caller is:

*  formatted_transfer_scalar_write, which calls it as:

         case FMT_Z:
           ...
#ifdef HAVE_GFC_REAL_17
           if (type == BT_REAL && kind == 17)
             kind = 16;
#endif
           write_z (dtp, f, p, kind);

I am not aware of any logigal/integer/real(+comples)/character kind > 16,
except for this PPC one. And complex numbers are pairs of BT_REAL.

Thus, I think that patch should be fine - except:

> Does anything error earlier if it is larger?  I mean, say user calling
> _gfortan_transfer_integer by hand with kind 1024?

I think this will fail. We have various ways to deal with this in libgfortran;
I see some cases where the switch "default:" sets the length to 0; we have
other places where we use an "assert", I think we have other places were
we run into UB.

Thus, one option would be to either 'assert(len <= 16)' or
'assert((size_t)len < GFC_OTOA_BUF_SIZE - 1)' instead.

Or we could handle it as len=0 and silently ignore the output or ...

I am fine with either of the many options - except that I like something
explicit involving 'len' and a comparison (unreachable, assert, regarding as len = 0)
better than the existing warning suppression which is too indirect for
me. (Besides: it does not work for LTO.) Preferences? Tobias

-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Test with an lto-build of libgfortran.
  2023-09-28 11:00         ` Tobias Burnus
@ 2023-09-28 11:02           ` Jakub Jelinek
  0 siblings, 0 replies; 15+ messages in thread
From: Jakub Jelinek @ 2023-09-28 11:02 UTC (permalink / raw)
  To: Tobias Burnus
  Cc: Richard Biener, Thomas Koenig, Toon Moene, gfortran, Jeff Law,
	gcc-patches

On Thu, Sep 28, 2023 at 01:00:41PM +0200, Tobias Burnus wrote:
> I am not aware of any logigal/integer/real(+comples)/character kind > 16,
> except for this PPC one. And complex numbers are pairs of BT_REAL.
> 
> Thus, I think that patch should be fine - except:
> 
> > Does anything error earlier if it is larger?  I mean, say user calling
> > _gfortan_transfer_integer by hand with kind 1024?
> 
> I think this will fail. We have various ways to deal with this in libgfortran;
> I see some cases where the switch "default:" sets the length to 0; we have
> other places where we use an "assert", I think we have other places were
> we run into UB.
> 
> Thus, one option would be to either 'assert(len <= 16)' or
> 'assert((size_t)len < GFC_OTOA_BUF_SIZE - 1)' instead.
> 
> Or we could handle it as len=0 and silently ignore the output or ...
> 
> I am fine with either of the many options - except that I like something
> explicit involving 'len' and a comparison (unreachable, assert, regarding as len = 0)
> better than the existing warning suppression which is too indirect for
> me. (Besides: it does not work for LTO.) Preferences? Tobias

Let's go with the __builtin_unreachable, ok for trunk.

	Jakub


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Test with an lto-build of libgfortran.
  2023-09-28  5:33 ` Thomas Koenig
@ 2023-09-28 19:03   ` Toon Moene
  2023-09-28 19:26     ` Jakub Jelinek
  0 siblings, 1 reply; 15+ messages in thread
From: Toon Moene @ 2023-09-28 19:03 UTC (permalink / raw)
  To: Thomas Koenig, gcc mailing list; +Cc: gfortran

On 9/28/23 07:33, Thomas Koenig wrote:

> Hi Toon,

[ I wrote: ]

>> The full question of "lto-ing" run time libraries is more complicated 
>> than just "whether it works" as those who attended the BoF will recall.
> 
> I didn't attend the Cauldron (but that discussion would have been
> very interesting).  I think for libgfortran, a first step would be
> additional work to get declarations on both sides to agree (which is
> worth doing anyway).
> 
> Best regards
> 
>      Thomas

The big problem in *distributing* GCC (i.e., the collection) with lto'd 
run-time libraries is that the format of the lto structure changes with 
releases. If a compiler (by accident) picks up a run time library with 
non-matching lto objects, it might crash (or "introduce subtle errors in 
a once working program").

I.e., like the problem the gfortran community had with the changing 
format of our .mod files.

But it would be a big win for Fortran ...

Kind regards,

-- 
Toon Moene - e-mail: toon@moene.org - phone: +31 346 214290
Saturnushof 14, 3738 XG  Maartensdijk, The Netherlands


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Test with an lto-build of libgfortran.
  2023-09-28 19:03   ` Toon Moene
@ 2023-09-28 19:26     ` Jakub Jelinek
  2023-09-28 19:59       ` Toon Moene
  2023-09-29  6:03       ` Thomas Koenig
  0 siblings, 2 replies; 15+ messages in thread
From: Jakub Jelinek @ 2023-09-28 19:26 UTC (permalink / raw)
  To: Toon Moene; +Cc: Thomas Koenig, gcc mailing list, gfortran

On Thu, Sep 28, 2023 at 09:03:39PM +0200, Toon Moene wrote:
> > > The full question of "lto-ing" run time libraries is more
> > > complicated than just "whether it works" as those who attended the
> > > BoF will recall.
> > 
> > I didn't attend the Cauldron (but that discussion would have been
> > very interesting).  I think for libgfortran, a first step would be
> > additional work to get declarations on both sides to agree (which is
> > worth doing anyway).
> > 
> > Best regards
> > 
> >      Thomas
> 
> The big problem in *distributing* GCC (i.e., the collection) with lto'd
> run-time libraries is that the format of the lto structure changes with
> releases. If a compiler (by accident) picks up a run time library with
> non-matching lto objects, it might crash (or "introduce subtle errors in a
> once working program").

It is worse than that, usually the LTO format changes e.g. any time any
option or parameter is added on a release branch (several times a year) and
at other times as well.
Though, admittedly GCC is the single package that actually could get away
with LTO in lib*.a libraries, at least in some packagings (if the static
libraries are in gcc specific subdirectories rather than say /usr/lib{,64}
or similar and if the packaging of gcc updates both the compiler and
corresponding static libraries in a lock-step.  Because in that case LTO
in there will be always used only by the same snapshot from the release
branch and so should be compatible with the LTO in it.

	Jakub


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Test with an lto-build of libgfortran.
  2023-09-28 19:26     ` Jakub Jelinek
@ 2023-09-28 19:59       ` Toon Moene
  2023-09-28 20:02         ` David Edelsohn
  2023-09-29 10:26         ` Andrew Stubbs
  2023-09-29  6:03       ` Thomas Koenig
  1 sibling, 2 replies; 15+ messages in thread
From: Toon Moene @ 2023-09-28 19:59 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Thomas Koenig, gcc mailing list, gfortran

On 9/28/23 21:26, Jakub Jelinek wrote:

> It is worse than that, usually the LTO format changes e.g. any time any
> option or parameter is added on a release branch (several times a year) and
> at other times as well.
> Though, admittedly GCC is the single package that actually could get away
> with LTO in lib*.a libraries, at least in some packagings (if the static
> libraries are in gcc specific subdirectories rather than say /usr/lib{,64}
> or similar and if the packaging of gcc updates both the compiler and
> corresponding static libraries in a lock-step.  Because in that case LTO
> in there will be always used only by the same snapshot from the release
> branch and so should be compatible with the LTO in it.
This might be an argument to make it a configure option, e.g. 
--enable-lto-runtime.

Kind regards,

-- 
Toon Moene - e-mail: toon@moene.org - phone: +31 346 214290
Saturnushof 14, 3738 XG  Maartensdijk, The Netherlands


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Test with an lto-build of libgfortran.
  2023-09-28 19:59       ` Toon Moene
@ 2023-09-28 20:02         ` David Edelsohn
  2023-09-29 10:26         ` Andrew Stubbs
  1 sibling, 0 replies; 15+ messages in thread
From: David Edelsohn @ 2023-09-28 20:02 UTC (permalink / raw)
  To: Toon Moene; +Cc: Jakub Jelinek, Thomas Koenig, gcc mailing list, gfortran

[-- Attachment #1: Type: text/plain, Size: 1025 bytes --]

On Thu, Sep 28, 2023 at 4:00 PM Toon Moene <toon@moene.org> wrote:

> On 9/28/23 21:26, Jakub Jelinek wrote:
>
> > It is worse than that, usually the LTO format changes e.g. any time any
> > option or parameter is added on a release branch (several times a year)
> and
> > at other times as well.
> > Though, admittedly GCC is the single package that actually could get away
> > with LTO in lib*.a libraries, at least in some packagings (if the static
> > libraries are in gcc specific subdirectories rather than say
> /usr/lib{,64}
> > or similar and if the packaging of gcc updates both the compiler and
> > corresponding static libraries in a lock-step.  Because in that case LTO
> > in there will be always used only by the same snapshot from the release
> > branch and so should be compatible with the LTO in it.
> This might be an argument to make it a configure option, e.g.
> --enable-lto-runtime.
>
> Note that not all targets support LTO, so the option cannot be added
unilaterally.

David

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Test with an lto-build of libgfortran.
  2023-09-28 19:26     ` Jakub Jelinek
  2023-09-28 19:59       ` Toon Moene
@ 2023-09-29  6:03       ` Thomas Koenig
  1 sibling, 0 replies; 15+ messages in thread
From: Thomas Koenig @ 2023-09-29  6:03 UTC (permalink / raw)
  To: Jakub Jelinek, Toon Moene; +Cc: gcc mailing list, gfortran

Hi Jakub,

> It is worse than that, usually the LTO format changes e.g. any time any
> option or parameter is added on a release branch (several times a year) and
> at other times as well.

Hm, that makes LTO not very well suited for libraries...

> Though, admittedly GCC is the single package that actually could get away
> with LTO in lib*.a libraries, at least in some packagings (if the static
> libraries are in gcc specific subdirectories rather than say /usr/lib{,64}
> or similar and if the packaging of gcc updates both the compiler and
> corresponding static libraries in a lock-step.  Because in that case LTO
> in there will be always used only by the same snapshot from the release
> branch and so should be compatible with the LTO in it.

Maybe another approach: Instead of storing version-dependent LTO code in
the *.a files, we could just store preprocessed C code there, somewhat
shortened by removing whitespace, comments and unused declarations
(and/or store them in compressed format).

This would also allow the something like sanitizers to look into the
runtime libraries, and other instrumentation.

It would also be a much more extensive project, also involving
modifications to the linker. Not sure how realistic that would be.

Best regards

	Thomas


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Test with an lto-build of libgfortran.
  2023-09-28 19:59       ` Toon Moene
  2023-09-28 20:02         ` David Edelsohn
@ 2023-09-29 10:26         ` Andrew Stubbs
  1 sibling, 0 replies; 15+ messages in thread
From: Andrew Stubbs @ 2023-09-29 10:26 UTC (permalink / raw)
  To: Toon Moene, Jakub Jelinek; +Cc: Thomas Koenig, gcc mailing list, gfortran

On 28/09/2023 20:59, Toon Moene wrote:
> On 9/28/23 21:26, Jakub Jelinek wrote:
> 
>> It is worse than that, usually the LTO format changes e.g. any time any
>> option or parameter is added on a release branch (several times a 
>> year) and
>> at other times as well.
>> Though, admittedly GCC is the single package that actually could get away
>> with LTO in lib*.a libraries, at least in some packagings (if the static
>> libraries are in gcc specific subdirectories rather than say 
>> /usr/lib{,64}
>> or similar and if the packaging of gcc updates both the compiler and
>> corresponding static libraries in a lock-step.  Because in that case LTO
>> in there will be always used only by the same snapshot from the release
>> branch and so should be compatible with the LTO in it.
> This might be an argument to make it a configure option, e.g. 
> --enable-lto-runtime.

This sort of thing should definitely Just Work for cross compilers and 
embedded platforms where the libraries are bundled with the compiler.

Andrew

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2023-09-29 10:26 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-27 18:21 Test with an lto-build of libgfortran Toon Moene
2023-09-27 21:48 ` Jeff Law
2023-09-28  6:25   ` Richard Biener
2023-09-28  6:29     ` Andrew Pinski
2023-09-28  7:29     ` Tobias Burnus
2023-09-28  9:51       ` Jakub Jelinek
2023-09-28 11:00         ` Tobias Burnus
2023-09-28 11:02           ` Jakub Jelinek
2023-09-28  5:33 ` Thomas Koenig
2023-09-28 19:03   ` Toon Moene
2023-09-28 19:26     ` Jakub Jelinek
2023-09-28 19:59       ` Toon Moene
2023-09-28 20:02         ` David Edelsohn
2023-09-29 10:26         ` Andrew Stubbs
2023-09-29  6:03       ` Thomas Koenig

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).