Re: Test with an lto-build of libgfortran.
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
Re: Test with an lto-build of libgfortran.
On Thu, Sep 28, 2023 at 4:00 PM 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.
>
> Note that not all targets support LTO, so the option cannot be added
unilaterally.
David
Re: Test with an lto-build of libgfortran.
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: t...@moene.org - phone: +31 346 214290
Saturnushof 14, 3738 XG Maartensdijk, The Netherlands
Re: Test with an lto-build of libgfortran.
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
Re: Test with an lto-build of libgfortran.
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: t...@moene.org - phone: +31 346 214290 Saturnushof 14, 3738 XG Maartensdijk, The Netherlands
Re: [Fortran, Patch, Coarray, PR 37336] Fix crash in finalizer when derived type coarray is already freed.
Hi Andre, The patch looks fine to me. Since you mention it in the comment, is it worth declaring the derived type 'foo' in a module and giving it a final routine? Thanks for the patch. Paul On Thu, 28 Sept 2023 at 13:45, Andre Vehreschild via Fortran wrote: > > Hi all, > > attached patch fixes a crash in coarray programs when an allocatable derived > typed coarray was freed explicitly. The generated cleanup code did not take > into account, that the coarray may have been deallocated already. The patch > fixes this by moving the statements accessing components inside the derived > type > into the block guard by its allocated check. > > Regtested ok on f37/x86_64. Ok for master? > > Regards, > Andre > -- > Andre Vehreschild * Email: vehre ad gmx dot de
Re: [EXT] gfortran: error: unrecognized argument in option '-mcmodel=medium'
Hi, > On 28 Sep 2023, at 14:56, Lingadahally, Vishakha (2023) > wrote: > Could you please let me know if there happens to be an update regarding how I > might be able to resolve it? when you are on an arm64 machine you need to add "-mcmodel=small” (applies to Arm) instead of "-mcmodel=medium” (applies to Intel) Iain > > Thanks! > > Warm regards, > Vishakha > > From: Iain Sandoe > Sent: 27 September 2023 08:35 > To: Lingadahally, Vishakha (2023) > Cc: GCC Fortran > Subject: [EXT] Re: gfortran: error: unrecognized argument in option > '-mcmodel=medium' > > > >> On 27 Sep 2023, at 08:33, Iain Sandoe wrote: >> >> >> >>> On 27 Sep 2023, at 08:25, Andrew Pinski via Fortran >>> wrote: >>> >>> On Tue, Sep 26, 2023 at 11:39 PM Richard Biener via Fortran >>> wrote: On Tue, Sep 26, 2023 at 4:44 PM Lingadahally, Vishakha (2023) wrote: > > Dear GCC Team, > > I'm running Ubuntu 22 on my Mac virtually and my gfortran version is > 11.4.0. When I try to install a certain software package, I encounter the > following error: > > gfortran: error: unrecognized argument in option '-mcmodel=medium' > gfortran: note: valid arguments to '-mcmodel=' are: large small tiny > > Is this due to attempting to run gfortran on arm64 architecture? Could > you please let me know how I could resolve the issue? You have to turn to Ubuntu here, -mcmodel=medium is certainly supported in GCC 11, maybe Ubuntu patches out the support? >>> >>> Well -mcmodel=medium is the x86_64 specific option while they are >>> trying to run on aarch64 which has a different set options. >> >> >> The current Arm64 port (aarch64 on macOS) only supports the “small” mcmodel >> “tiny” is not supported by macOS and we have not yet implemented “large”. >> >> Despite it’s name “small” should suffice as an approximate equivalent for the >> x86_64 “medium” (at least on macOS). >> >> So, either you need to change the flag depending on the architecture, or >> omit it >> (on macOS currently GCC only supports the default mcmodel). >> >> We plan to support the “large” model at some stage on Arm64 (but unlikely for >> GCC 14) > > Ah, and now I see you are not running native so the comments above are not > relevant. > Iain > > > This email, its contents and any attachments are intended solely for the > addressee and may contain confidential information. In certain circumstances, > it may also be subject to legal privilege. Any unauthorised use, disclosure, > or copying is not permitted. If you have received this email in error, please > notify us and immediately and permanently delete it. Any views or opinions > expressed in personal emails are solely those of the author and do not > necessarily represent those of Royal Holloway, University of London. It is > your responsibility to ensure that this email and any attachments are virus > free.
Re: [EXT] Re: gfortran: error: unrecognized argument in option '-mcmodel=medium'
Hi, Could you please let me know if there happens to be an update regarding how I might be able to resolve it? Thanks! Warm regards, Vishakha From: Iain Sandoe Sent: 27 September 2023 08:35 To: Lingadahally, Vishakha (2023) Cc: GCC Fortran Subject: [EXT] Re: gfortran: error: unrecognized argument in option '-mcmodel=medium' > On 27 Sep 2023, at 08:33, Iain Sandoe wrote: > > > >> On 27 Sep 2023, at 08:25, Andrew Pinski via Fortran >> wrote: >> >> On Tue, Sep 26, 2023 at 11:39 PM Richard Biener via Fortran >> wrote: >>> >>> On Tue, Sep 26, 2023 at 4:44 PM Lingadahally, Vishakha (2023) >>> wrote: Dear GCC Team, I'm running Ubuntu 22 on my Mac virtually and my gfortran version is 11.4.0. When I try to install a certain software package, I encounter the following error: gfortran: error: unrecognized argument in option '-mcmodel=medium' gfortran: note: valid arguments to '-mcmodel=' are: large small tiny Is this due to attempting to run gfortran on arm64 architecture? Could you please let me know how I could resolve the issue? >>> >>> You have to turn to Ubuntu here, -mcmodel=medium is certainly >>> supported in GCC 11, maybe Ubuntu patches out >>> the support? >> >> Well -mcmodel=medium is the x86_64 specific option while they are >> trying to run on aarch64 which has a different set options. > > > The current Arm64 port (aarch64 on macOS) only supports the “small” mcmodel > “tiny” is not supported by macOS and we have not yet implemented “large”. > > Despite it’s name “small” should suffice as an approximate equivalent for the > x86_64 “medium” (at least on macOS). > > So, either you need to change the flag depending on the architecture, or omit > it > (on macOS currently GCC only supports the default mcmodel). > > We plan to support the “large” model at some stage on Arm64 (but unlikely for > GCC 14) Ah, and now I see you are not running native so the comments above are not relevant. Iain This email, its contents and any attachments are intended solely for the addressee and may contain confidential information. In certain circumstances, it may also be subject to legal privilege. Any unauthorised use, disclosure, or copying is not permitted. If you have received this email in error, please notify us and immediately and permanently delete it. Any views or opinions expressed in personal emails are solely those of the author and do not necessarily represent those of Royal Holloway, University of London. It is your responsibility to ensure that this email and any attachments are virus free.
[Fortran, Patch, Coarray, PR 37336] Fix crash in finalizer when derived type coarray is already freed.
Hi all,
attached patch fixes a crash in coarray programs when an allocatable derived
typed coarray was freed explicitly. The generated cleanup code did not take
into account, that the coarray may have been deallocated already. The patch
fixes this by moving the statements accessing components inside the derived type
into the block guard by its allocated check.
Regtested ok on f37/x86_64. Ok for master?
Regards,
Andre
--
Andre Vehreschild * Email: vehre ad gmx dot de
diff --git a/gcc/fortran/trans-array.cc b/gcc/fortran/trans-array.cc
index e0fc8ebc46b..8e94a9a469f 100644
--- a/gcc/fortran/trans-array.cc
+++ b/gcc/fortran/trans-array.cc
@@ -9320,6 +9320,12 @@ structure_alloc_comps (gfc_symbol * der_type, tree decl, tree dest,
gfc_add_expr_to_block (&fnblock, tmp);
}
+ /* Still having a descriptor array of rank == 0 here, indicates an
+ allocatable coarrays. Dereference it correctly. */
+ if (GFC_DESCRIPTOR_TYPE_P (decl_type))
+{
+ decl = build_fold_indirect_ref (gfc_conv_array_data (decl));
+}
/* Otherwise, act on the components or recursively call self to
act on a chain of components. */
for (c = der_type->components; c; c = c->next)
@@ -11507,7 +11513,11 @@ gfc_trans_deferred_array (gfc_symbol * sym, gfc_wrapped_block * block)
{
int rank;
rank = sym->as ? sym->as->rank : 0;
- tmp = gfc_deallocate_alloc_comp (sym->ts.u.derived, descriptor, rank);
+ tmp = gfc_deallocate_alloc_comp (sym->ts.u.derived, descriptor, rank,
+ (sym->attr.codimension
+ && flag_coarray == GFC_FCOARRAY_LIB)
+ ? GFC_STRUCTURE_CAF_MODE_IN_COARRAY
+ : 0);
gfc_add_expr_to_block (&cleanup, tmp);
}
@@ -11521,9 +11531,11 @@ gfc_trans_deferred_array (gfc_symbol * sym, gfc_wrapped_block * block)
NULL_TREE, NULL_TREE, true, e,
sym->attr.codimension
? GFC_CAF_COARRAY_DEREGISTER
- : GFC_CAF_COARRAY_NOCOARRAY);
+ : GFC_CAF_COARRAY_NOCOARRAY,
+ NULL_TREE, gfc_finish_block (&cleanup));
if (e)
gfc_free_expr (e);
+ gfc_init_block (&cleanup);
gfc_add_expr_to_block (&cleanup, tmp);
}
diff --git a/gcc/testsuite/gfortran.dg/coarray/alloc_comp_6.f90 b/gcc/testsuite/gfortran.dg/coarray/alloc_comp_6.f90
new file mode 100644
index 000..e8a74db2c18
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/coarray/alloc_comp_6.f90
@@ -0,0 +1,29 @@
+! { dg-do run }
+
+program alloc_comp_6
+
+ implicit none
+
+ type :: foo
+real :: x
+integer, allocatable :: y(:)
+ end type
+
+ call check()
+
+contains
+
+ subroutine check()
+block
+ type(foo), allocatable :: example[:] ! needs to be a coarray
+
+ allocate(example[*])
+ allocate(example%y(10))
+ example%x = 3.4
+ example%y = 4
+
+ deallocate(example)
+end block ! example%y shall not be accessed here by the finalizer,
+ ! because example is already deallocated
+ end subroutine check
+end program alloc_comp_6
Fortran: Free alloc. comp. in allocated coarrays only.
When freeing allocatable components of an allocatable coarray, add
a check that the coarray is still allocated, before accessing the
components.
This patch adds to PR fortran/37336, but does not fix it completely.
gcc/fortran/ChangeLog:
PR fortran/37336
* trans-array.cc (structure_alloc_comps): Deref coarray.
(gfc_trans_deferred_array): Add freeing of components after
check for allocated coarray.
gcc/testsuite/ChangeLog:
PR fortran/37336
* gfortran.dg/coarray/alloc_comp_6.f90: New test.
Re: Test with an lto-build of libgfortran.
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
Re: Test with an lto-build of libgfortran.
(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
Re: Test with an lto-build of libgfortran.
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
Re: Test with an lto-build of libgfortran.
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 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";
