Re: [PATCH] Fortran: rank checking with explicit-/assumed-size arrays and CLASS [PR58331]
Hi Tobias, Am 15.03.23 um 10:10 schrieb Tobias Burnus: Hi Harald, On 14.03.23 20:38, Harald Anlauf wrote: The testcase covers only non-coarray cases, as playing with coarray variants hit pre-exisiting issues in gfortran that are very likely unrelated to the interface checks. I concur (but would not rule out additional interface issues). More testing seems to mostly uncover issues later on in trans*.cc, e.g. when passing type to class. I'll open a PR on this as a followup. I consider this rather as post 13-release stuff. In any case, the coarray issue can be fixed separately. And I think post-GCC-13 makes sense. Good. Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks – LGTM! + formal_as = formal->ts.type == BT_CLASS ? CLASS_DATA (formal)->as + : formal->as; + (Jakub remarks for such code that some editor (emacs?), he does not use, mis--auto-indent such a code - and proposes to add a parentheses around the right-hand side of the assignment.) Ah, adding parentheses helps! I've reformatted this block accordingly. Pushed as: https://gcc.gnu.org/g:901edd99b44976b3c2b13a7d525d9e315540186a * * * I also wonder whether we need some run-time testcase. The interface check now works and I also tend to write dg-do-compile testcases, but given what can go wrong with all the array descriptor, class etc handling, we may want to ensure it works at run time. – Thoughts? If you comment out the lines with dg-error, the code compiles and seems to run fine here. I've even found cases where passing array sections works correctly here and with current Intel it does not ;-) I'd prefer to postpone more elaborate run-time tests until we have more non-ICEing related code. Thanks, Harald (That's independent of the patch it and could be done as follow up, if it deemed reasonable. The included testcase is surely compile-only as it has dg-error checks.) 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: [PATCH] Fortran: rank checking with explicit-/assumed-size arrays and CLASS [PR58331]
Hi Harald, On 14.03.23 20:38, Harald Anlauf wrote: The testcase covers only non-coarray cases, as playing with coarray variants hit pre-exisiting issues in gfortran that are very likely unrelated to the interface checks. I concur (but would not rule out additional interface issues). I consider this rather as post 13-release stuff. In any case, the coarray issue can be fixed separately. And I think post-GCC-13 makes sense. Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks – LGTM! + formal_as = formal->ts.type == BT_CLASS ? CLASS_DATA (formal)->as + : formal->as; + (Jakub remarks for such code that some editor (emacs?), he does not use, mis--auto-indent such a code - and proposes to add a parentheses around the right-hand side of the assignment.) * * * I also wonder whether we need some run-time testcase. The interface check now works and I also tend to write dg-do-compile testcases, but given what can go wrong with all the array descriptor, class etc handling, we may want to ensure it works at run time. – Thoughts? (That's independent of the patch it and could be done as follow up, if it deemed reasonable. The included testcase is surely compile-only as it has dg-error checks.) 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: [Patch, fortran] PR37336 finalization
On Wed, 15 Mar 2023, Paul Richard Thomas wrote: > Hi All, > > I am awaiting a green light to commit this patch or not. I'd say go ahead. Richard. > Cheers > > Paul > > > On Fri, 10 Mar 2023 at 16:49, Paul Richard Thomas < > paul.richard.tho...@gmail.com> wrote: > > > Hi Thomas, > > > > Before answering that, I thought that I had better try the polyhedron > > suite with -fwrapv and -std=legacy together. As far as I can see, all is > > well and so, yes, I think that is a good idea. > > > > Cheers > > > > Paul > > > > > > > > Date & Time : 9 Mar 2023 18:13:04 > > Test Name : gfor_13 > > Compile Command : gfc13 -ffast-math -funroll-loops -O3 -fwrapv -std=legacy > > %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 55904 7.17 2 0.0628 > > aermod 0.00 1280104 10.77 2 1.2769 > > air 0.00 136392 4.09 2 0.4276 > > capacita 0.00 102680 23.79 2 0.7587 > > channel2 0.00 43928108.59 2 0.5834 > >doduc 0.00 194224 13.98 2 0.2468 > > gas_dyn2 0.00 104080105.46 2 0.1659 > > fatigue2 0.00 90752 62.86 2 1.3092 > > induct2 0.00 224920 58.08 2 0.6594 > >linpk 0.00 51768 7.15 2 0.4892 > > mp_prop_desi 0.00 48432 94.28 2 0.0164 > > nf 0.00 64480 11.16 2 0.0134 > > protein 0.00 140592 24.22 2 10.0347 > > rnflow 0.00 197704 20.74 2 0.1904 > >test_fpu2 0.00 147232 53.54 2 0.1093 > >tfft2 0.00 43896 55.09 2 3.8688 > > > > Geometric Mean Execution Time = 26.19 seconds > > > > > > > > > > On Thu, 9 Mar 2023 at 17:30, Thomas Koenig 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 > >> > > > > > > -- > > "If you can't explain it simply, you don't understand it well enough" - > > Albert Einstein > > > > > -- Richard Biener SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg, Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman; HRB 36809 (AG Nuernberg)
Re: [Patch, fortran] PR37336 finalization
Hi All, I am awaiting a green light to commit this patch or not. Cheers Paul On Fri, 10 Mar 2023 at 16:49, Paul Richard Thomas < paul.richard.tho...@gmail.com> wrote: > Hi Thomas, > > Before answering that, I thought that I had better try the polyhedron > suite with -fwrapv and -std=legacy together. As far as I can see, all is > well and so, yes, I think that is a good idea. > > Cheers > > Paul > > > > Date & Time : 9 Mar 2023 18:13:04 > Test Name : gfor_13 > Compile Command : gfc13 -ffast-math -funroll-loops -O3 -fwrapv -std=legacy > %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 55904 7.17 2 0.0628 > aermod 0.00 1280104 10.77 2 1.2769 > air 0.00 136392 4.09 2 0.4276 > capacita 0.00 102680 23.79 2 0.7587 > channel2 0.00 43928108.59 2 0.5834 >doduc 0.00 194224 13.98 2 0.2468 > gas_dyn2 0.00 104080105.46 2 0.1659 > fatigue2 0.00 90752 62.86 2 1.3092 > induct2 0.00 224920 58.08 2 0.6594 >linpk 0.00 51768 7.15 2 0.4892 > mp_prop_desi 0.00 48432 94.28 2 0.0164 > nf 0.00 64480 11.16 2 0.0134 > protein 0.00 140592 24.22 2 10.0347 > rnflow 0.00 197704 20.74 2 0.1904 >test_fpu2 0.00 147232 53.54 2 0.1093 >tfft2 0.00 43896 55.09 2 3.8688 > > Geometric Mean Execution Time = 26.19 seconds > > > > > On Thu, 9 Mar 2023 at 17:30, Thomas Koenig 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 >> > > > -- > "If you can't explain it simply, you don't understand it well enough" - > Albert Einstein > -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
