Re: [patch] OpenMP: Add -Wopenmp and use it
Hi! On Fri, 24 Nov 2023 at 15:08, Jakub Jelinek wrote: > > On Fri, Nov 24, 2023 at 02:51:28PM +0100, Tobias Burnus wrote: > > Following the general trend to add a "[-W...]" to the warning messages > > for both better grouping of the warnings and - more importantly - for > > providing > > a means to silence such a warning (or to -Werror= them explicitly), this > > patch > > replaces several '0' by OPT_Wopenmp. > > > > Comments or remarks before I commit it? > > LGTM, thanks for working on it. > > Jakub > I think the lack of final '.' in: gcc/c-family/c.opt + Warn about suspicious OpenMP code has caused the following regressions: Running gcc:gcc.misc-tests/help.exp ... FAIL: compiler driver --help=c option(s): "^ +-.*[^:.]$" absent from output: " -WopenmpWarn about suspicious OpenMP code" FAIL: compiler driver --help=c++ option(s): "^ +-.*[^:.]$" absent from output: " -WopenmpWarn about suspicious OpenMP code" FAIL: compiler driver --help=fortran option(s): "^ +-.*[^:.]$" absent from output: " -WopenmpWarn about suspicious OpenMP code" FAIL: compiler driver --help=warnings option(s): "^ +-.*[^:.]$" absent from output: " -WopenmpWarn about suspicious OpenMP code" I think you have received a notification from our CI about that? Can you check it's as simple as that? Thanks, Christophe
Re: [patch] OpenMP: Add -Wopenmp and use it
On Mon, 27 Nov 2023 at 11:33, Tobias Burnus wrote: > > Hi, > > On 27.11.23 11:20, Christophe Lyon wrote: > > > I think the lack of final '.' in: > > Indeed - but you are lagging a bit behind: > > https://gcc.gnu.org/pipermail/gcc-patches/2023-November/638128.html > > [committed] c-family/c.opt (-Wopenmp): Add missing tailing '.' > > Fri Nov 24 18:56:21 GMT 2023 > > Committed as r14-5835-g6eb1507107dee3 > Great thanks! Sorry for the noise, it's a bit hard and error-prone to track which regressions have already fixed and/or are being worked on. Our bisect started at r14-5830, just a bit too early :-) Thanks, Christophe > 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: Updated Sourceware infrastructure plans
Hi, On Thu, 18 Apr 2024 at 10:15, FX Coudert wrote: > > > I regenerate auto* files from time to time for libgfortran. Regenerating > > them has always been very fragile (using --enable-maintainer-mode), > > and difficult to get right. > > I have never found them difficult to regenerate, but if you have only a non > maintainer build, it is a pain to have to make a new maintainer build for a > minor change. > FWIW, we have noticed lots of warnings from autoreconf in libgfortran. I didn't try to investigate, since the regenerated files are identical to what is currently in the repo. For instance, you can download the "stdio" output from the autoregen.py step in https://builder.sourceware.org/buildbot/#/builders/269/builds/4373 Thanks, Christophe > Moreover, our m4 code is particularly painful to use and unreadable. I have > been wondering for some time: should we switch to simpler Python scripts? It > would also mean that we would have fewer files in the generated/ folder: > right now, every time we add new combinations of types, we have a > combinatorial explosion of files. > > $ ls generated/sum_* > generated/sum_c10.c generated/sum_c17.c generated/sum_c8.c > generated/sum_i16.c generated/sum_i4.c generated/sum_r10.c > generated/sum_r17.c generated/sum_r8.c > generated/sum_c16.c generated/sum_c4.c generated/sum_i1.c > generated/sum_i2.c generated/sum_i8.c generated/sum_r16.c generated/sum_r4.c > > We could imagine having a single file for all sum intrinsics. > > How do Fortran maintainers feel about that? > > FX
Re: [PATCH] libgfortran: Fix up the autoreconf warnings.
On Thu, 2 May 2024 at 23:13, FX Coudert wrote: > > > libgfortran/ChangeLog: > > * Makefile.am: Use sub-dirs, amend recipies accordingly. > > * Makefile.in: Regenerate. > > Thanks Iain, I’ve tested it both with and without maintainer mode, and > regenerated files with no issue. I can also confirm that the many autoreconf > warnings that plagued libgfortran are now gone. > > Push as affd24bfc62203db9f9937c0d6cf8f1f75b80d72 > Nice, I can see on Sourceware's buildbot that there is now zero warning in libgfortran. Thanks, Christophe > FX
Re: [Patch, fortran] PR119460 - gfortran.dg/reduce_1.f90 FAILs
On Sun, 6 Apr 2025 at 14:39, Paul Richard Thomas wrote: > > Hi All, > > As far as I can tell, the attached patch fixes the problems with the reduce > intrinsic. I would be grateful to the reporters if they would confirm that > this is the case. > > The key to the fix appears in reduce_3.f90, which failed even with -m64. > Although it was not apparent from the tree dump, the scalar result was going > on the stack. Once it became larger than the word size, it pushed the > arguments out of alignment with the library prototype. > > I took the opportunity to add character length checking to the library. I > think that it might be redundant and so might not appear in the submitted > version. Thus far, I have failed to trigger the errors because the frontend > seems to catch them all. reduce_c and reduce_scalar_c will look a lot neater > without them. > > Harald has been enormously helpful in hunting out remaining problems and > providing fixes. These are woven into the patch. > > Regtests on FC41/x86_64 - OK for mainline after confirmations from the > reporters? > Hi! I've just verified that all gfortran.dg/reduce_*.f90 now pass on arm-unknown-linux-gnueabihf. Thanks, Christophe > Paul >
Re: [Fortran, Patch, PR120483, v1] Fix wrong type of saved allocatable strings.
Hi! On Mon, 2 Jun 2025 at 20:53, Andre Vehreschild wrote: > > Hi Thomas, > > thanks for the ok. Unfortunately does the patch regress in gomp (test case > gomp/pr104382 when I am not mistaken ; the one with the lone 'save' > statement). This was reported by the regression testing host at first for > arm, but also occurs on x86_64. Since when are proposed patches checked by a > CI? That's fantastic! > On the Linaro side, we put "postcommit" CI in production e/o summer 2023, and "precommit" CI a few weeks/months later. We made presentations during the GNU Cauldron 2023 and 2024, as well as during Linaro Connect 2024 :-) In summary we test various configurations of "arm" and "aarch64" targets. If you didn't notice before, it's because your patches were regression-free :-) Always happy to read positive feedback! Thanks Christophe > I will continue to investigate how to fix that issue. > > Regards, > Andre > Andre Vehreschild * ve...@gmx.de > > Am 2. Juni 2025 20:10:06 schrieb Thomas Koenig : > >> Hi Andre, >> >> >>> attached patch fixes a missing substring ref on a saved allocatable string. >>> The issue seems to be, that the variable is declared to be a character >>> pointer >>> and not a character array. When using the latter (why not), it works as >>> expected and does not produce any regressions. >>> >>> Regtests ok on x86_64-pc-linux-gnu / F41. Ok for mainlines? >> >> >> OK for trunk and also for backporting to gcc 15 (it is a 15/16 >> regression). >> >> Best regards >> >> Thomas > >
Re: [Patch] OpenMP: Add iterator support to Fortran's depend; add affinity clause
On Sat, 29 May 2021 at 10:03, Jakub Jelinek via Gcc-patches
wrote:
>
> On Fri, May 28, 2021 at 12:59:20AM +0200, Tobias Burnus wrote:
> > * gfortran.dg/gomp/depend-iterator-1.f90: New test.
> > * gfortran.dg/gomp/depend-iterator-2.f90: New test.
>
> Something I've missed during the review but it shows up during testing:
>
> > diff --git a/gcc/testsuite/gfortran.dg/gomp/depend-iterator-1.f90
> > b/gcc/testsuite/gfortran.dg/gomp/depend-iterator-1.f90
> > new file mode 100644
> > index 000..cad36aaf8b7
> > --- /dev/null
> > +++ b/gcc/testsuite/gfortran.dg/gomp/depend-iterator-1.f90
> > @@ -0,0 +1,45 @@
> > +! { dg-do run }
>
> gcc/testsuite/*/gomp/ shouldn't have dg-do run tests, either
> it is meant to be a runtime test and then it should be moved to
> libgomp/testsuite/libgomp.fortran/ , or it should be changed to
> dg-do compile.
>
In addition, on arm/aarch64 at least, I can see the test failing to compile:
gfortran: fatal error: cannot read spec file 'libgomp.spec': No such
file or directory
Christophe
> > --- /dev/null
> > +++ b/gcc/testsuite/gfortran.dg/gomp/depend-iterator-2.f90
> > @@ -0,0 +1,44 @@
> > +! { dg-do run }
>
> And this one has dg-error in it, so this one must be dg-do compile.
>
> > + !$omp task depend (iterator (j=i:i+1) , out : foo (j)) ! { dg-error
> > "is not a variable" }
> > +arr(i) = i
> > + !!$omp end task
> > +!$omp task depend(iterator(i=1:5), source ) ! { dg-error "ITERATOR
> > may not be combined with SOURCE" }
> > + !!$omp end task
> > + !$omp task affinity (iterator(i=1:5): a) depend(iterator(i=1:5), sink :
> > x) ! { dg-error "ITERATOR may not be combined with SINK" }
>
> Jakub
>
Re: [Patch v2] C, C++, Fortran, OpenMP: Add support for device-modifiers for 'omp target device'
On Mon, Aug 30, 2021 at 8:27 AM Jakub Jelinek via Gcc-patches < gcc-patc...@gcc.gnu.org> wrote: > On Wed, Aug 25, 2021 at 12:14:09PM +0200, Marcel Vollweiler wrote: > > Add support for device-modifiers for 'omp target device'. > > > > 'device_num' and 'ancestor' are now parsed on target device constructs > for C, > > C++, and Fortran (see OpenMP specification 5.0, p. 170). When 'ancestor' > is > > used, then 'sorry, not supported' is output. Moreover, the restrictions > for > > 'ancestor' are implemented (see OpenMP specification 5.0, p. 174f). > > > > gcc/c/ChangeLog: > > > > * c-parser.c (c_parser_omp_clause_device): Parse device-modifiers > 'device_num' > > and 'ancestor' in 'target device' clauses. > > > > gcc/cp/ChangeLog: > > > > * parser.c (cp_parser_omp_clause_device): Parse device-modifiers > 'device_num' > > and 'ancestor' in 'target device' clauses. > > * semantics.c (finish_omp_clauses): Error handling. Constant > device ids must > > evaluate to '1' if 'ancestor' is used. > > > > gcc/fortran/ChangeLog: > > > > * gfortran.h: Add variable for 'ancestor' in struct > gfc_omp_clauses. > > * openmp.c (gfc_match_omp_clauses): Parse device-modifiers > 'device_num' > > and 'ancestor' in 'target device' clauses. > > * trans-openmp.c (gfc_trans_omp_clauses): Set > OMP_CLAUSE_DEVICE_ANCESTOR. > > > > gcc/ChangeLog: > > > > * gimplify.c (gimplify_scan_omp_clauses): Error handling. > 'ancestor' only > > allowed on target constructs and only with particular other > clauses. > > * omp-expand.c (expand_omp_target): Output of 'sorry, not > supported' if > > 'ancestor' is used. > > * omp-low.c (check_omp_nesting_restrictions): Error handling. No > nested OpenMP > > structs when 'ancestor' is used. > > (scan_omp_1_stmt): No usage of OpenMP runtime routines in a target > region when > > 'ancestor' is used. > > * tree-pretty-print.c (dump_omp_clause): Append 'ancestor'. > > * tree.h (OMP_CLAUSE_DEVICE_ANCESTOR): Define macro. > > > > gcc/testsuite/ChangeLog: > > > > * c-c++-common/gomp/target-device-1.c: New test. > > * c-c++-common/gomp/target-device-2.c: New test. > > * c-c++-common/gomp/target-device-ancestor-1.c: New test. > > * c-c++-common/gomp/target-device-ancestor-2.c: New test. > > * c-c++-common/gomp/target-device-ancestor-3.c: New test. > > * c-c++-common/gomp/target-device-ancestor-4.c: New test. > > * gfortran.dg/gomp/target-device-1.f90: New test. > > * gfortran.dg/gomp/target-device-2.f90: New test. > > * gfortran.dg/gomp/target-device-ancestor-1.f90: New test. > > * gfortran.dg/gomp/target-device-ancestor-2.f90: New test. > > * gfortran.dg/gomp/target-device-ancestor-3.f90: New test. > > * gfortran.dg/gomp/target-device-ancestor-4.f90: New test. > The last new test fails on aarch64: /gcc/testsuite/gfortran.dg/gomp/target-device-ancestor-4.f90:7:15: Error: Sorry, 'reverse_offload' clause at (1) on REQUIRES directive is not yet supported compiler exited with status 1 PASS: gfortran.dg/gomp/target-device-ancestor-4.f90 -O (test for errors, line 7) XFAIL: gfortran.dg/gomp/target-device-ancestor-4.f90 -O sorry, unimplemented: 'ancestor' not yet supported (test for warnings, line 9) PASS: gfortran.dg/gomp/target-device-ancestor-4.f90 -O (test for excess errors) gfortran.dg/gomp/target-device-ancestor-4.f90 -O : dump file does not exist UNRESOLVED: gfortran.dg/gomp/target-device-ancestor-4.f90 -O scan-tree-dump original "pragma omp target [^\n\r)]*device\\(ancestor:1\\)" Can you fix it? Thanks, Christophe > Ok, thanks. > > Jakub > >
Re: [PATCH v3, Fortran] TS 29113 testsuite
Hi, On Thu, Aug 19, 2021 at 7:29 PM Sandra Loosemore wrote: > On 7/27/21 5:07 AM, Tobias Burnus wrote: > > Hi Sandra, hi Thomas, hi all, > > > > @Thomas K: Comments about the following - and of course to the > > testsuite itself - are highly welcome. > > > > In my opinion, the testsuite LGTM and can be committed. > > > > @Sandra: > > - Thoughts on the directory name? (cf. below) > > - Give others/Thomas a chance to comment on this, > >before committing it. (And remove the now passing xfails.) > >Thanks for the testsuite! > > > > Regarding: > > > > * XFAILS - as discussed before, I think having some XFAILS is > >not ideal but fine, especially if the XFAIL/PASS ratio is low > >and there are plans to fix the known fails, some posted patches > >for those, and open PRs for the issues. > > > > (I think there is one patch pending review and two patches pending > > committal with some modifications by Sandra - plus several patches > > by José which still need to be reviewed.) > > > > > > * Naming of the directory + .exp file: > > ts29113/ts29113.exp > >is okay. Given that 'select rank' (in F2018 but not in TS29113) > >is also tested, there was some controversy regarding the name > >and the coverage; additionally, TS29113 is a name which is not > >immediately clear. Thus, we could use some other name like: > > c-interop/c-interop.exp > >or (suggestions?). > >In any case, I do not feel strong about either name. > > > > * I had a closer look at earlier versions of the testsuite, I did > >browse through the current one + looked at the diff to previous > >version, but it is big enough and the spec is complex enough that > >I have likely missed something. > >Thus: Additional reviews are highly welcome! > > Here is the current version of the testsuite. Changes since the last > version include: > > * Renaming the directory and .exp file from ts29113 -> c-interop per the > request above. There have been no additional review comments. > > * I also made it explicit that section and constraint numbers mentioned > in comments in the test cases refer to TS 29113. I considered using the > numbering from 2018 standard, but given that the standard already > renumbered things twice since the time TS 29113 was published I didn't > really see the point, as long as it is unambiguous what document is > being cited. > > * I flattened the subdirectory structure after realizing that > dg-additional-sources can't cope with relative pathnames in remote-host > testing. > > * I split up the typecodes tests (for testing that descriptors > constructed by the front end match ISO_Fortran_binding.h) to allow for > finer-grained control over xfails and dg-require-effective-target, and > added a new effective target for Fortran C_FLOAT128 support. There are > also some additional things being tested now in this group. > > The current xfails in the tests reflect the two patches I posted last > night that are still waiting for review: > > https://gcc.gnu.org/pipermail/fortran/2021-August/056382.html > https://gcc.gnu.org/pipermail/fortran/2021-August/056383.html > > I've been testing on x86 (both 32- and 64-bit) and powerpc64le-linux-gnu. > > I'm not quite sure I understand the expected status of this patch: are all the "expected" failures tagged as XFAIL? If yes, then there's a problem because I see lots of unresolved on aarch64/arm For instance on aarch64: /gcc/testsuite/gfortran.dg/c-interop/cf-descriptor-5.f90:10:19: Error: Sorry, character dummy argument 'a' at (1) with assumed length is not yet supported for procedure 'ftest' with BIND(C) attribute compiler exited with status 1 XFAIL: gfortran.dg/c-interop/cf-descriptor-5.f90 -O0 pr92482 (test for bogus messages, line 10) PASS: gfortran.dg/c-interop/cf-descriptor-5.f90 -O0 (test for excess errors) UNRESOLVED: gfortran.dg/c-interop/cf-descriptor-5.f90 -O0 compilation failed to produce executable /gcc/testsuite/gfortran.dg/c-interop/cf-out-descriptor-5.f90:9:19: Error: Sorry, character dummy argument 'a' at (1) with assumed length is not yet supported for procedure 'ftest' with BIND(C) attribute /gcc/testsuite/gfortran.dg/c-interop/cf-out-descriptor-5.f90:23:23: Error: Sorry, character dummy argument 'a' at (1) with assumed length is not yet supported for procedure 'ctest' with BIND(C) attribute /gcc/testsuite/gfortran.dg/c-interop/cf-out-descriptor-5.f90:29:23: Error: Sorry, character dummy argument 'a' at (1) with assumed length is not yet supported for procedure 'ftest' with BIND(C) attribute compiler exited with status 1 XFAIL: gfortran.dg/c-interop/cf-out-descriptor-5.f90 -O0 pr92482 (test for bogus messages, line 9) XFAIL: gfortran.dg/c-interop/cf-out-descriptor-5.f90 -O0 pr92482 (test for bogus messages, line 23) XFAIL: gfortran.dg/c-interop/cf-out-descriptor-5.f90 -O0 pr92482 (test for bogus messages, line 29) PASS: gfortran.dg/c-interop/cf-out-descriptor-5.f90 -O0 (test for excess e
Re: [PATCH, Fortran] Skip gfortran.dg/PR100914.f90 on targets that don't provide quadmath.h
On Mon, Sep 6, 2021 at 7:21 AM Sandra Loosemore wrote: > On 9/5/21 7:29 PM, H.J. Lu wrote: > > On Sun, Sep 5, 2021 at 11:02 AM Sandra Loosemore > > wrote: > >> > >> On 9/5/21 7:31 AM, H.J. Lu wrote: > >>> On Sat, Sep 4, 2021 at 7:31 PM Sandra Loosemore < > san...@codesourcery.com> wrote: > > The testcase gfortran.dg/PR100914.f90 that I recently checked in > (originally written by José Rui Faustino de Sousa) depends on the > header file to obtain a typedef for __complex128. It > appears not to be possible to define an equivalent type in a portable > way in the testcase itself (see > https://gcc.gnu.org/onlinedocs/gcc-11.2.0/gcc/Floating-Types.html) so > this patch skips the test entirely on targets where quadmath.h is not > available. > > The target-supports.exp change was cut-and-pasted from similar code in > that file, but I haven't figured out how to test this change in a > build > that doesn't provide quadmath.h (e.g., my aarch64-linux-gnu toolchain > build attempt croaked with an unrelated compilation error in glibc). > Perhaps someone who previously encountered the FAILs on this testcase > can confirm that it's skipped with this change? > >>> > >>> Since libqaudmath provides , I prefer either of 2 patches > >>> enclosed here. > >> > >> Of these two, the first one looks better to me, and seems to work OK in > >> my x86 build. But, I'm not sure it is the right thing for ARM/Aarch64 > >> targets, which apparently have _Float128 support without the __float128 > >> type or libquadmath. It's pretty clear quadmath.h depends on having > >> __float128. > > > > The only used by GCC is the one in libquadmath. I > > will check in my first patch tomorrow if there are no objections. > > > >> See Christophe's mail here: > >> > >> https://gcc.gnu.org/pipermail/fortran/2021-September/056467.html > >> > >> As I said in my last mail, I ran into some problems getting an aarch64 > >> toolchain built so I haven't been able to do any testing or experiments > >> on that target myself yet. :-( > > I finally got an aarch64-linux-gnu toolchain built and confirmed that it > is still broken there: it indeed does not define __float128, and > including quadmath.h results in a gazillion errors like > > /path/to/quadmath.h:47:8: error: unknown type name '__float128' > > I also observed that _Float128 is the same type as long double on this > target. > > Unless the aarch64 maintainers think it is a bug that __float128 is not > supported, I think the right solution here is the one I was thinking of > previously, to fix the Fortran front end to tie the C_FLOAT128 kind to > _Float128 rather than __float128, and fix the runtime support and test > cases accordingly. Then there should be no need to depend on quadmath.h > at all. C_FLOAT128 is a GNU extension and _Float128 is supported on a > superset of targets that __float128 is, so there should be no issue with > backward compatibility. > > I'm not really in a position to make a comment, but any patch aiming at skipping pr100914.f90 only will have no effect on the other errors I reported, which are related to __float128 vs _Float128. Christophe > -Sandra >
