Re: [PATCH] Fortran: fix ICE with bind(c) in block data [PR104332]
On 3/9/23 10:08 AM, Harald Anlauf via Fortran wrote: Dear all, the attached almost obvious patch fixes a NULL pointer dereference in a check of a symbol with the bind(c) attribute. Regtested on x86_64-pc-linux-gnu. OK for mainline? This PR is marked as 10/11/12/13 regression, thus it should qualify for a backport. It's simple enough anyway. Thanks, Harald OK, please proceed. Thanks for the patch. Jerry
Re: [Patch, fortran] PR37336 finalization
While recovering from an illness here folks I have been following all of these discussions. I think I will put in my two cents worth. From what i can see, Paul's patch breaks nothing and fixes many things. The only thing holding us back is fear we might break something. The likelihood of actually breaking something is very low. The consequences of breaking anything, worst case, is to revert a patch. (ie no consequence) It is clear to me the value added clearly exceeds the the risks by a lot. Therefore, as Spock would say, it is illogical to not make a decision and move forward with this finalization patch, and commit it. More will be advanced from making this decision from not making this decision. Regards, Jerry On 3/9/23 9:30 AM, Thomas Koenig via Fortran wrote: Hi Paul, -fdefault-integer-8 does indeed fix the problem with rnflow.f90 but breaks tfft2.f90, with a type mismatch at lines 36 and 44. integer(8), parameter :: jmul = 843314861 ! multiplicateur integer(8), parameter :: jadd = 453816693 ! constante additive Does the job and is portable. I think -frwapv (as suggested by Jakub) would be the better choice. The problem is the linear congruential pseudo-random number generators which were much used in earlier times (and are still present in legacy code), which violate the Fortran standards by assuming silent truncation. If a new optimization breaks this (widespread, but illegal) idiom, maybe the best way to deal with it is to add -frwapv to -std=legacy. What do you think? Best regards Thomas
[PATCH] Fortran: fix ICE with bind(c) in block data [PR104332]
Dear all,
the attached almost obvious patch fixes a NULL pointer dereference
in a check of a symbol with the bind(c) attribute.
Regtested on x86_64-pc-linux-gnu. OK for mainline?
This PR is marked as 10/11/12/13 regression, thus it should
qualify for a backport. It's simple enough anyway.
Thanks,
Harald
From ef96d7d360c088d68e3b405401bdb8b589d562f2 Mon Sep 17 00:00:00 2001
From: Harald Anlauf
Date: Thu, 9 Mar 2023 18:59:08 +0100
Subject: [PATCH] Fortran: fix ICE with bind(c) in block data [PR104332]
gcc/fortran/ChangeLog:
PR fortran/104332
* resolve.cc (resolve_symbol): Avoid NULL pointer dereference while
checking a symbol with the BIND(C) attribute.
gcc/testsuite/ChangeLog:
PR fortran/104332
* gfortran.dg/bind_c_usage_34.f90: New test.
---
gcc/fortran/resolve.cc| 4 ++--
gcc/testsuite/gfortran.dg/bind_c_usage_34.f90 | 21 +++
2 files changed, 23 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/gfortran.dg/bind_c_usage_34.f90
diff --git a/gcc/fortran/resolve.cc b/gcc/fortran/resolve.cc
index 2780c82c798..46585879ddc 100644
--- a/gcc/fortran/resolve.cc
+++ b/gcc/fortran/resolve.cc
@@ -15933,8 +15933,8 @@ resolve_symbol (gfc_symbol *sym)
/* First, make sure the variable is declared at the
module-level scope (J3/04-007, Section 15.3). */
- if (sym->ns->proc_name->attr.flavor != FL_MODULE &&
- sym->attr.in_common == 0)
+ if (!(sym->ns->proc_name && sym->ns->proc_name->attr.flavor == FL_MODULE)
+ && !sym->attr.in_common)
{
gfc_error ("Variable %qs at %L cannot be BIND(C) because it "
"is neither a COMMON block nor declared at the "
diff --git a/gcc/testsuite/gfortran.dg/bind_c_usage_34.f90 b/gcc/testsuite/gfortran.dg/bind_c_usage_34.f90
new file mode 100644
index 000..40c8e9363cf
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/bind_c_usage_34.f90
@@ -0,0 +1,21 @@
+! { dg-do compile }
+! PR fortran/104332 - ICE with bind(c) in block data
+! Contributed by G. Steinmetz
+
+block data
+ bind(c) :: a ! { dg-error "cannot be BIND\\(C\\)" }
+end
+
+block data aa
+ real, bind(c) :: a ! { dg-error "cannot be BIND\\(C\\)" }
+end
+
+block data bb
+ real:: a ! { dg-error "cannot be BIND\\(C\\)" }
+ bind(c) :: a
+end
+
+block data cc
+ common /a/ x
+ bind(c) :: /a/
+end
--
2.35.3
Re: [Patch, fortran] PR37336 finalization
Hi Paul, -fdefault-integer-8 does indeed fix the problem with rnflow.f90 but breaks tfft2.f90, with a type mismatch at lines 36 and 44. integer(8), parameter :: jmul = 843314861 ! multiplicateur integer(8), parameter :: jadd = 453816693 ! constante additive Does the job and is portable. I think -frwapv (as suggested by Jakub) would be the better choice. The problem is the linear congruential pseudo-random number generators which were much used in earlier times (and are still present in legacy code), which violate the Fortran standards by assuming silent truncation. If a new optimization breaks this (widespread, but illegal) idiom, maybe the best way to deal with it is to add -frwapv to -std=legacy. What do you think? Best regards Thomas
Re: [Patch, fortran] PR37336 finalization
On Thu, Mar 09, 2023 at 08:18:08AM +, Richard Biener wrote: > > the existing comment already explains the issue. I suppose > -fdefault-integer-8 would also work around the issue? > Please, no. -fdefault-* options should have been removed from gfortran years ago. Without a careful review, one might be stepping into a minefield, because the -fdefault-* option break storage association rules. If you want to use an option, then you likely want -finteger-4-integer-8. This option preserves storage association. -- Steve
Re: [Patch, fortran] PR37336 finalization
Hi Richard, Good spot! -fdefault-integer-8 does indeed fix the problem with rnflow.f90 but breaks tfft2.f90, with a type mismatch at lines 36 and 44. integer(8), parameter :: jmul = 843314861 ! multiplicateur integer(8), parameter :: jadd = 453816693 ! constante additive Does the job and is portable. With pr37336 patch applied: Date & Time : 9 Mar 2023 11:33:31 Test Name : gfor_13 Compile Command : gfc13 -ffast-math -funroll-loops -O2 %n.f90 -o %n Benchmarks : ac aermod air capacita channel2 doduc gas_dyn2 fatigue2 induct2 linpk mp_prop_design nf protein rnflow test_fpu2 tfft2 Maximum Times :1.0 Target Error % : 0.100 Minimum Repeats : 1 Maximum Repeats : 2 Benchmark Compile Executable Ave Run Number Estim Name(secs) (bytes)(secs) Repeats Err % - --- -- --- --- -- ac 0.00 51720 7.78 2 0. aermod 0.00 1079416 10.35 2 0.1642 air 0.00 87096 3.66 2 0.1230 capacita 0.00 65936 22.88 2 0.0153 channel2 0.00 39832 92.17 2 0.4410 doduc 0.00 182104 14.55 2 0.2509 gas_dyn2 0.00 91784182.65 2 0.8576 fatigue2 0.00 86496 95.72 2 0.7182 induct2 0.00 183824100.85 2 0.0084 linpk 0.00 43576 6.19 2 0.4442 mp_prop_desi 0.00 48376 94.72 2 0.4276 nf 0.00 52192 9.84 2 0.1321 protein 0.00 128248 22.06 2 0.0476 rnflow 0.00 136296 25.99 2 0.0462 test_fpu2 0.00 106232 82.97 2 0.0042 tfft2 0.00 35608 47.94 2 0.7071 Geometric Mean Execution Time = 28.68 seconds I think that the PR can be closed. Cheers Paul On Thu, 9 Mar 2023 at 08:18, Richard Biener wrote: > On Wed, 8 Mar 2023, Thomas Koenig wrote: > > > On 08.03.23 15:55, Paul Richard Thomas via Fortran wrote: > > > As noted below, rnflow.f90 hangs with the unpatched mainline at -O3 but > > > runs successfully at -O2. > > > > I can confirm that. > > > > > I presume that this is a serious regression since it involves > optimization? > > > Which component should I post it against? > > > > Probably against tree-optimization. If later analysis determines that > > it is something else, people will reassign it. > > > > This one probably calls for bisection. > > I have the following local change to rnflow with that I can't reproduce: > > --- rnflow.f90.orig 2016-06-01 14:50:16.922376347 +0200 > +++ rnflow.f90 2016-06-01 14:56:54.739045162 +0200 > @@ -884,6 +884,7 @@ > ! generation maison d'une valeur pseudo-aleatoire uniforme sur (0,1) > ! >integer, intent (inout) :: jsee ! germe courant > + integer(kind=8) :: jsee_long >integer, parameter :: jmul = 843314861 ! multiplicateur >integer, parameter :: jadd = 453816693 ! constante additive >integer, parameter :: j230 = 1073741824 ! 2 puissance 30 > @@ -899,7 +900,9 @@ > !CRAY - The following multiply must be done with 64 bits (not 46 bits) > ! The algoritm depends on the overflow characteristics of > ! a 32 or 64 bit multiply. > - jsee = jsee * jmul + jadd > + jsee_long = jsee; > + jsee_long = jsee_long * jmul + jadd > + jsee = jsee_long; > !CRAY - Change to avoid 32 bit integer dependency >! >! The original line is needlessly dependent on the > > the existing comment already explains the issue. I suppose > -fdefault-integer-8 would also work around the issue? > > ISTR there is an old (closed invalid) bugreport about this as well. > > Richard. > -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
Re: [Patch, fortran] PR37336 finalization
On Thu, 9 Mar 2023, Thomas Koenig wrote: > On 08.03.23 22:35, I wrote: > > On 08.03.23 15:55, Paul Richard Thomas via Fortran wrote: > >> As noted below, rnflow.f90 hangs with the unpatched mainline at -O3 but > >> runs successfully at -O2. > > > > I can confirm that. > > > >> I presume that this is a serious regression since it involves optimization? > >> Which component should I post it against? > > > > Probably against tree-optimization. If later analysis determines that > > it is something else, people will reassign it. > > > > This one probably calls for bisection. > > I have submitted this as > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109075 . > > Paul, thanks for catching this! > > @Richard: Is there a possibility of doing regular Polyhedron runs > at Suse in addition to the SPEC runs? This could also be interesting. We do run Polyhedron on a variety of machines already, you can visit https://lnt.opensuse.org/db_default/v4/CPP/latest_runs_report which is the benchmark set that includes polyhedron. Richard.
Re: [Patch, fortran] PR37336 finalization
On Wed, 8 Mar 2023, Thomas Koenig wrote: > On 08.03.23 15:55, Paul Richard Thomas via Fortran wrote: > > As noted below, rnflow.f90 hangs with the unpatched mainline at -O3 but > > runs successfully at -O2. > > I can confirm that. > > > I presume that this is a serious regression since it involves optimization? > > Which component should I post it against? > > Probably against tree-optimization. If later analysis determines that > it is something else, people will reassign it. > > This one probably calls for bisection. I have the following local change to rnflow with that I can't reproduce: --- rnflow.f90.orig 2016-06-01 14:50:16.922376347 +0200 +++ rnflow.f90 2016-06-01 14:56:54.739045162 +0200 @@ -884,6 +884,7 @@ ! generation maison d'une valeur pseudo-aleatoire uniforme sur (0,1) ! integer, intent (inout) :: jsee ! germe courant + integer(kind=8) :: jsee_long integer, parameter :: jmul = 843314861 ! multiplicateur integer, parameter :: jadd = 453816693 ! constante additive integer, parameter :: j230 = 1073741824 ! 2 puissance 30 @@ -899,7 +900,9 @@ !CRAY - The following multiply must be done with 64 bits (not 46 bits) ! The algoritm depends on the overflow characteristics of ! a 32 or 64 bit multiply. - jsee = jsee * jmul + jadd + jsee_long = jsee; + jsee_long = jsee_long * jmul + jadd + jsee = jsee_long; !CRAY - Change to avoid 32 bit integer dependency ! ! The original line is needlessly dependent on the the existing comment already explains the issue. I suppose -fdefault-integer-8 would also work around the issue? ISTR there is an old (closed invalid) bugreport about this as well. Richard.
Re: F77 indexed file support
Right, 3270 was the terminal. Wonderful beasts :). Anyway, this reminded me of an experiment I did a couple of years ago with wrapping the BerkeleyDB library in Fortran. I never had much use for it, but it works for small enough value of "work". But this is diverting a lot from the purpose of this mailing list. Regards, Arjen Op wo 8 mrt 2023 om 16:21 schreef Roland Hughes : > That would have been a 360/370 IBM Mini. The 3270 was the "smart" terminal. > > > https://imgs.search.brave.com/9CW5yhzliePl3PmZJJad0-GoiArzOyOIKkKfa0cntW8/rs:fit:640:540:1/g:ce/aHR0cHM6Ly9pLnBp/bmltZy5jb20vb3Jp/Z2luYWxzLzRlL2Nk/L2JlLzRlY2RiZTBl/YjQ0YmFlNGUzOTQ4/YjVlNDk2MWY1OWMx/LmpwZw > > Yes, I use database libraries all the time with C/C++. Given Gnu COBOL had > utilized the Berkley DB so they could provide full (or at least nearly > complete) language syntax I had hoped Gnu Fortran did the same. > > C/C++ never provided any indexed file or "record" level support. FORTRAN > always did, so I had hopes. > > Thanks, > > Roland > On 3/8/23 08:30, Arjen Markus wrote: > > Well, that is indeed something completely different.My main frame of > reference (pun not intentional) of that era was our IBM mini, I am not > quite sure of the type number, 3270? It had a very specific record > structure for unformatted files. Normally that was almost completely > hidden, except in the job control, but when we started exchanging data > files with the personal computers that were then coming out, I could write > programs that did the necessary conversions. Jolly good fun. My department > did not use VAXes, other departments did. > > So, in your case these files contain data identifiable via some index. Hm, > today you would do that via some library instead of via some builtin > language feature, at least when using Fortran, C, C++, ... > > Regards, > > Arjen > > Op wo 8 mrt 2023 om 14:31 schreef Roland Hughes via Fortran < > fortran@gcc.gnu.org>: > >> Thank you! >> >> >> On 3/8/2023 1:57 AM, Bernhard Reutner-Fischer wrote: >> > On 7 March 2023 23:18:58 CET, Roland Hughes via Fortran < >> fortran@gcc.gnu.org> wrote: >> > >> > [ snip namelist IO ] >> > >> >> Btw, is there a "search" utility for the archives or do I have to pull >> down all of the zip files, unzip into directory, and grep to look for stuff >> like this? I'm guessing it has come up before. >> > Indeed we have >> > https://inbox.sourceware.org/fortran/ >> > >> > along the traditional pipermail ml interface. >> > >> > thanks, >> >> -- >> Roland Hughes, President >> Logikal Solutions >> (630)-205-1593 (cell) >> http://www.theminimumyouneedtoknow.com >> http://www.infiniteexposure.net >> http://www.johnsmith-book.com >> >> -- > Roland Hughes, President > Logikal Solutions > (630)-205-1593 > http://www.theminimumyouneedtoknow.comhttp://www.infiniteexposure.nethttp://www.johnsmith-book.comhttp://www.logikalblog.comhttp://www.interestingauthors.com/blog > >
