Re: libgfortran.so SONAME and powerpc64le-linux ABI changes (2nd patch)
Hi Michael, It adds target hooks so the back end can overwrite the kind number for types. I made the IBM long double type use KIND=15 instead of KIND=16, and Float128 uses KIND=16 as we've discussed. The tests for long double are still failing, so I suspect we will need some way of signalling about the long double which uses a funky kind (king=15). There is still no conclusion how to deal with the two 16-bit types. I have also asked on the J3 mailing list, and received a few different opinions and options as well. We can: - Support KIND=16 == IEEE_QP only in the compiler, and supply a CONVERT option from IBM_QP to IEEE_QP only. People who would need the old format (why anybody would do that, I have no idea, but some may) would have to use the old compiler. - Support KIND=16 == IEEE_QP and KIND=15 = IBM_QP in the compiler, and implement SELECTED_REAL_KIND according to the Fortran standard. This would mean that people who put in a precision of 20 digits as a shorthand for REAL(KIND=16) will get the old version. This will lead to endless confusion, and penalize people who used SELECTED_REAL_KIND, so it should be avoided. - Support KIND=16 == IEEE_QP and KIND=15 = IBM_QP in the compiler, and have SELECTED_REAL_KIND return 16 for anything above double precision. This would actually violate the Fortran standard, and has been generally viewed by J3 as not a good idea. It would, however, work well for most users. Those who actually need IBM_QP would have to specify it specifically. - Have a compiler switch which selects between IEEE_QP and IBM_QP. This was a suggestion by Steve Lionel formerly of DEC and Intel, and they did that when they had a few floating point formats on the Alpha to choose from. We would then have to specially annotate the KIND=16 library routines, and also maybe indicate the different argument types in module files. Anything else would be user error. They also had the CONVERT options to go with it. Question: Which option would we want to pursue? I actually think the fourth one (the suggestion by Steve Lionel) is the best one. Other opinions? Best regards Thomas
Re: libgfortran.so SONAME and powerpc64le-linux ABI changes (2nd patch)
On Sat, Oct 30, 2021 at 11:30:29AM +0200, Thomas Koenig wrote:
> - Have a compiler switch which selects between IEEE_QP and IBM_QP.
> This was a suggestion by Steve Lionel formerly of DEC and Intel,
> and they did that when they had a few floating point formats on
> the Alpha to choose from. We would then have to specially annotate
> the KIND=16 library routines, and also maybe indicate the different
> argument types in module files. Anything else would be user error.
> They also had the CONVERT options to go with it.
>
> Question: Which option would we want to pursue? I actually think the
> fourth one (the suggestion by Steve Lionel) is the best one.
That was the last option I was mentioning in the initial mail
https://gcc.gnu.org/pipermail/gcc/2021-October/237478.html
Copying it here:
Or the last option would be to try to make libgfortran.so.5 ABI compatible
with both choices on powerpc64le-linux. From quick skimming of libgfortran,
we have lots of generated functions, which use HAVE_GFC_REAL_16 and
GFC_REAL_16 etc. macros. So, we could perhaps arrange for the compiler
to use r16i or r17 instead of r16 in the names when real(kind=16) is the
IEEE quad on powerpc64le and keep using r16 for the IBM double double.
For the *.F90 generated files, one could achieve it by making sure
the *r16* files are compiled with -mabi=ibmlongdouble, for
*r16i* or *r17* with -mabi=ieeelongdouble and otherwise use kind=16 in
those, for *.c generated files the *GFC_* macros could just ensure that
it doesn't use long double but __ibm128 or __float128 depending on which one
is needed.
But then I see e.g. the io routines to just pass in kind and so
switch (kind) // or len
{
case ...:
*(GFC_REAL_*) = ...;
}
etc. Could we just pretend in the compiler to libgfortran ABI that
powerpc64le-linux real(kind=16) is kind 17 and make sure that if anything
would actually think it is 17 bytes it uses 16 instead (though, kind=10
on x86-64 or i686 also isn't 10 bytes but 16 or 12, right?).
That solution would most closely match what we do e.g. for C/C++, especially
libstdc++ or glibc, we already have an option to select that -
-mabi=ibmlongdouble vs. -mabi=ieeelongdouble and default selected during
configure. libgfortran would be ABI compatible with both ABIs, but user
binaries or libraries wouldn't be. Similarly to C, there is no different
mangling of user symbols.
Jakub
Re: libgfortran.so SONAME and powerpc64le-linux ABI changes (2nd patch)
Hi Jakub,
On Sat, Oct 30, 2021 at 11:30:29AM +0200, Thomas Koenig wrote:
- Have a compiler switch which selects between IEEE_QP and IBM_QP.
This was a suggestion by Steve Lionel formerly of DEC and Intel,
and they did that when they had a few floating point formats on
the Alpha to choose from. We would then have to specially annotate
the KIND=16 library routines, and also maybe indicate the different
argument types in module files. Anything else would be user error.
They also had the CONVERT options to go with it.
Question: Which option would we want to pursue? I actually think the
fourth one (the suggestion by Steve Lionel) is the best one.
That was the last option I was mentioning in the initial mail
https://gcc.gnu.org/pipermail/gcc/2021-October/237478.html
Copying it here:
Or the last option would be to try to make libgfortran.so.5 ABI compatible
with both choices on powerpc64le-linux. From quick skimming of libgfortran,
we have lots of generated functions, which use HAVE_GFC_REAL_16 and
GFC_REAL_16 etc. macros. So, we could perhaps arrange for the compiler
to use r16i or r17 instead of r16 in the names when real(kind=16) is the
IEEE quad on powerpc64le and keep using r16 for the IBM double double.
That is quite doable.
For the *.F90 generated files, one could achieve it by making sure
the *r16* files are compiled with -mabi=ibmlongdouble, for
*r16i* or *r17* with -mabi=ieeelongdouble and otherwise use kind=16 in
those, for *.c generated files the *GFC_* macros could just ensure that
it doesn't use long double but __ibm128 or __float128 depending on which one
is needed.
Yep.
But then I see e.g. the io routines to just pass in kind and so
switch (kind) // or len
{
case ...:
*(GFC_REAL_*) = ...;
}
etc. Could we just pretend in the compiler to libgfortran ABI that
powerpc64le-linux real(kind=16) is kind 17 and make sure that if anything
would actually think it is 17 bytes it uses 16 instead (though, kind=10
on x86-64 or i686 also isn't 10 bytes but 16 or 12, right?).
That is not something I would like to have, for purposes of cleanness.
We can always switch at run-time to a different function. This
branch should be rather well-predicted, and additional execution
will be insignificant compared to the rest of the I/O.
That solution would most closely match what we do e.g. for C/C++, especially
libstdc++ or glibc, we already have an option to select that -
-mabi=ibmlongdouble vs. -mabi=ieeelongdouble and default selected during
configure. libgfortran would be ABI compatible with both ABIs, but user
binaries or libraries wouldn't be. Similarly to C, there is no different
mangling of user symbols.
Hm, there's a difference. I meant that we can select this at compile
time for the user, and we should at least consider adding a flag to
the module files if the routine has been compiled for IBM_QP or IEEE_QP.
We should not change the mangling of the routine names themselves,
I agree.
Best regards
Thomas
Re: [PATCH] PR fortran/99853 - ICE: Cannot convert 'LOGICAL(4)' to 'INTEGER(8)' (etc.)
Committed as simple and obvious after discussion in PR. Harald Am 28.10.21 um 23:03 schrieb Harald Anlauf via Fortran: Dear Fortranners, the original fix by Steve was lingering in the PR. We did ICE in situations where in a SELECT CASE a kind conversion was deemed necessary, but it did involve different types. The check gfc_convert_type_warn () was invoked with arguments requesting to generate an internal error. A regular gfc_error is good enough here. Regtested on x86_64-pc-linux-gnu. OK? Thanks, also to Steve, Harald Fortran: generate regular error on invalid conversions of CASE expressions gcc/fortran/ChangeLog: PR fortran/99853 * resolve.c (resolve_select): Generate regular gfc_error on invalid conversions instead of an gfc_internal_error. gcc/testsuite/ChangeLog: PR fortran/99853 * gfortran.dg/pr99853.f90: New test.
Re: [PATCH] Fortran: recognize Gerhard Steinmetz
Committed as simple and obvious as r12-4803. Harald Am 30.10.21 um 01:18 schrieb Manfred Schwarb via Fortran: Am 29.10.21 um 21:58 schrieb Harald Anlauf via Fortran: Hi Manfred, Am 29.10.21 um 16:33 schrieb Manfred Schwarb via Fortran: Hi, there were really a lot of test cases provided by Gerhard Steinmetz lately. Although I'm not really in the position to suggest this, I would appreciate it, if one could recognize him by adding an entry to gfortran.texi. As e.g. in the proposed patch. Such a patch should probably be signed-off my someone of the inner circle and not by me ;-) well, this is sth. close to obvious to everybody. ;-) Anyway, a ChangeLog entry would be nice. I would feel more comfortable if such a patch would origin from somebody else, not from me, actually. Harald Cheers, Manfred
Re: [PATCH,FORTRAN] Fix memory leak of gsymbol
On Fri, 29 Oct 2021 01:23:02 +0200 Bernhard Reutner-Fischer wrote: > On Thu, 28 Oct 2021 23:37:59 +0200 > Harald Anlauf wrote: > > > Hi Bernhard, > > > > Am 27.10.21 um 23:43 schrieb Bernhard Reutner-Fischer via Gcc-patches: > > > ping > > > [I'll rebase and retest this too since it's been a while. > > > Ok if it passes?] > > > > > > On Sun, 21 Oct 2018 16:04:34 +0200 > > > Bernhard Reutner-Fischer wrote: > > > >> gcc/fortran/ChangeLog: > > >> > > >> 2018-10-21 Bernhard Reutner-Fischer > > >> > > >> * parse.c (clean_up_modules): Free gsym. > > > this essentially looks fine, but did you inspect the callers? > > > > With the change to the interface (*gsym -> *&gsym), it could have > > effects not visible here due to the explicit gsym = NULL. > > > > Assuming you checked that, and if it regtests fine, then it is > > OK for mainline. > > The only caller is translate_all_program_units. > Since we free only module gsyms, even -fdump-fortran-global is > unaffected by this, fwiw. > > It regtests cleanly and i will push it when the rest is approved. > Thanks! Pushed as r12-4804 thanks,
Re: [PATCH] Fortran: adjust column sizes in intrinsic.texi
Committed as r12-4805. Thanks for the patch! Harald Am 30.10.21 um 01:14 schrieb Manfred Schwarb via Fortran: Am 29.10.21 um 21:44 schrieb Harald Anlauf via Fortran: Hi Manfred, Am 29.10.21 um 16:05 schrieb Manfred Schwarb via Fortran: Hi, in intrinsic.texi, a lot of tables wrap lines when watching the resulting info file in a 80char terminal. Adjust the @columnfractions items to fit screen. Some minor white space changes are added as well to help saving space. the patch looks fine. However, could you please provide a properly formatted patch that applies using git patch and a ChangeLog entry? Sorry, forgot the changelog entry, I added it to the patch now. Thanks, Harald Signed-off-by Manfred Schwarb [Note: I do not have commit access]
Re: [PATCH] Fortran: Correct documentation for REAL intrinsic
Committed as r12-4806. Thanks for the patch! Harald Am 30.10.21 um 01:17 schrieb Manfred Schwarb via Fortran: Am 29.10.21 um 21:56 schrieb Harald Anlauf via Fortran: Hi Manfred, Am 29.10.21 um 16:18 schrieb Manfred Schwarb via Gcc-patches: Hi, documentation for REAL intrinsic is slightly wrong. Fix it. Patch is done on top of the column adjustment patch. the patch looks fine, but it would help a lot to have a ChangeLog entry. Sorry, forgot the changelog entry, I added it to the patch now. Thanks, Harald Signed-off-by Manfred Schwarb [Note: I do not have commit access]
Re: [PATCH] Fortran: adjust error message for SHORT and LONG intrinsics
Committed as r12-4807. Thanks for the patch! Harald Am 30.10.21 um 01:15 schrieb Manfred Schwarb via Gcc-patches: Am 29.10.21 um 21:51 schrieb Harald Anlauf via Fortran: Hi Manfred, Am 29.10.21 um 16:12 schrieb Manfred Schwarb via Fortran: Hi, on 2019-07-23, support for SHORT and LONG intrinsics were removed be Steve Kargl by adding an error message in check.c. However, the error message Error: 'long' intrinsic subprogram at (1) has been deprecated is misleading, as support has been disabled by this patch. Adjust the error message. This error message does not appear in the testsuite AFAIK. the patch looks fine. A testcase checking the error message is missing, as well as a ChangeLog entry. Sorry, forgot the changelog entry, I added it to the patch now. Testcase was missing already before, but I added a trivial test to the patch for completeness. Thanks, Harald Signed-off-by Manfred Schwarb [Note: I do not have commit access]
Re: [PATCH] Fortran: Remove documentation for SHORT and LONG intrinics
Committed as r12-4808 after checking "make dvi". Thanks for the patch! Harald Am 30.10.21 um 01:16 schrieb Manfred Schwarb via Gcc-patches: Am 29.10.21 um 21:52 schrieb Harald Anlauf via Fortran: Hi Manfred, Am 29.10.21 um 16:13 schrieb Manfred Schwarb via Gcc-patches: Hi, on 2019-07-23, support for SHORT and LONG intrinsics was removed be Steve Kargl by adding an error message in check.c. As far as I can see code support is still there, though. Remove documentation for these intrinsics. could you please provide a formatted patch that applies using git apply? And a ChangeLog entry? Sorry, forgot the changelog entry, I added it to the patch now. Thanks, Harald Signed-off-by Manfred Schwarb [Note: I do not have commit access]
Re: [PATCH,FORTRAN 01/29] gdbinit: break on gfc_internal_error
On 30 October 2021 00:13:06 CEST, Jerry D wrote: >Looks OK. Thanks! I guess I need an OK from some global maintainer, too? The breakpoint is ignored by automatically answering the question with n if the symbol is not found when loading .gdbinit for e.g. cc1. thanks, > >Cheers > >On 10/29/21 11:58 AM, Bernhard Reutner-Fischer via Fortran wrote: >> ping >> >> On Wed, 5 Sep 2018 14:57:04 + >> Bernhard Reutner-Fischer wrote: >> >>> From: Bernhard Reutner-Fischer >>> >>> Aids debugging the fortran FE. >>> >>> gcc/ChangeLog: >>> >>> 2017-11-12 Bernhard Reutner-Fischer >>> >>> * gdbinit.in: Break on gfc_internal_error. >>> --- >>> gcc/gdbinit.in | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git a/gcc/gdbinit.in b/gcc/gdbinit.in >>> index 4db977f0bab..ac4d7c42e21 100644 >>> --- a/gcc/gdbinit.in >>> +++ b/gcc/gdbinit.in >>> @@ -227,6 +227,7 @@ b fancy_abort >>> >>> # Put a breakpoint on internal_error to help with debugging ICEs. >>> b internal_error >>> +b gfc_internal_error >>> >>> set complaints 0 >>> # Don't let abort actually run, as it will make >
Re: [PATCH,FORTRAN] Fix memory leak of gsymbol
> > The only caller is translate_all_program_units.
> > Since we free only module gsyms, even -fdump-fortran-global is
> > unaffected by this, fwiw.
AFAICS we do not have a test for -fdump-fortran-global
Do we want to add one, would the attached be OK?
thanks,
>From 7e7856cf9ec88ab7fb48e7c73f9cc6495a4a9c22 Mon Sep 17 00:00:00 2001
From: Bernhard Reutner-Fischer
Date: Sat, 30 Oct 2021 23:43:12 +0200
Subject: [PATCH] Fortran: add testcase
gcc/testsuite/ChangeLog:
2021-10-30 Bernhard Reutner-Fischer
* gfortran.dg/dump-fortran-global-1.f90: New test.
---
.../gfortran.dg/dump-fortran-global-1.f90 | 25 +++
1 file changed, 25 insertions(+)
create mode 100644 gcc/testsuite/gfortran.dg/dump-fortran-global-1.f90
diff --git a/gcc/testsuite/gfortran.dg/dump-fortran-global-1.f90 b/gcc/testsuite/gfortran.dg/dump-fortran-global-1.f90
new file mode 100644
index 000..a8a1b15cce3
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/dump-fortran-global-1.f90
@@ -0,0 +1,25 @@
+! { dg-do compile }
+! { dg-options "-fdump-fortran-global" }
+!
+! Test that -fdump-fortran-global works
+
+integer function myifun ()
+ myifun = 42
+end function myifun
+
+program testprogram
+ implicit none
+ interface
+subroutine sub1()
+end subroutine sub1
+ end interface
+ integer :: myifun
+ integer :: myint1 = 42
+ real :: myreal1 = .0815
+ if (myreal1 > 1.0) stop 1
+ if (myint1 < 1) stop 2
+ if (myint1 /= myifun()) stop 3
+end program testprogram
+! { dg-output "\[\n\r]*name=myifun, sym_name=myifun\[\n\r]" }
+! { dg-output "\[\n\r]*name=testprogram\[\n\r]" }
+! { dg-prune-output "\[\n\r]*" }
--
2.33.0
Re: [PATCH] Fortran: adjust error message for SHORT and LONG intrinsics
On Sat, 30 Oct 2021 20:11:13 +0200
Harald Anlauf via Fortran wrote:
> Committed as r12-4807.
>
> Thanks for the patch!
>
> Harald
>
> Am 30.10.21 um 01:15 schrieb Manfred Schwarb via Gcc-patches:
> > Am 29.10.21 um 21:51 schrieb Harald Anlauf via Fortran:
> >> Hi Manfred,
> >>
> >> Am 29.10.21 um 16:12 schrieb Manfred Schwarb via Fortran:
> >>> Hi,
> >>>
> >>> on 2019-07-23, support for SHORT and LONG intrinsics were removed be
> >>> Steve Kargl by
> >>> adding an error message in check.c. However, the error message
> >>> Error: 'long' intrinsic subprogram at (1) has been deprecated
> >>> is misleading, as support has been disabled by this patch.
> >>>
> >>> Adjust the error message. This error message does not appear in the
> >>> testsuite AFAIK.
I just saw a related spot where we may want to give a hint on the
modern replacement iff folks think that's a good thing to have.
match.c::gfc_match_intrinsic_op
---8<---
case 'x':
if (gfc_next_ascii_char () == 'o'
&& gfc_next_ascii_char () == 'r'
&& gfc_next_ascii_char () == '.')
{
if (!gfc_notify_std (GFC_STD_LEGACY, ".XOR. operator at %C"))
return MATCH_ERROR;
/* Matched ".xor." - equivalent to ".neqv.". */
*result = INTRINSIC_NEQV;
return MATCH_YES;
}
break;
---8<---
Re: [Patch, committed] Fortran: Fix 'fn spec' for deferred character length (was: Re: [r12-4457 Regression] FAIL: gfortran.dg/deferred_type_param_6.f90 -Os execution test on Linux/x86_64)
On Tue, 19 Oct 2021 16:49:02 +0200 Tobias Burnus wrote: > On 16.10.21 20:54, Jan Hubicka wrote: > > I wrote: > >> Fortran has for a long time 'character(len=5), allocatable" or > >> "character(len=*)". In the first case, the "5" can be ignored as both > >> caller and callee know the length. In the second case, the length is > >> determined by the argument, but it cannot be changed. > >> > >> Since a not-that-short while, 'len=:' together with allocatable/pointer > >> is supported. > >> > >> In the latter case, the value can be change when the array > >> association/allocation is changed. > >> ... > >> + if (!sym->ts.u.cl->length > >> + && ((sym->attr.allocatable && sym->attr.target) > >> + || sym->attr.pointer)) > >> +spec[spec_len++] = '.'; > >> + if (!sym->ts.u.cl->length && sym->attr.allocatable) > >> +spec[spec_len++] = 'w'; > >> + else > >> +spec[spec_len++] = 'R'; > > Also escaping is quite important bit of information so it would be > > good to figure out if it really can escape rather than playing safe. > > The pointer to the string length variable itself does not escape, > only its integer string value: > > subroutine foo(x) >character(len=:), pointer :: x >character(len=:), pointer :: y >y => x > has in the dump: >.y = *_x; >y = (character(kind=1)[1:.y] *) *x; > > Thus, 'w' can always be used. > > Committed as obvious as r12-4511-gff0eec94e87dfb7dc387f120ca5ade2707aecf50 I guess we don't have intrinsics with allocatable or pointer character types? Just asking because this fix went to trans-types.c::create_fn_spec but we also construct fn spec strings in trans-intrinsic.c::intrinsic_fnspec In the former, the if (spec_len < sizeof (spec)) looks like it's suboptimal, I'd rather not have this went unnoticed? Maybe it would be worthwhile to have just one fnspec generator which is used in both spots? thanks,
