[PATCH, fortran] Fix simple typo in libgfortran
Hi Tobias, Thanks for your time and guidance on this GSOC project. > * This is a trivial patch but for larger patches, you need >grant the right to use your patch. There are two ways of doing so: >(a) The simple one, called "Developer Certificate of Origin" (DCO). >This requires a "Signed-off-by:" line (as added by 'git commit -s'. >(b) A copyright assignment with the Free Software Foundation (FSF). You're right. I found the information on the website stating that minor patches can omit the DCO. I have now added the "Signed-off-by:" line in the new patch. > (a) FYI - if you run for the last commit > ./contrib/gcc-changelog/git_check_commit.py -v -p or for some commit > ./contrib/gcc-changelog/git_check_commit.py -v -p Then you > see how the resulting ChangeLog will look like - it also shows errors > and warnings. (That's also used on the server.) (b) Small nit picking: Thanks for the tip! I ran the git_check_commit.py script, and it reported "OK." However, when I regenerated the ChangeLog, the function name wasn't included automatically. I've manually added the function name to the ChangeLog for now and will investigate the script's behavior later. The updated patch is attached for your review. Yuao 0001-fortran-fix-simple-typo-in-libgfortran.patch Description: 0001-fortran-fix-simple-typo-in-libgfortran.patch
[PATCH] fortran: map atand(y, x) to atan2d(y, x)
Hi all, According to the Fortran standard, atand(y, x) is equivalent to atan2d(y, x). However, the current atand(y, x) function produces an error. This patch includes the necessary intrinsic mapping, related test, and intrinsic documentation. The minor comment change in intrinsic.cc is cherry-picked from Steve's previous work. Best regards, Yuao 0001-fortran-map-atand-y-x-to-atan2d-y-x.patch Description: 0001-fortran-map-atand-y-x-to-atan2d-y-x.patch
[PATCH, fortran] Fix simple typo in libgfortran
Hi GCC Maintainers, I'm writing to express my sincere gratitude for the opportunity to participate in Google Summer of Code with GCC this year. I am very enthusiastic about this program and fully committed to making a valuable contribution and fulfilling my responsibilities. As I am relatively new to submitting patches via mailing lists, I would like to send a very simple patch primarily to familiarize myself with the correct procedure. I have reviewed the contribution guidelines on the GCC website and have tried to follow them closely. Please let me know if this approach is acceptable. I'm eager to learn and ensure my future contributions align with the project's standards. Thank you for your time and guidance. Best regards, Yuao 0001-Fix-simple-typo-in-libgfortran.patch Description: 0001-Fix-simple-typo-in-libgfortran.patch
[PATCH, fortran] Fix simple typo in libgfortran
Hi Thomas, Thanks for your quick response. > There should be a ChangeLog entry. You can generate a template by > running your patch through contrib/mklog.py, which you can then fill > out with the details. I think the ChangeLog entry is already in the patch. I used git gcc-commit-mklog, and it produced the following: ``` libgfortran/ChangeLog: * io/read.c: typo fix, explict -> explicit ``` Is this sufficient, or does it need more detail? Yuao 0001-Fix-simple-typo-in-libgfortran.patch Description: 0001-Fix-simple-typo-in-libgfortran.patch
Re: [PATCH] libgfortran: add fallback for trigonometric pi-based functions
On 7/8/2025 2:03 AM, Steve Kargl wrote: On Sun, Jul 06, 2025 at 09:09:53PM +0800, Yuao Ma wrote: Hi Tobias, On 7/6/2025 6:34 PM, Tobias Burnus wrote: As that commit is from 2020 and 2.69 in from 2012, it seems as if your autoconf is too new. Can you re-check that the right version is at the beginning of the PATH? Note that there is a CI job that checks whether the generated files are in deed up to date, i.e. the current trunk should be fine unless someone has messed up. It is possible that autoconf 2.69 contains this commit, as we can see from https://github.com/autotools-mirror/autoconf/commit/a1d8293f3bfa2516f9a0424e3a6e63c2f8e93c6e that it has been backported to v2.69b - v2.69e. The main reason is that my devbox has autoconf2.69 (2.69-3.1) from Debian, released on Sat, 19 Nov 2022 21:40:11 +0200, which includes the commit from 2020. This version takes precedence over my compiled version. Once I switch to my compiled version, the generation output functions as expected. The other question is whether the autoconf version shouldn't be updated, given that 2.69 is quite old. Upgrading this basic component may seem like a major change, but opting for the same version with a backported patch appears to be a better choice. Please see https://gcc.gnu.org/install/prerequisites.html. That page specifically says "autoconf version 2.69"; not some 2.69-patch-level. Everyone using the same release prevents conflicting changes. Yes, I am aware of that. The newest version of the patch now uses the correct autoconf version to generate the configure file, eliminating the LARGE_OFF_T noise. Besides the LARGE_OFF_T issue, are there any other issues this patch needs to address? Thanks, Yuao
Re: [PATCH] libquadmath: add quad support for trig-pi functions
Hi all, On 7/3/2025 9:21 PM, Jakub Jelinek wrote: On Thu, Jul 03, 2025 at 02:43:43PM +0200, Michael Matz wrote: Hello, On Thu, 3 Jul 2025, Yuao Ma wrote: This patch adds the required function for Fortran trigonometric functions to work with glibc versions prior to 2.26. It's based on glibc source commit 632d895f3e5d98162f77b9c3c1da4ec19968b671. I've built it successfully on my end. Documentation is also included. Please take a look when you have a moment. +__float128 +cospiq (__float128 x) +{ ... + if (__builtin_islessequal (x, 0.25Q)) +return cosq (M_PIq * x); Isn't the whole raison d'etre for the trig-pi functions that the internal argument reduction against multiples of pi becomes trivial and hence (a) performant, and (b) doesn't introduce rounding artifacts? Expressing the trig-pi functions in terms of their counterparts completely defeats this purpose. The other way around would be more sensible for the cases where it works, but the above doesn't seem very attractive. glibc has FLOAT M_DECL_FUNC (__cospi) (FLOAT x) { if (isless (M_FABS (x), M_EPSILON)) return M_LIT (1.0); if (__glibc_unlikely (isinf (x))) __set_errno (EDOM); x = M_FABS (x - M_LIT (2.0) * M_SUF (round) (M_LIT (0.5) * x)); if (islessequal (x, M_LIT (0.25))) return M_SUF (__cos) (M_MLIT (M_PI) * x); else if (x == M_LIT (0.5)) return M_LIT (0.0); else if (islessequal (x, M_LIT (0.75))) return M_SUF (__sin) (M_MLIT (M_PI) * (M_LIT (0.5) - x)); else return -M_SUF (__cos) (M_MLIT (M_PI) * (M_LIT (1.0) - x)); } for this case, so if it is incorrect, has too bad precision or there are better ways to do it, then perhaps it should be changed on the glibc side first and then ported to libquadmath. That's exactly what I want to say. I only touched the generation script; the other main changes are borrowed from glibc. Therefore, it should be at least as precise as the glibc generic implementation. If the current implementation isn't sufficient, we can update glibc first and then mirror the changes back. This is only used for systems with glibc versions prior to 2.26. Modern systems will use the implementation directly from glibc, which should avoid this potential issue. Regarding math function implementation, I think LLVM-libc provides a great example. While it may not be as performant as its glibc equivalent, its structure is very clear. Even for a newcomer like me, who isn't very familiar with computational mathematics, I can grasp the basic ideas of implementation and testing, such as nan/inf handling, polynomial curve fitting, exception values, and range reduction. I'd love to learn from and contribute to the glibc generic implementation as Joseph suggested, but it seems it will take more time to get familiar with the glibc source code. Hopefully, I'll be able to do that one day. Regarding the patch itself, the previous version was missing the declaration in the header file. The newly attached version fixes this. Is this patch good to go, or does it need further amendment? Looking forward to your further review comments. Thanks, Yuao From 719886689fd21555c9ba1bd63a44111408c0f01d Mon Sep 17 00:00:00 2001 From: Yuao Ma Date: Thu, 3 Jul 2025 23:01:38 +0800 Subject: [PATCH] libquadmath: add quad support for trig-pi functions This function is required for Fortran trigonometric functions with glibc <2.26. Use glibc commit 632d895f3e5d98162f77b9c3c1da4ec19968b671. libquadmath/ChangeLog: * Makefile.am: Add sources to makefile. * Makefile.in: Regen makefile. * libquadmath.texi: Add doc for trig-pi funcs. * quadmath.h (acospiq, asinpiq, atanpiq, atan2piq, cospiq, sinpiq, tanpiq): New. * update-quadmath.py: Update generation script. * math/acospiq.c: New file. * math/asinpiq.c: New file. * math/atan2piq.c: New file. * math/atanpiq.c: New file. * math/cospiq.c: New file. * math/sinpiq.c: New file. * math/tanpiq.c: New file. Signed-off-by: Yuao Ma --- libquadmath/Makefile.am| 2 ++ libquadmath/Makefile.in| 26 -- libquadmath/libquadmath.texi | 7 libquadmath/math/acospiq.c | 33 ++ libquadmath/math/asinpiq.c | 40 ++ libquadmath/math/atan2piq.c| 36 libquadmath/math/atanpiq.c | 35 +++ libquadmath/math/cospiq.c | 37 libquadmath/math/sinpiq.c | 44 libquadmath/math/tanpiq.c | 62 ++ libquadmath/quadmath.h | 7 libquadmath/update-quadmath.py | 48 ++ 12 files changed, 359 insertions(+), 18 deletions(-) create mode 100644 libquadmath/math/acospiq.c create mode 100644 libquadmath/math/asinpiq.c create mode 100644 libquadmath/math/atan2piq.c
[PATCH] libgfortran: add fallback for trigonometric pi-based functions
Hi all, This patch introduces a fallback implementation for trigonometric pi-based functions. This implementation supports float, double, and long double data types. I've revised the test cases for r4 and r8 types, and all tests passed successfully on the aarch64-linux platform. If this looks good, I will address f128/r16 in a subsequent patch. (A related patch concerning libquadmath is currently under review.) Thanks, YuaoFrom f8f2031e5e4bcd03e7382342907334cd33dd2ec6 Mon Sep 17 00:00:00 2001 From: Yuao Ma Date: Sat, 5 Jul 2025 17:06:18 +0800 Subject: [PATCH] libgfortran: add fallback for trigonometric pi-based functions This patch introduces a fallback implementation for pi-based trigonometric functions, ensuring broader compatibility and robustness. The new implementation supports float, double, and long double data types. Accordingly, the test cases for r4 and r8 have been revised to reflect these changes. libgfortran/ChangeLog: * Makefile.am: Add c23_functions.c to Makefile. * Makefile.in: Regenerate. * config.h.in: Regenerate. * configure: Regenerate. * configure.ac: Check if trig-pi functions exist. * gfortran.map: Add a section for c23 functions. * libgfortran.h: Include c23 proto file. * c23_protos.h: Add c23 proto file and trig-pi funcs. * intrinsics/c23_functions.c: Add trig-pi fallback impls. gcc/testsuite/ChangeLog: * gfortran.dg/dec_math_5.f90: Revise test to use fallback. Signed-off-by: Yuao Ma Co-authored-by: Steven G. Kargl --- gcc/testsuite/gfortran.dg/dec_math_5.f90 | 36 +- libgfortran/Makefile.am |4 + libgfortran/Makefile.in |8 + libgfortran/c23_protos.h | 133 +++ libgfortran/config.h.in | 63 ++ libgfortran/configure| 1020 +- libgfortran/configure.ac | 23 + libgfortran/gfortran.map | 25 + libgfortran/intrinsics/c23_functions.c | 308 +++ libgfortran/libgfortran.h|1 + 10 files changed, 1598 insertions(+), 23 deletions(-) create mode 100644 libgfortran/c23_protos.h create mode 100644 libgfortran/intrinsics/c23_functions.c diff --git a/gcc/testsuite/gfortran.dg/dec_math_5.f90 b/gcc/testsuite/gfortran.dg/dec_math_5.f90 index a7ff3275236..7bcf07fce67 100644 --- a/gcc/testsuite/gfortran.dg/dec_math_5.f90 +++ b/gcc/testsuite/gfortran.dg/dec_math_5.f90 @@ -102,26 +102,26 @@ program p if (abs(c1 - 0.5) > e3) stop 39 if (abs(d1 - 0.5) > e4) stop 40 - a1 = acospi(0.5) - b1 = acospi(-0.5) + a1 = 0.5; a1 = acospi(a1) + b1 = -0.5; b1 = acospi(b1) c1 = acospi(0.5) d1 = acospi(-0.5) - if (abs(a1 - 1.0 / 3) > e1) stop 41 - if (abs(b1 - 2.0 / 3) > e2) stop 42 + if (abs(a1 - 1._4 / 3) > e1) stop 41 + if (abs(b1 - 2._8 / 3) > e2) stop 42 if (abs(c1 - 1.0 / 3) > e3) stop 43 if (abs(d1 - 2.0 / 3) > e4) stop 44 - a1 = asinpi(0.5) - b1 = asinpi(-0.5) + a1 = 0.5; a1 = asinpi(a1) + b1 = -0.5; b1 = asinpi(b1) c1 = asinpi(0.5) d1 = asinpi(-0.5) - if (abs(a1 - 1.0 / 6) > e1) stop 45 - if (abs(b1 + 1.0 / 6) > e2) stop 46 + if (abs(a1 - 1._4 / 6) > e1) stop 45 + if (abs(b1 + 1._8 / 6) > e2) stop 46 if (abs(c1 - 1.0 / 6) > e3) stop 47 if (abs(d1 + 1.0 / 6) > e4) stop 48 - a1 = atanpi(1.0) - b1 = atanpi(-1.0) + a1 = 1.0; a1 = atanpi(a1) + b1 = -1.0; b1 = atanpi(b1) c1 = atanpi(1.0) d1 = atanpi(-1.0) if (abs(a1 - 0.25) > e1) stop 49 @@ -129,8 +129,8 @@ program p if (abs(c1 - 0.25) > e3) stop 51 if (abs(d1 + 0.25) > e4) stop 52 - a1 = atan2pi(1.0, 1.0) - b1 = atan2pi(1.0, 1.0) + a1 = 1.0; a1 = atan2pi(a1, a1) + b1 = 1.0; b1 = atan2pi(b1, b1) c1 = atan2pi(1.0, 1.0) d1 = atan2pi(1.0, 1.0) if (abs(a1 - 0.25) > e1) stop 53 @@ -138,8 +138,8 @@ program p if (abs(c1 - 0.25) > e3) stop 55 if (abs(d1 - 0.25) > e4) stop 56 - a1 = cospi(1._4 / 3) - b1 = cospi(-1._8 / 3) + a1 = 1._4 / 3; a1 = cospi(a1) + b1 = -1._8 / 3; b1 = cospi(b1) c1 = cospi(4._ep / 3) d1 = cospi(-4._16 / 3) if (abs(a1 - 0.5) > e1) stop 57 @@ -147,8 +147,8 @@ program p if (abs(c1 + 0.5) > e3) stop 59 if (abs(d1 + 0.5) > e4) stop 60 - a1 = sinpi(1._4 / 6) - b1 = sinpi(-1._8 / 6) + a1 = 1._4 / 6; a1 = sinpi(a1) + b1 = -1._8 / 6; b1 = sinpi(b1) c1 = sinpi(5._ep / 6) d1 = sinpi(-7._16 / 6) if (abs(a1 - 0.5) > e1) stop 61 @@ -156,8 +156,8 @@ program p if (abs(c1 - 0.5) > e3) stop 63 if (abs(d1 - 0.5) > e4) stop 64 - a1 = tanpi(0.25) - b1 = tanpi(-0.25) + a1 = 0.25; a1 = tanpi(a1) + b1 = -0.25; b1 = tanpi(b1) c1 = tanpi(1.25) d1 = tanpi(-1.25) if (abs(a1 - 1.0) > e1) stop 65 diff --git a/libgfortran/Makefile.am b/libgfortran/Makefile.am index 4f3b3033224..82cf51b6bcc 100644 --- a/libgfortran/Makefile.am +++ b/libgfortran/Makefile.am @@ -165,6 +
Re: [PATCH] libgfortran: add fallback for trigonometric pi-based functions
Hi Tobias, On 7/6/2025 6:34 PM, Tobias Burnus wrote: As that commit is from 2020 and 2.69 in from 2012, it seems as if your autoconf is too new. Can you re-check that the right version is at the beginning of the PATH? Note that there is a CI job that checks whether the generated files are in deed up to date, i.e. the current trunk should be fine unless someone has messed up. It is possible that autoconf 2.69 contains this commit, as we can see from https://github.com/autotools-mirror/autoconf/commit/a1d8293f3bfa2516f9a0424e3a6e63c2f8e93c6e that it has been backported to v2.69b - v2.69e. The main reason is that my devbox has autoconf2.69 (2.69-3.1) from Debian, released on Sat, 19 Nov 2022 21:40:11 +0200, which includes the commit from 2020. This version takes precedence over my compiled version. Once I switch to my compiled version, the generation output functions as expected. The other question is whether the autoconf version shouldn't be updated, given that 2.69 is quite old. Upgrading this basic component may seem like a major change, but opting for the same version with a backported patch appears to be a better choice. I have used the old version to create a new patch. Hope this looks good to you. YuaoFrom 2d62a2f707e43f37b4d886b7ed3aa40f2ab62437 Mon Sep 17 00:00:00 2001 From: Yuao Ma Date: Sun, 6 Jul 2025 20:55:08 +0800 Subject: [PATCH] libgfortran: add fallback for trigonometric pi-based functions This patch introduces a fallback implementation for pi-based trigonometric functions, ensuring broader compatibility and robustness. The new implementation supports float, double, and long double data types. Accordingly, the test cases for r4 and r8 have been revised to reflect these changes. libgfortran/ChangeLog: * Makefile.am: Add c23_functions.c to Makefile. * Makefile.in: Regenerate. * config.h.in: Regenerate. * configure: Regenerate. * configure.ac: Check if trig-pi functions exist. * gfortran.map: Add a section for c23 functions. * libgfortran.h: Include c23 proto file. * c23_protos.h: Add c23 proto file and trig-pi funcs. * intrinsics/c23_functions.c: Add trig-pi fallback impls. gcc/testsuite/ChangeLog: * gfortran.dg/dec_math_5.f90: Revise test to use fallback. Signed-off-by: Yuao Ma Co-authored-by: Steven G. Kargl --- gcc/testsuite/gfortran.dg/dec_math_5.f90 | 36 +- libgfortran/Makefile.am |4 + libgfortran/Makefile.in |8 + libgfortran/c23_protos.h | 133 +++ libgfortran/config.h.in | 63 ++ libgfortran/configure| 1010 ++ libgfortran/configure.ac | 23 + libgfortran/gfortran.map | 25 + libgfortran/intrinsics/c23_functions.c | 308 +++ libgfortran/libgfortran.h|1 + 10 files changed, 1593 insertions(+), 18 deletions(-) create mode 100644 libgfortran/c23_protos.h create mode 100644 libgfortran/intrinsics/c23_functions.c diff --git a/gcc/testsuite/gfortran.dg/dec_math_5.f90 b/gcc/testsuite/gfortran.dg/dec_math_5.f90 index a7ff3275236..7bcf07fce67 100644 --- a/gcc/testsuite/gfortran.dg/dec_math_5.f90 +++ b/gcc/testsuite/gfortran.dg/dec_math_5.f90 @@ -102,26 +102,26 @@ program p if (abs(c1 - 0.5) > e3) stop 39 if (abs(d1 - 0.5) > e4) stop 40 - a1 = acospi(0.5) - b1 = acospi(-0.5) + a1 = 0.5; a1 = acospi(a1) + b1 = -0.5; b1 = acospi(b1) c1 = acospi(0.5) d1 = acospi(-0.5) - if (abs(a1 - 1.0 / 3) > e1) stop 41 - if (abs(b1 - 2.0 / 3) > e2) stop 42 + if (abs(a1 - 1._4 / 3) > e1) stop 41 + if (abs(b1 - 2._8 / 3) > e2) stop 42 if (abs(c1 - 1.0 / 3) > e3) stop 43 if (abs(d1 - 2.0 / 3) > e4) stop 44 - a1 = asinpi(0.5) - b1 = asinpi(-0.5) + a1 = 0.5; a1 = asinpi(a1) + b1 = -0.5; b1 = asinpi(b1) c1 = asinpi(0.5) d1 = asinpi(-0.5) - if (abs(a1 - 1.0 / 6) > e1) stop 45 - if (abs(b1 + 1.0 / 6) > e2) stop 46 + if (abs(a1 - 1._4 / 6) > e1) stop 45 + if (abs(b1 + 1._8 / 6) > e2) stop 46 if (abs(c1 - 1.0 / 6) > e3) stop 47 if (abs(d1 + 1.0 / 6) > e4) stop 48 - a1 = atanpi(1.0) - b1 = atanpi(-1.0) + a1 = 1.0; a1 = atanpi(a1) + b1 = -1.0; b1 = atanpi(b1) c1 = atanpi(1.0) d1 = atanpi(-1.0) if (abs(a1 - 0.25) > e1) stop 49 @@ -129,8 +129,8 @@ program p if (abs(c1 - 0.25) > e3) stop 51 if (abs(d1 + 0.25) > e4) stop 52 - a1 = atan2pi(1.0, 1.0) - b1 = atan2pi(1.0, 1.0) + a1 = 1.0; a1 = atan2pi(a1, a1) + b1 = 1.0; b1 = atan2pi(b1, b1) c1 = atan2pi(1.0, 1.0) d1 = atan2pi(1.0, 1.0) if (abs(a1 - 0.25) > e1) stop 53 @@ -138,8 +138,8 @@ program p if (abs(c1 - 0.25) > e3) stop 55 if (abs(d1 - 0.25) > e4) stop 56 - a1 = cospi(1._4 / 3) - b1 = cospi(-1._8 / 3) + a1 = 1._4 / 3; a1 = cospi(a1) + b1 = -1._8 / 3; b1 = cospi(b1) c1 = cospi(4._ep / 3) d1 = cosp
Re: [PATCH] libgfortran: add fallback for trigonometric pi-based functions
On 7/6/2025 12:49 PM, Steve Kargl wrote: On Sun, Jul 06, 2025 at 08:43:06AM +0800, Yuao Ma wrote: Hi Steve, On 7/6/2025 12:25 AM, Steve Kargl wrote: On Sat, Jul 05, 2025 at 05:20:02PM +0800, Yuao Ma wrote: diff --git a/libgfortran/configure b/libgfortran/configure index 9898a94a372..971f1e9df5e 100755 --- a/libgfortran/configure +++ b/libgfortran/configure @@ -16413,7 +16413,7 @@ else We can't simply define LARGE_OFF_T to be 9223372036854775807, since some C++ compilers masquerading as C compilers incorrectly reject 9223372036854775807. */ -#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62)) +#define LARGE_OFF_T off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31)) What is the purpose of this change? Since I don't have root/sudo permissions on my devbox, I manually downloaded and compiled the autoconf 2.69 tarball. This means there might be some minor discrepancies compared to the version shipped with OS distributions. I suspect the issue could be related to platforms where `off_t` is 32-bit, causing a left shift of 62 to result in undefined behavior. The commit at https://cgit.git.savannah.gnu.org/cgit/autoconf.git/commit/?id=a1d8293f3bfa2516f9a0424e3a6e63c2f8e93c6e seems to support my theory. This patch is not okay to commit with this change. Changing LARGE_OFF_T has nothing to do with implementing the half-cycle trig functions. Would it be possible to regenerate the configure file in a separate patch first, before we address the trig-pi patch? I believe this regeneration is a bug fix originating from autoconf 2.69, and it would be beneficial for GCC to incorporate this modification. Beyond libgfortran, libcpp and libiberty are also affected by this issue. This is indeed the direct output from my autoconf 2.69, and manually reverting parts of the generated file seems odd. Additionally, besides the LARGE_OFF_T issue, are there any other issues this patch needs to address? Yuao
Re: [PATCH] libgfortran: add fallback for trigonometric pi-based functions
Hi Steve, On 7/6/2025 12:25 AM, Steve Kargl wrote: On Sat, Jul 05, 2025 at 05:20:02PM +0800, Yuao Ma wrote: diff --git a/libgfortran/configure b/libgfortran/configure index 9898a94a372..971f1e9df5e 100755 --- a/libgfortran/configure +++ b/libgfortran/configure @@ -16413,7 +16413,7 @@ else We can't simply define LARGE_OFF_T to be 9223372036854775807, since some C++ compilers masquerading as C compilers incorrectly reject 9223372036854775807. */ -#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62)) +#define LARGE_OFF_T off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31)) What is the purpose of this change? Since I don't have root/sudo permissions on my devbox, I manually downloaded and compiled the autoconf 2.69 tarball. This means there might be some minor discrepancies compared to the version shipped with OS distributions. I suspect the issue could be related to platforms where `off_t` is 32-bit, causing a left shift of 62 to result in undefined behavior. The commit at https://cgit.git.savannah.gnu.org/cgit/autoconf.git/commit/?id=a1d8293f3bfa2516f9a0424e3a6e63c2f8e93c6e seems to support my theory. Thanks, Yuao
[PATCH] libquadmath: add quad support for trig-pi functions
Hi all, This patch adds the required function for Fortran trigonometric functions to work with glibc versions prior to 2.26. It's based on glibc source commit 632d895f3e5d98162f77b9c3c1da4ec19968b671. I've built it successfully on my end. Documentation is also included. Please take a look when you have a moment. Best regards, YuaoFrom 46ed3a1817e87567a7510eb4ca918589afcc9c3c Mon Sep 17 00:00:00 2001 From: Yuao Ma Date: Thu, 3 Jul 2025 00:40:58 +0800 Subject: [PATCH] libquadmath: add quad support for trig-pi functions This function is required for Fortran trigonometric functions with glibc <2.26. Use glibc commit 632d895f3e5d98162f77b9c3c1da4ec19968b671. libquadmath/ChangeLog: * Makefile.am: Add sources to makefile. * Makefile.in: Regen makefile. * libquadmath.texi: Add doc for trig-pi funcs. * update-quadmath.py: Update generation script. * math/acospiq.c: New file. * math/asinpiq.c: New file. * math/atan2piq.c: New file. * math/atanpiq.c: New file. * math/cospiq.c: New file. * math/sinpiq.c: New file. * math/tanpiq.c: New file. Signed-off-by: Yuao Ma --- libquadmath/Makefile.am| 2 ++ libquadmath/Makefile.in| 26 -- libquadmath/libquadmath.texi | 7 libquadmath/math/acospiq.c | 33 ++ libquadmath/math/asinpiq.c | 40 ++ libquadmath/math/atan2piq.c| 36 libquadmath/math/atanpiq.c | 35 +++ libquadmath/math/cospiq.c | 37 libquadmath/math/sinpiq.c | 44 libquadmath/math/tanpiq.c | 62 ++ libquadmath/update-quadmath.py | 48 ++ 11 files changed, 352 insertions(+), 18 deletions(-) create mode 100644 libquadmath/math/acospiq.c create mode 100644 libquadmath/math/asinpiq.c create mode 100644 libquadmath/math/atan2piq.c create mode 100644 libquadmath/math/atanpiq.c create mode 100644 libquadmath/math/cospiq.c create mode 100644 libquadmath/math/sinpiq.c create mode 100644 libquadmath/math/tanpiq.c diff --git a/libquadmath/Makefile.am b/libquadmath/Makefile.am index 93806106abb..a1b17fd7897 100644 --- a/libquadmath/Makefile.am +++ b/libquadmath/Makefile.am @@ -70,6 +70,8 @@ libquadmath_la_SOURCES = \ math/llrintq.c math/log2q.c math/lrintq.c math/nearbyintq.c math/remquoq.c \ math/ccoshq.c math/cexpq.c math/clog10q.c math/clogq.c math/csinq.c \ math/csinhq.c math/csqrtq.c math/ctanq.c math/ctanhq.c \ + math/acospiq.c math/asinpiq.c math/atanpiq.c math/atan2piq.c \ + math/cospiq.c math/sinpiq.c math/tanpiq.c \ printf/addmul_1.c printf/add_n.c printf/cmp.c printf/divrem.c \ printf/flt1282mpn.c printf/fpioconst.c printf/lshift.c printf/mul_1.c \ printf/mul_n.c printf/mul.c printf/printf_fphex.c printf/printf_fp.c \ diff --git a/libquadmath/Makefile.in b/libquadmath/Makefile.in index ff3373064b1..b1d542c 100644 --- a/libquadmath/Makefile.in +++ b/libquadmath/Makefile.in @@ -197,9 +197,13 @@ am__dirstamp = $(am__leading_dot)dirstamp @BUILD_LIBQUADMATH_TRUE@ math/clog10q.lo math/clogq.lo \ @BUILD_LIBQUADMATH_TRUE@ math/csinq.lo math/csinhq.lo \ @BUILD_LIBQUADMATH_TRUE@ math/csqrtq.lo math/ctanq.lo \ -@BUILD_LIBQUADMATH_TRUE@ math/ctanhq.lo printf/addmul_1.lo \ -@BUILD_LIBQUADMATH_TRUE@ printf/add_n.lo printf/cmp.lo \ -@BUILD_LIBQUADMATH_TRUE@ printf/divrem.lo printf/flt1282mpn.lo \ +@BUILD_LIBQUADMATH_TRUE@ math/ctanhq.lo math/acospiq.lo \ +@BUILD_LIBQUADMATH_TRUE@ math/asinpiq.lo math/atanpiq.lo \ +@BUILD_LIBQUADMATH_TRUE@ math/atan2piq.lo math/cospiq.lo \ +@BUILD_LIBQUADMATH_TRUE@ math/sinpiq.lo math/tanpiq.lo \ +@BUILD_LIBQUADMATH_TRUE@ printf/addmul_1.lo printf/add_n.lo \ +@BUILD_LIBQUADMATH_TRUE@ printf/cmp.lo printf/divrem.lo \ +@BUILD_LIBQUADMATH_TRUE@ printf/flt1282mpn.lo \ @BUILD_LIBQUADMATH_TRUE@ printf/fpioconst.lo printf/lshift.lo \ @BUILD_LIBQUADMATH_TRUE@ printf/mul_1.lo printf/mul_n.lo \ @BUILD_LIBQUADMATH_TRUE@ printf/mul.lo printf/printf_fphex.lo \ @@ -495,6 +499,8 @@ AUTOMAKE_OPTIONS = foreign info-in-builddir @BUILD_LIBQUADMATH_TRUE@ math/llrintq.c math/log2q.c math/lrintq.c math/nearbyintq.c math/remquoq.c \ @BUILD_LIBQUADMATH_TRUE@ math/ccoshq.c math/cexpq.c math/clog10q.c math/clogq.c math/csinq.c \ @BUILD_LIBQUADMATH_TRUE@ math/csinhq.c math/csqrtq.c math/ctanq.c math/ctanhq.c \ +@BUILD_LIBQUADMATH_TRUE@ math/acospiq.c math/asinpiq.c math/atanpiq.c math/atan2piq.c \ +@BUILD_LIBQUADMATH_TRUE@ math/cospiq.c math/sinpiq.c math/tanpiq.c \ @BUILD_LIBQUADMATH_TRUE@ printf/addmul_1.c printf/add_n.c printf/cmp.c printf/divrem.c \ @BUILD_LIBQUADMATH_TRUE@ printf/flt1282mpn.c printf/fpioconst.c printf/lshift.c printf/mul_1.c \ @BUILD_LIBQUADMATH_TRUE@ printf/mul_n.c printf/mul.c printf/printf_fp
[PATCH] gcc: middle-end opt for trigonometric pi-based functions builtins
Hi Jakub, > I think the __builtin_constant_p(acospi(0.5)) approach is usable, but would > be much better done on the lib/target-supports.exp side. > So, have foldable_pi_based_trigonometry effective target, which would test > if __builtin_constant_p(acospi(0.5)) is 1. Thanks again for your helpful advice. I've added the foldable_pi_based_trigonometry effective target and removed the conditional branch in the test case. The test results look good. Thanks, Yuao 0001-gcc-middle-end-opt-for-trigonometric-pi-based-functi.patch Description: 0001-gcc-middle-end-opt-for-trigonometric-pi-based-functi.patch
[PATCH] gcc: middle-end opt for trigonometric pi-based functions builtins
Hi Jakub, > Please don't include math.h here. Done. > And instead of this line use __builtin_acospi (0.5). > and, in dejagnu for runtime tests we prefer __builtin_abort on failure, so Done. Yuao 0001-gcc-middle-end-opt-for-trigonometric-pi-based-functi.patch Description: 0001-gcc-middle-end-opt-for-trigonometric-pi-based-functi.patch
[PATCH] gcc: middle-end opt for trigonometric pi-based functions builtins
Hi Jakub, > signbit is documented to be a macro, so please don't declare > int signbit (double); > function in the testcase and instead of signbit use __builtin_signbit. This is indeed my negligence. Done. If everything looks good, could you please help me merge this patch? Thank you! Yuao 0001-gcc-middle-end-opt-for-trigonometric-pi-based-functi.patch Description: 0001-gcc-middle-end-opt-for-trigonometric-pi-based-functi.patch
[PATCH] fortran: add constant input support for trig functions with half-revolutions
Hi Tobias, The patch has been updated. Could you please take a look when you have a chance? > Can you add the PR number to the commit log ("PR fortran/113152")? Done. > Can you also update the documentation – as you already did for ATAND? I think that's quite a lot of wording... Hoping to tackle that in another patch, if that's okay. > As now the builtins are available, how about updating mathbuiltins.def > already? Done. > As the comment states, it seems to make sense to put the new symbols at > the end. GCC generates for "module m" a module file (.mod) which is a > compress lisp-like file. That one also stores the intrinsic procedure's > ID – and it makes sense to be upward compatible by appending the new > ones at the end. Done. > Testcases: I think it makes sense to check – either in this patch or in > the follow up – that -std=f2018 gives a diagnostic that the intrinsic is > unsupported while -std=f2023 supports it. Done. When testing invalid input, I'm only testing the first half of the sentence. The original test case used `.*`, which defaults to a greedy match and caused issues. I attempted to use `.*?` for a non-greedy match, but it didn't resolve the problem. > But this change should be should also mentioned in the ChangeLog to > avoid going through the change letter by letter to understand whether > there was a significant change or it was just reformatted. Done. Yuao 0001-fortran-add-constant-input-support-for-trig-function.patch Description: 0001-fortran-add-constant-input-support-for-trig-function.patch
[Patch] Fortran: gfc_simplify_{cospi,sinpi} - fix for MPFR < 4.2.0 (was: [PATCH] fortran: add constant input support for trig functions with half-revolutions)
Hi Tobias and Harald, I sincerely apologize for the breakage! I did test both preprocessor branches, but I tested against the same MPFR version (4.2.2, which is the latest on Arch Linux). Going forward, I will test against versions both above and below 4.2.0 to ensure this type of breakage doesn't happen again. Best regards, Yuao
[PATCH] gcc: middle-end opt for trigonometric pi-based functions builtins
Hi Joseph, > I don't see tests for the various special cases from Annex F (for example, > "tanpi(n) returns +0, for positive even and negative odd integers n." and > "tanpi(n) returns -0, for positive odd and negative even integers n."). > In such cases the sign of zero would need to be checked specifically (via > copysign or signbit). Thanks for the suggestions! The tests are now added; please check the attached patch. > Similar considerations may apply to other functions as well - and would be > avoided by not trying to do the optimizations with older MPFR versions. While conditionally optimizing these built-in functions based on the available MPFR version seems beneficial, it poses a testing challenge. Specifically, test cases would need a mechanism to dynamically determine the MPFR version in use. Yuao 0001-gcc-middle-end-opt-for-trigonometric-pi-based-functi.patch Description: 0001-gcc-middle-end-opt-for-trigonometric-pi-based-functi.patch
[PATCH] gcc: middle-end opt for trigonometric pi-based functions builtins
Hello world, This patch partially handled PR118592. This patch builds upon r16-710-g591d3d02664c7b and r16-711-g89935d56f768b4. It introduces middle-end optimizations, such as constant folding, for our trigonometric pi-based function built-ins. For MPFR versions older than 4.2.0, we've included our own folding functions. I've also added tests to ensure everything works as expected. Best regards, Yuao 0001-gcc-middle-end-opt-for-trigonometric-pi-based-functi.patch Description: 0001-gcc-middle-end-opt-for-trigonometric-pi-based-functi.patch
[PATCH] gcc: remove atan from edom_only_function
Hi Richard, > OK. > > Thanks, > Richard. Thanks for the quick review! Could you please help me merge this patch? I'll post the rest of the original patch soon. Thanks, Yuao
[PATCH] gcc: remove atan from edom_only_function
Hi all, This patch addresses previous review feedback by splitting the atan handling into a separate patch. This patch is part of https://gcc.gnu.org/pipermail/fortran/attachments/20250607/4a4a9cb6/attachment.obj Please take a look when you are available. Thanks! Best regards, Yuao 0001-gcc-remove-atan-from-edom_only_function.patch Description: 0001-gcc-remove-atan-from-edom_only_function.patch
[PATCH] gcc: middle-end opt for trigonometric pi-based functions builtins
Hi Dave, > but the testcases don't seem to be conditionalized on this. Would the > new tests fail if gcc is built against an insufficiently recent version > of mpfr, and is/should there be some kind of dg-requires for this, so > that the new tests gracefully are "UNSUPPORTED" on such configurations? The test case is indeed conditionalized, though in a different manner than you might expect. The condition depends on the version of MPFR we're using, and unfortunately, I haven't found a predefined macro that indicates which MPFR version GCC is linked against. I tried `gcc -E -dM - < /dev/null`, but didn't find any relevant macros. My current approach uses `__builtin_constant_p(acospi(0.5))`. If we're using a newer MPFR version, acospi will be constant-folded, causing the condition to evaluate to true and enabling the rest of the test. Otherwise, the condition will be false, and the entire test case will be omitted. Do you see any other parts of the patch that require further revision? Thanks, Yuao
[PATCH] gcc: middle-end opt for trigonometric pi-based functions builtins
Hi all, This patch, a follow-up to r16-1652-g0606d2b979f401, implements middle-end optimizations (e.g., constant folding) for our trigonometric pi-based function built-ins. This patch is part of https://gcc.gnu.org/pipermail/fortran/attachments/20250607/4a4a9cb6/attachment.obj Please take a look when you are available. Thanks! Please disregard the previous email as I forgot to attach the patch... Best regards, Yuao 0001-gcc-middle-end-opt-for-trigonometric-pi-based-functi.patch Description: 0001-gcc-middle-end-opt-for-trigonometric-pi-based-functi.patch
[PATCH] gcc: middle-end opt for trigonometric pi-based functions builtins
Hi all, This patch, a follow-up to r16-1652-g0606d2b979f401, implements middle-end optimizations (e.g., constant folding) for our trigonometric pi-based function built-ins. This patch is part of https://gcc.gnu.org/pipermail/fortran/attachments/20250607/4a4a9cb6/attachment.obj Please take a look when you are available. Thanks! Best regards, Yuao
[PATCH] fortran: implment split for fortran 2023
Hi all, This patch introduces a complete implementation of the SPLIT intrinsic, including documentation and test cases. While Tobias previously mentioned that a similar effect could be achieved with existing functionality, I believe having direct support for this operation won't cause any issues. Users will still have the flexibility to choose their preferred method. Please take a look when you have a moment! Thanks, YuaoFrom 3fb261141dbc61296adb9c9361bafcd9922ad8bd Mon Sep 17 00:00:00 2001 From: Yuao Ma Date: Sat, 26 Jul 2025 20:59:26 +0800 Subject: [PATCH] fortran: implment split for fortran 2023 This patch includes the implementation, documentation, and test case for SPLIT. gcc/fortran/ChangeLog: * check.cc (gfc_check_split): Argument check for SPLIT. * gfortran.h (enum gfc_isym_id): Define GFC_ISYM_SPLIT. * intrinsic.cc (add_subroutines): Register SPLIT intrinsic. * intrinsic.h (gfc_check_split): New decl. (gfc_resolve_split): Ditto. * intrinsic.texi: SPLIT documentation. * iresolve.cc (gfc_resolve_split): Add resolved_sym for SPLIT. * trans-decl.cc (gfc_build_intrinsic_function_decls): Add decl for SPLIT in libgfortran. * trans-intrinsic.cc (conv_intrinsic_split): SPLIT codegen. (gfc_conv_intrinsic_subroutine): Handle SPLIT case. * trans.h (GTY): Declare gfor_fndecl_string_split{, _char4}. libgfortran/ChangeLog: * gfortran.map: Add split symbol. * intrinsics/string_intrinsics_inc.c (string_split): Runtime support for SPLIT. gcc/testsuite/ChangeLog: * gfortran.dg/split_1.f90: New test. * gfortran.dg/split_2.f90: New test. --- gcc/fortran/check.cc | 21 ++ gcc/fortran/gfortran.h| 2 + gcc/fortran/intrinsic.cc | 8 +++ gcc/fortran/intrinsic.h | 2 + gcc/fortran/intrinsic.texi| 64 + gcc/fortran/iresolve.cc | 13 gcc/fortran/trans-decl.cc | 14 gcc/fortran/trans-intrinsic.cc| 71 +++ gcc/fortran/trans.h | 2 + gcc/testsuite/gfortran.dg/split_1.f90 | 28 gcc/testsuite/gfortran.dg/split_2.f90 | 22 ++ libgfortran/gfortran.map | 2 + .../intrinsics/string_intrinsics_inc.c| 52 ++ 13 files changed, 301 insertions(+) create mode 100644 gcc/testsuite/gfortran.dg/split_1.f90 create mode 100644 gcc/testsuite/gfortran.dg/split_2.f90 diff --git a/gcc/fortran/check.cc b/gcc/fortran/check.cc index 838d523f7c4..862652683a7 100644 --- a/gcc/fortran/check.cc +++ b/gcc/fortran/check.cc @@ -5559,6 +5559,27 @@ gfc_check_scan (gfc_expr *x, gfc_expr *y, gfc_expr *z, gfc_expr *kind) return true; } +bool +gfc_check_split (gfc_expr *string, gfc_expr *set, gfc_expr *pos, gfc_expr *back) +{ + if (!type_check (string, 0, BT_CHARACTER)) +return false; + + if (!type_check (set, 1, BT_CHARACTER)) +return false; + + if (!type_check (pos, 2, BT_INTEGER) || !scalar_check (pos, 2)) +return false; + + if (back != NULL + && (!type_check (back, 3, BT_LOGICAL) || !scalar_check (back, 3))) +return false; + + if (!same_type_check (string, 0, set, 1)) +return false; + + return true; +} bool gfc_check_secnds (gfc_expr *r) diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h index 85feb18be8c..d9dcd1b504f 100644 --- a/gcc/fortran/gfortran.h +++ b/gcc/fortran/gfortran.h @@ -729,6 +729,8 @@ enum gfc_isym_id GFC_ISYM_COSPI, GFC_ISYM_SINPI, GFC_ISYM_TANPI, + + GFC_ISYM_SPLIT, }; enum init_local_logical diff --git a/gcc/fortran/intrinsic.cc b/gcc/fortran/intrinsic.cc index 9e07627503d..259b59edc3b 100644 --- a/gcc/fortran/intrinsic.cc +++ b/gcc/fortran/intrinsic.cc @@ -3933,6 +3933,14 @@ add_subroutines (void) pt, BT_INTEGER, di, OPTIONAL, INTENT_IN, gt, BT_INTEGER, di, OPTIONAL, INTENT_OUT); + add_sym_4s ("split", GFC_ISYM_SPLIT, CLASS_IMPURE, + BT_UNKNOWN, 0, GFC_STD_F2023, + gfc_check_split, NULL, gfc_resolve_split, + "string", BT_CHARACTER, dc, REQUIRED, INTENT_IN, + "set", BT_CHARACTER, dc, REQUIRED, INTENT_IN, + "pos", BT_INTEGER, di, REQUIRED, INTENT_INOUT, + "back", BT_LOGICAL, dl, OPTIONAL, INTENT_IN); + /* The following subroutines are part of ISO_C_BINDING. */ add_sym_3s ("c_f_pointer", GFC_ISYM_C_F_POINTER, CLASS_IMPURE, BT_UNKNOWN, 0, diff --git a/gcc/fortran/intrinsic.h b/gcc/fortran/intrinsic.h index fd54588054f..8a0ab935e1f 100644 --- a/gcc/fortran/intrinsic.h +++ b/gcc/fortran/intrinsic.h @@ -215,6 +215,7 @@ bool gfc_check_mvbits (gfc_expr *, gfc_expr *, gfc_expr *, gfc_expr *, bool gfc_check_random_init (g
Re: [PATCH] fortran: implment split for fortran 2023
Hi Steve, On 7/26/2025 11:25 PM, Steve Kargl wrote: On Sat, Jul 26, 2025 at 09:14:43PM +0800, Yuao Ma wrote: + add_sym_4s ("split", GFC_ISYM_SPLIT, CLASS_IMPURE, + BT_UNKNOWN, 0, GFC_STD_F2023, + gfc_check_split, NULL, gfc_resolve_split, + "string", BT_CHARACTER, dc, REQUIRED, INTENT_IN, + "set", BT_CHARACTER, dc, REQUIRED, INTENT_IN, + "pos", BT_INTEGER, di, REQUIRED, INTENT_INOUT, + "back", BT_LOGICAL, dl, OPTIONAL, INTENT_IN); + This looks incorrect. SPLIT is a simple subroutine. (Yes, I know gfortran does not have the simple concept, yet). See Fortran 2023, 15.8: A simple procedure is also a pure procedure and is subject to the constraints for pure procedures (15.7). A simple procedure can also be an elemental procedure. Shouldn't you have CLASS_PURE? That was indeed my oversight. Initially, I believed split had an argument with INTENT_INOUT, which suggested a side effect. However, I then discovered move_alloc also has an INOUT argument, but with the CLASS_PURE type. This has been corrected in the new patch. The rest of the patch looks Ok. Thanks for the review! If the new patch looks good for trunk and there are no other objections, could you please help me merge it? Thanks, YuaoFrom 177549017521d4c5215ff0e18fdb3492c31796a4 Mon Sep 17 00:00:00 2001 From: Yuao Ma Date: Sat, 26 Jul 2025 23:45:55 +0800 Subject: [PATCH] fortran: implment split for fortran 2023 This patch includes the implementation, documentation, and test case for SPLIT. gcc/fortran/ChangeLog: * check.cc (gfc_check_split): Argument check for SPLIT. * gfortran.h (enum gfc_isym_id): Define GFC_ISYM_SPLIT. * intrinsic.cc (add_subroutines): Register SPLIT intrinsic. * intrinsic.h (gfc_check_split): New decl. (gfc_resolve_split): Ditto. * intrinsic.texi: SPLIT documentation. * iresolve.cc (gfc_resolve_split): Add resolved_sym for SPLIT. * trans-decl.cc (gfc_build_intrinsic_function_decls): Add decl for SPLIT in libgfortran. * trans-intrinsic.cc (conv_intrinsic_split): SPLIT codegen. (gfc_conv_intrinsic_subroutine): Handle SPLIT case. * trans.h (GTY): Declare gfor_fndecl_string_split{, _char4}. libgfortran/ChangeLog: * gfortran.map: Add split symbol. * intrinsics/string_intrinsics_inc.c (string_split): Runtime support for SPLIT. gcc/testsuite/ChangeLog: * gfortran.dg/split_1.f90: New test. * gfortran.dg/split_2.f90: New test. Signed-off-by: Yuao Ma --- gcc/fortran/check.cc | 21 ++ gcc/fortran/gfortran.h| 2 + gcc/fortran/intrinsic.cc | 8 +++ gcc/fortran/intrinsic.h | 2 + gcc/fortran/intrinsic.texi| 64 + gcc/fortran/iresolve.cc | 13 gcc/fortran/trans-decl.cc | 14 gcc/fortran/trans-intrinsic.cc| 71 +++ gcc/fortran/trans.h | 2 + gcc/testsuite/gfortran.dg/split_1.f90 | 28 gcc/testsuite/gfortran.dg/split_2.f90 | 22 ++ libgfortran/gfortran.map | 2 + .../intrinsics/string_intrinsics_inc.c| 52 ++ 13 files changed, 301 insertions(+) create mode 100644 gcc/testsuite/gfortran.dg/split_1.f90 create mode 100644 gcc/testsuite/gfortran.dg/split_2.f90 diff --git a/gcc/fortran/check.cc b/gcc/fortran/check.cc index 838d523f7c4..862652683a7 100644 --- a/gcc/fortran/check.cc +++ b/gcc/fortran/check.cc @@ -5559,6 +5559,27 @@ gfc_check_scan (gfc_expr *x, gfc_expr *y, gfc_expr *z, gfc_expr *kind) return true; } +bool +gfc_check_split (gfc_expr *string, gfc_expr *set, gfc_expr *pos, gfc_expr *back) +{ + if (!type_check (string, 0, BT_CHARACTER)) +return false; + + if (!type_check (set, 1, BT_CHARACTER)) +return false; + + if (!type_check (pos, 2, BT_INTEGER) || !scalar_check (pos, 2)) +return false; + + if (back != NULL + && (!type_check (back, 3, BT_LOGICAL) || !scalar_check (back, 3))) +return false; + + if (!same_type_check (string, 0, set, 1)) +return false; + + return true; +} bool gfc_check_secnds (gfc_expr *r) diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h index 85feb18be8c..d9dcd1b504f 100644 --- a/gcc/fortran/gfortran.h +++ b/gcc/fortran/gfortran.h @@ -729,6 +729,8 @@ enum gfc_isym_id GFC_ISYM_COSPI, GFC_ISYM_SINPI, GFC_ISYM_TANPI, + + GFC_ISYM_SPLIT, }; enum init_local_logical diff --git a/gcc/fortran/intrinsic.cc b/gcc/fortran/intrinsic.cc index 9e07627503d..c99a7a86aea 100644 --- a/gcc/fortran/intrinsic.cc +++ b/gcc/fortran/intrinsic.cc @@ -3933,6 +3933,14 @@ add_subroutines (void) pt, BT_INTEGER, di, OPTIONAL, INTENT_IN,
Re: [PATCH] fortran: implment split for fortran 2023
("If BACK is present with the value false, the value of " + "POS shall be in the range [0, LEN (STRING)]"); + The condition doesn't check pos >= 0; I think the case pos < 0 doesn't work. The variable pos is an unsigned type, so checking for < 0 is unnecessary. (I was not initially aware of this, but the compiler warning alerted me.) Obviously there are tests missing that check the runtime error. Done. + for (i = pos + 1; i <= stringlen; i++) + { + for (j = 0; j < setlen; j++) + { + if (string[i - 1] == set[j]) + { + return i; + } Style issue: No brace for a single statement. Done. + } + } + + return stringlen + 1; + } + else + { + if (pos < 1 || pos > (stringlen + 1)) + runtime_error ("If BACK is present with the value true, the value of " + "POS shall be in the range [1, LEN (STRING) + 1]"); + + for (i = pos - 1; i != 0; i--) + { + for (j = 0; j < setlen; j++) + { + if (string[i - 1] == set[j]) + { + return i; + } Same here. Done. Thanks, YuaoFrom 06bd5eff1306a82f35e7a9f9ba0db7f1af415f98 Mon Sep 17 00:00:00 2001 From: Yuao Ma Date: Sun, 27 Jul 2025 11:32:12 +0800 Subject: [PATCH] fortran: implment split for fortran 2023 This patch includes the implementation, documentation, and test case for SPLIT. gcc/fortran/ChangeLog: * check.cc (gfc_check_split): Argument check for SPLIT. * gfortran.h (enum gfc_isym_id): Define GFC_ISYM_SPLIT. * intrinsic.cc (add_subroutines): Register SPLIT intrinsic. * intrinsic.h (gfc_check_split): New decl. (gfc_resolve_split): Ditto. * intrinsic.texi: SPLIT documentation. * iresolve.cc (gfc_resolve_split): Add resolved_sym for SPLIT. * trans-decl.cc (gfc_build_intrinsic_function_decls): Add decl for SPLIT in libgfortran. * trans-intrinsic.cc (conv_intrinsic_split): SPLIT codegen. (gfc_conv_intrinsic_subroutine): Handle SPLIT case. * trans.h (GTY): Declare gfor_fndecl_string_split{, _char4}. libgfortran/ChangeLog: * gfortran.map: Add split symbol. * intrinsics/string_intrinsics_inc.c (string_split): Runtime support for SPLIT. gcc/testsuite/ChangeLog: * gfortran.dg/split_1.f90: New test. * gfortran.dg/split_2.f90: New test. * gfortran.dg/split_3.f90: New test. * gfortran.dg/split_4.f90: New test. Signed-off-by: Yuao Ma --- gcc/fortran/check.cc | 21 ++ gcc/fortran/gfortran.h| 2 + gcc/fortran/intrinsic.cc | 8 ++ gcc/fortran/intrinsic.h | 2 + gcc/fortran/intrinsic.texi| 64 gcc/fortran/iresolve.cc | 13 gcc/fortran/trans-decl.cc | 14 gcc/fortran/trans-intrinsic.cc| 73 +++ gcc/fortran/trans.h | 2 + gcc/testsuite/gfortran.dg/split_1.f90 | 28 +++ gcc/testsuite/gfortran.dg/split_2.f90 | 22 ++ gcc/testsuite/gfortran.dg/split_3.f90 | 11 +++ gcc/testsuite/gfortran.dg/split_4.f90 | 11 +++ libgfortran/gfortran.map | 6 ++ .../intrinsics/string_intrinsics_inc.c| 48 15 files changed, 325 insertions(+) create mode 100644 gcc/testsuite/gfortran.dg/split_1.f90 create mode 100644 gcc/testsuite/gfortran.dg/split_2.f90 create mode 100644 gcc/testsuite/gfortran.dg/split_3.f90 create mode 100644 gcc/testsuite/gfortran.dg/split_4.f90 diff --git a/gcc/fortran/check.cc b/gcc/fortran/check.cc index 838d523f7c4..862652683a7 100644 --- a/gcc/fortran/check.cc +++ b/gcc/fortran/check.cc @@ -5559,6 +5559,27 @@ gfc_check_scan (gfc_expr *x, gfc_expr *y, gfc_expr *z, gfc_expr *kind) return true; } +bool +gfc_check_split (gfc_expr *string, gfc_expr *set, gfc_expr *pos, gfc_expr *back) +{ + if (!type_check (string, 0, BT_CHARACTER)) +return false; + + if (!type_check (set, 1, BT_CHARACTER)) +return false; + + if (!type_check (pos, 2, BT_INTEGER) || !scalar_check (pos, 2)) +return false; + + if (back != NULL + && (!type_check (back, 3, BT_LOGICAL) || !scalar_check (back, 3))) +return false; + + if (!same_type_check (string, 0, set, 1)) +return false; + + return true; +} bool gfc_check_secnds (gfc_expr *r) diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h index 85feb18be8c..d9dcd1b504f 100644 --- a/gcc/fortran/gfortran.h +++ b/gcc/fortran/gfortran.h @@ -729,6 +729,8 @@ enum gfc_isym_id GFC_ISYM_COSPI, GFC_ISYM_SINPI, GFC_ISYM_TANPI, + + GFC_ISYM_SPLIT, }; enum init_local_logical diff --git a/gcc/fortran/intrinsic.cc b/gcc/fortran/intrinsic.cc index 9e07627503d..c99a7a86aea 100644 -
Re: [PATCH] fortran: implment split for fortran 2023
On 7/27/2025 5:19 PM, Mikael Morin wrote: +gfc_charlen_type +string_split (gfc_charlen_type stringlen, const CHARTYPE *string, + gfc_charlen_type setlen, const CHARTYPE *set, + gfc_charlen_type pos, GFC_LOGICAL_4 back) +{ + gfc_charlen_type i, j; + + if (!back) + { + if (pos > stringlen) + runtime_error ("If BACK is present with the value false, the value of " + "POS shall be in the range [0, LEN (STRING)]"); + The condition doesn't check pos >= 0; I think the case pos < 0 doesn't work. The variable pos is an unsigned type, so checking for < 0 is unnecessary. (I was not initially aware of this, but the compiler warning alerted me.) Mmmh, good point. But POS is a user variable, so a signed type originally. Can you check the following: integer(kind=1) :: my_pos = -1 character(len=300) :: my_str = "" call split(my_str, " ", my_pos) Is it rejected at runtime? I expect the unsigned conversion to convert -1 to 255 and then the call would be (wrongly) accepted. Yes, it's rejected at runtime. The trick lies in how the pos is handled: it's first converted to gfc_charlen_type (an unsigned long long). So, even if kind=1 and the input is -1, it wraps around to a very large positive value (specifically, ULLONG_MAX - 1). This value far exceeds 300, leading to a runtime error. + pos_for_call = fold_convert (gfc_charlen_type_node, pos); Thanks, Yuao
Re: [PATCH] fortran: implment split for fortran 2023
On 7/27/2025 5:43 PM, Mikael Morin wrote: I forgot to add: + else + { + if (pos < 1 || pos > (stringlen + 1)) + runtime_error ("If BACK is present with the value true, the value of " + "POS shall be in the range [1, LEN (STRING) + 1]"); + can you provide the value of pos and stringlen in the error messages? Done.From d1c8a89d49f9b66bb9a434f0058180b21061df42 Mon Sep 17 00:00:00 2001 From: Yuao Ma Date: Sun, 27 Jul 2025 18:12:53 +0800 Subject: [PATCH] fortran: implment split for fortran 2023 This patch includes the implementation, documentation, and test case for SPLIT. gcc/fortran/ChangeLog: * check.cc (gfc_check_split): Argument check for SPLIT. * gfortran.h (enum gfc_isym_id): Define GFC_ISYM_SPLIT. * intrinsic.cc (add_subroutines): Register SPLIT intrinsic. * intrinsic.h (gfc_check_split): New decl. (gfc_resolve_split): Ditto. * intrinsic.texi: SPLIT documentation. * iresolve.cc (gfc_resolve_split): Add resolved_sym for SPLIT. * trans-decl.cc (gfc_build_intrinsic_function_decls): Add decl for SPLIT in libgfortran. * trans-intrinsic.cc (conv_intrinsic_split): SPLIT codegen. (gfc_conv_intrinsic_subroutine): Handle SPLIT case. * trans.h (GTY): Declare gfor_fndecl_string_split{, _char4}. libgfortran/ChangeLog: * gfortran.map: Add split symbol. * intrinsics/string_intrinsics_inc.c (string_split): Runtime support for SPLIT. gcc/testsuite/ChangeLog: * gfortran.dg/split_1.f90: New test. * gfortran.dg/split_2.f90: New test. * gfortran.dg/split_3.f90: New test. * gfortran.dg/split_4.f90: New test. Signed-off-by: Yuao Ma --- gcc/fortran/check.cc | 21 ++ gcc/fortran/gfortran.h| 2 + gcc/fortran/intrinsic.cc | 8 ++ gcc/fortran/intrinsic.h | 2 + gcc/fortran/intrinsic.texi| 64 gcc/fortran/iresolve.cc | 13 gcc/fortran/trans-decl.cc | 14 gcc/fortran/trans-intrinsic.cc| 73 +++ gcc/fortran/trans.h | 2 + gcc/testsuite/gfortran.dg/split_1.f90 | 28 +++ gcc/testsuite/gfortran.dg/split_2.f90 | 22 ++ gcc/testsuite/gfortran.dg/split_3.f90 | 11 +++ gcc/testsuite/gfortran.dg/split_4.f90 | 11 +++ libgfortran/gfortran.map | 6 ++ .../intrinsics/string_intrinsics_inc.c| 52 + 15 files changed, 329 insertions(+) create mode 100644 gcc/testsuite/gfortran.dg/split_1.f90 create mode 100644 gcc/testsuite/gfortran.dg/split_2.f90 create mode 100644 gcc/testsuite/gfortran.dg/split_3.f90 create mode 100644 gcc/testsuite/gfortran.dg/split_4.f90 diff --git a/gcc/fortran/check.cc b/gcc/fortran/check.cc index 838d523f7c4..862652683a7 100644 --- a/gcc/fortran/check.cc +++ b/gcc/fortran/check.cc @@ -5559,6 +5559,27 @@ gfc_check_scan (gfc_expr *x, gfc_expr *y, gfc_expr *z, gfc_expr *kind) return true; } +bool +gfc_check_split (gfc_expr *string, gfc_expr *set, gfc_expr *pos, gfc_expr *back) +{ + if (!type_check (string, 0, BT_CHARACTER)) +return false; + + if (!type_check (set, 1, BT_CHARACTER)) +return false; + + if (!type_check (pos, 2, BT_INTEGER) || !scalar_check (pos, 2)) +return false; + + if (back != NULL + && (!type_check (back, 3, BT_LOGICAL) || !scalar_check (back, 3))) +return false; + + if (!same_type_check (string, 0, set, 1)) +return false; + + return true; +} bool gfc_check_secnds (gfc_expr *r) diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h index 85feb18be8c..d9dcd1b504f 100644 --- a/gcc/fortran/gfortran.h +++ b/gcc/fortran/gfortran.h @@ -729,6 +729,8 @@ enum gfc_isym_id GFC_ISYM_COSPI, GFC_ISYM_SINPI, GFC_ISYM_TANPI, + + GFC_ISYM_SPLIT, }; enum init_local_logical diff --git a/gcc/fortran/intrinsic.cc b/gcc/fortran/intrinsic.cc index 9e07627503d..c99a7a86aea 100644 --- a/gcc/fortran/intrinsic.cc +++ b/gcc/fortran/intrinsic.cc @@ -3933,6 +3933,14 @@ add_subroutines (void) pt, BT_INTEGER, di, OPTIONAL, INTENT_IN, gt, BT_INTEGER, di, OPTIONAL, INTENT_OUT); + add_sym_4s ("split", GFC_ISYM_SPLIT, CLASS_PURE, + BT_UNKNOWN, 0, GFC_STD_F2023, + gfc_check_split, NULL, gfc_resolve_split, + "string", BT_CHARACTER, dc, REQUIRED, INTENT_IN, + "set", BT_CHARACTER, dc, REQUIRED, INTENT_IN, + "pos", BT_INTEGER, di, REQUIRED, INTENT_INOUT, + "back", BT_LOGICAL, dl, OPTIONAL, INTENT_IN); + /* The following subroutines are part of ISO_C_BINDING. */ add_sym_3s ("c_f_pointer", GFC_ISYM_C_F_POINTER, CLASS_IMPURE, BT_UNKNOWN, 0, diff --git a/gcc
Re: [PATCH] fortran: implment split for fortran 2023
On 7/27/2025 8:51 PM, Mikael Morin wrote: Le 27/07/2025 à 13:46, Yuao Ma a écrit : On 7/27/2025 7:14 PM, Mikael Morin wrote: Le 27/07/2025 à 11:37, Yuao Ma a écrit : On 7/27/2025 5:19 PM, Mikael Morin wrote: +gfc_charlen_type +string_split (gfc_charlen_type stringlen, const CHARTYPE *string, + gfc_charlen_type setlen, const CHARTYPE *set, + gfc_charlen_type pos, GFC_LOGICAL_4 back) +{ + gfc_charlen_type i, j; + + if (!back) + { + if (pos > stringlen) + runtime_error ("If BACK is present with the value false, the value of " + "POS shall be in the range [0, LEN (STRING)]"); + The condition doesn't check pos >= 0; I think the case pos < 0 doesn't work. The variable pos is an unsigned type, so checking for < 0 is unnecessary. (I was not initially aware of this, but the compiler warning alerted me.) Mmmh, good point. But POS is a user variable, so a signed type originally. Can you check the following: integer(kind=1) :: my_pos = -1 character(len=300) :: my_str = "" call split(my_str, " ", my_pos) Is it rejected at runtime? I expect the unsigned conversion to convert -1 to 255 and then the call would be (wrongly) accepted. Yes, it's rejected at runtime. The trick lies in how the pos is handled: it's first converted to gfc_charlen_type (an unsigned long long). So, even if kind=1 and the input is -1, it wraps around to a very large positive value (specifically, ULLONG_MAX - 1). This value far exceeds 300, leading to a runtime error. + pos_for_call = fold_convert (gfc_charlen_type_node, pos); Ok, according to the dump the conversion is to integer(kind=8) which is signed. Anyway, I think it works for any sensible case. So the case above is rejected, but now the value of pos printed is not what the user specified. This is what I get: Fortran runtime error: If BACK is present with the value false, the value of POS shall be in the range [0, LEN (STRING)], where POS = 18446744073709551615 and LEN (STRING) = 300 Good point. Since the conversion is always to integer(kind=8), we can simply use %ld instead of %lu as the format argument. Fixed. Another thing I noticed: + gfc_add_expr_to_block (&block, call); + gfc_add_modify (&block, pos, fold_convert (TREE_TYPE (pos), call)); You are using the call twice; this results in two calls to the function split in the dump: _gfortran_string_split (300, &my_str, 1, &" "[1]{lb: 1 sz: 1}, (integer(kind=8)) my_pos, 0); my_pos = (integer(kind=1)) _gfortran_string_split (300, &my_str, 1, &" "[1]{lb: 1 sz: 1}, (integer(kind=8)) my_pos, 0); The gfc_add_expr_to_block call appears to be redundant. Fixed. I'm fine with this version. I'll leave one more day for someone else to comment further and then I'll push it for you. Thanks for the patch. Thanks for the detailed review! I'll keep working on other Fortran 2023 features as part of my GSoC project. Best regards, Yuao
Re: [PATCH] fortran: implement conditional expression for fortran 2023
Hi Paul, On 7/31/2025 6:02 AM, Paul Richard Thomas wrote: That's exactly how I had a mind to do it. You beat me to it :-( Just get on, polish the patch and add more tests. I've updated the patch with improvements to both the functionality and test cases. It should now work well for simple scalar types. However, I've found that the issue is more complex than I initially thought. The current implementation causes an ICE with character and array types. It seems that we need to handle EXPR_CONDITIONAL every time we switch against an expr's type, and a quick search shows that there are many instances of this. I'm wondering if we could create separate, incremental patches for this. For example, in this patch, we could deny other complex types in the resolution process and gradually relax that constraint in future patches. I also look forward to your comments on the patch itself. Best regards, Yuaodiff --git a/gcc/fortran/dump-parse-tree.cc b/gcc/fortran/dump-parse-tree.cc index 3cd2eeef11a..eda0659d6e2 100644 --- a/gcc/fortran/dump-parse-tree.cc +++ b/gcc/fortran/dump-parse-tree.cc @@ -767,6 +767,16 @@ show_expr (gfc_expr *p) break; +case EXPR_CONDITIONAL: + fputc ('(', dumpfile); + show_expr (p->value.conditional.condition); + fputs (" ? ", dumpfile); + show_expr (p->value.conditional.true_expr); + fputs (" : ", dumpfile); + show_expr (p->value.conditional.false_expr); + fputc (')', dumpfile); + break; + case EXPR_COMPCALL: show_compcall (p); break; diff --git a/gcc/fortran/expr.cc b/gcc/fortran/expr.cc index b8d04ff6f36..ed6d9adf5a6 100644 --- a/gcc/fortran/expr.cc +++ b/gcc/fortran/expr.cc @@ -116,6 +116,25 @@ gfc_get_operator_expr (locus *where, gfc_intrinsic_op op, return e; } +/* Get a new expression node that is an conditional expression node. */ + +gfc_expr * +gfc_get_conditional_expr (locus *where, gfc_expr *condition, + gfc_expr *true_expr, gfc_expr *false_expr) +{ + gfc_expr *e; + + e = gfc_get_expr (); + e->expr_type = EXPR_CONDITIONAL; + e->value.conditional.condition = condition; + e->value.conditional.true_expr = true_expr; + e->value.conditional.false_expr = false_expr; + + if (where) +e->where = *where; + + return e; +} /* Get a new expression node that is an structure constructor of given type and kind. */ @@ -393,6 +412,15 @@ gfc_copy_expr (gfc_expr *p) break; +case EXPR_CONDITIONAL: + q->value.conditional.condition + = gfc_copy_expr (p->value.conditional.condition); + q->value.conditional.true_expr + = gfc_copy_expr (p->value.conditional.true_expr); + q->value.conditional.false_expr + = gfc_copy_expr (p->value.conditional.false_expr); + break; + case EXPR_FUNCTION: q->value.function.actual = gfc_copy_actual_arglist (p->value.function.actual); @@ -502,6 +530,12 @@ free_expr0 (gfc_expr *e) gfc_free_expr (e->value.op.op2); break; +case EXPR_CONDITIONAL: + gfc_free_expr (e->value.conditional.condition); + gfc_free_expr (e->value.conditional.true_expr); + gfc_free_expr (e->value.conditional.false_expr); + break; + case EXPR_FUNCTION: gfc_free_actual_arglist (e->value.function.actual); break; @@ -1083,6 +1117,11 @@ gfc_is_constant_expr (gfc_expr *e) && (e->value.op.op2 == NULL || gfc_is_constant_expr (e->value.op.op2))); +case EXPR_CONDITIONAL: + return gfc_is_constant_expr (e->value.conditional.condition) +&& gfc_is_constant_expr (e->value.conditional.true_expr) +&& gfc_is_constant_expr (e->value.conditional.false_expr); + case EXPR_VARIABLE: /* The only context in which this can occur is in a parameterized derived type declaration, so returning true is OK. */ @@ -1354,6 +1393,36 @@ simplify_intrinsic_op (gfc_expr *p, int type) return true; } +/* Try to collapse conditional expressions. */ + +static bool +simplify_conditional (gfc_expr *p, int type) +{ + gfc_expr *condition, *true_expr, *false_expr; + + condition = p->value.conditional.condition; + true_expr = p->value.conditional.true_expr; + false_expr = p->value.conditional.false_expr; + + if (!gfc_simplify_expr (condition, type) + || !gfc_simplify_expr (true_expr, type) + || !gfc_simplify_expr (false_expr, type)) +return false; + + if (!gfc_is_constant_expr (condition)) +return true; + + p->value.conditional.condition = NULL; + p->value.conditional.true_expr = NULL; + p->value.conditional.false_expr = NULL; + + if (condition->value.logical) +gfc_replace_expr (p, true_expr); + else +gfc_replace_expr (p, false_expr); + + return true; +} /* Subroutine to simplify constructor expressions. Mutually recursive with gfc_simplify_expr(). */ @@ -2459,6 +2528,11 @@ gfc_simplify_expr (gfc_expr *p, int type) return false; break;
Re: [PATCH] libgfortran: add fallback for trigonometric pi-based functions
On 7/31/2025 1:45 AM, Joseph Myers wrote: On Sun, 6 Jul 2025, Yuao Ma wrote: +#ifndef HAVE_COSPI +#define HAVE_COSPI 1 +double cospi (double); + +double +cospi (double x) +{ + return cos (x * pihi_d + x * pilo_d); For reasonable results for large x you should first reduce mod 2 to the range [-1, 1] (or reduce mod 1 and keep track of the parity of the integer you subtracted, etc.), before multiplying. And for reasonable results for x near a half-integer, you need to adjust to calling sin in such a case; cospi (0.5) should be exactly 0, which you won't get from calling cos on an argument close to pi/2. See the type-generic templates in glibc for example logic. Similar issues apply to all of cospi, sinpi, tanpi, for all floating-point formats. Thanks for the suggestions! I'll reference the glibc implementation and update the patch accordingly. By the way, could you please take another look at the libquadmath update? https://inbox.sourceware.org/fortran/kl1pr0601mb4291e1457dc09fe3aa6652c884...@kl1pr0601mb4291.apcprd06.prod.outlook.com/ This update uses a script to transform glibc's implementation for libquadmath. I'm wondering if the current implementation is good enough for inclusion in GCC or if we need to address any potential precision issues in glibc first. Thanks, Yuao
[PATCH] fortran: cleanup duplicate tests for c_f_pointer_shape_driver; nfc
Hi all, This patch cleans up a duplicate test driver. Regression-tested. OK for trunk? Thanks, YuaoFrom f4e821d346652ff4f9ac1a934a6afd019f13e026 Mon Sep 17 00:00:00 2001 From: Yuao Ma Date: Mon, 4 Aug 2025 20:19:27 +0800 Subject: [PATCH] fortran: cleanup duplicate tests for c_f_pointer_shape_driver; nfc tests_2_driver and tests_4_driver are identical, and tests_4_driver is not used at all. This patch clean this up, and format the driver with gcc style. gcc/testsuite/ChangeLog: * gfortran.dg/c_f_pointer_shape_tests_2.f03: Use the new driver. * gfortran.dg/c_f_pointer_shape_tests_4.f03: Ditto. * gfortran.dg/c_f_pointer_shape_tests_2_driver.c: Removed. * gfortran.dg/c_f_pointer_shape_tests_4_driver.c: Removed. * gfortran.dg/c_f_pointer_shape_tests_driver.c: New test. --- .../gfortran.dg/c_f_pointer_shape_tests_2.f03 | 2 +- .../c_f_pointer_shape_tests_2_driver.c| 46 -- .../gfortran.dg/c_f_pointer_shape_tests_4.f03 | 2 +- .../c_f_pointer_shape_tests_4_driver.c| 46 -- .../c_f_pointer_shape_tests_driver.c | 47 +++ 5 files changed, 49 insertions(+), 94 deletions(-) delete mode 100644 gcc/testsuite/gfortran.dg/c_f_pointer_shape_tests_2_driver.c delete mode 100644 gcc/testsuite/gfortran.dg/c_f_pointer_shape_tests_4_driver.c create mode 100644 gcc/testsuite/gfortran.dg/c_f_pointer_shape_tests_driver.c diff --git a/gcc/testsuite/gfortran.dg/c_f_pointer_shape_tests_2.f03 b/gcc/testsuite/gfortran.dg/c_f_pointer_shape_tests_2.f03 index 79cf2c1bae3..da20835891f 100644 --- a/gcc/testsuite/gfortran.dg/c_f_pointer_shape_tests_2.f03 +++ b/gcc/testsuite/gfortran.dg/c_f_pointer_shape_tests_2.f03 @@ -1,5 +1,5 @@ ! { dg-do run } -! { dg-additional-sources c_f_pointer_shape_tests_2_driver.c } +! { dg-additional-sources c_f_pointer_shape_tests_driver.c } ! Verify that the optional SHAPE parameter to c_f_pointer can be of any ! valid integer kind. We don't test all kinds here since it would be ! difficult to know what kinds are valid for the architecture we're running on. diff --git a/gcc/testsuite/gfortran.dg/c_f_pointer_shape_tests_2_driver.c b/gcc/testsuite/gfortran.dg/c_f_pointer_shape_tests_2_driver.c deleted file mode 100644 index 1282beb12d7..000 --- a/gcc/testsuite/gfortran.dg/c_f_pointer_shape_tests_2_driver.c +++ /dev/null @@ -1,46 +0,0 @@ -#define NUM_ELEMS 10 -#define NUM_ROWS 2 -#define NUM_COLS 3 - -void test_long_long_1d(int *array, int num_elems); -void test_long_long_2d(int *array, int num_rows, int num_cols); -void test_long_1d(int *array, int num_elems); -void test_int_1d(int *array, int num_elems); -void test_short_1d(int *array, int num_elems); -void test_mixed(int *array, int num_elems); - -int main(int argc, char **argv) -{ - int my_array[NUM_ELEMS]; - int my_2d_array[NUM_ROWS][NUM_COLS]; - int i, j; - - for(i = 0; i < NUM_ELEMS; i++) -my_array[i] = i; - - for(i = 0; i < NUM_ROWS; i++) -for(j = 0; j < NUM_COLS; j++) - my_2d_array[i][j] = (i*NUM_COLS) + j; - - /* Test c_f_pointer where SHAPE is of type integer, kind=c_long_long. */ - test_long_long_1d(my_array, NUM_ELEMS); - - /* Test c_f_pointer where SHAPE is of type integer, kind=c_long_long. - The indices are transposed for Fortran. */ - test_long_long_2d(my_2d_array[0], NUM_COLS, NUM_ROWS); - - /* Test c_f_pointer where SHAPE is of type integer, kind=c_long. */ - test_long_1d(my_array, NUM_ELEMS); - - /* Test c_f_pointer where SHAPE is of type integer, kind=c_int. */ - test_int_1d(my_array, NUM_ELEMS); - - /* Test c_f_pointer where SHAPE is of type integer, kind=c_short. */ - test_short_1d(my_array, NUM_ELEMS); - - /* Test c_f_pointer where SHAPE is of type integer, kind=c_int and - kind=c_long_long. */ - test_mixed(my_array, NUM_ELEMS); - - return 0; -} diff --git a/gcc/testsuite/gfortran.dg/c_f_pointer_shape_tests_4.f03 b/gcc/testsuite/gfortran.dg/c_f_pointer_shape_tests_4.f03 index 3f60f17e4bb..519087a2db6 100644 --- a/gcc/testsuite/gfortran.dg/c_f_pointer_shape_tests_4.f03 +++ b/gcc/testsuite/gfortran.dg/c_f_pointer_shape_tests_4.f03 @@ -1,5 +1,5 @@ ! { dg-do run } -! { dg-additional-sources c_f_pointer_shape_tests_2_driver.c } +! { dg-additional-sources c_f_pointer_shape_tests_driver.c } ! Verify that the optional SHAPE parameter to c_f_pointer can be of any ! valid integer kind. We don't test all kinds here since it would be ! difficult to know what kinds are valid for the architecture we're running on. diff --git a/gcc/testsuite/gfortran.dg/c_f_pointer_shape_tests_4_driver.c b/gcc/testsuite/gfortran.dg/c_f_pointer_shape_tests_4_driver.c deleted file mode 100644 index 1282beb12d7..000 --- a/gcc/testsuite/gfortran.dg/c_f_pointer_shape_tests_4_driver.c +++ /dev/null @@ -1,46 +0,0 @@ -#define NUM_ELEMS 10 -#define NUM_ROWS 2 -#define NUM_COLS 3 - -void test_long_long_1d(int *array, int num_elems); -void test
Re: [PATCH] libgfortran: add fallback for trigonometric pi-based functions
On 8/6/2025 5:05 AM, Joseph Myers wrote: On Tue, 5 Aug 2025, Yuao Ma wrote: On 8/5/2025 4:05 AM, Joseph Myers wrote: On Sun, 3 Aug 2025, Yuao Ma wrote: By the way, could you please take another look at the libquadmath update? https://inbox.sourceware.org/fortran/kl1pr0601mb4291e1457dc09fe3aa6652c884...@kl1pr0601mb4291.apcprd06.prod.outlook.com/ Using the script is what's expected for such a libquadmath update. Thanks for the clarification! I'm wondering if this patch needs more refinement or if it's ready to go. The seven trig-pi functions are needed to implement a fallback for trig-pi functions in gfortran. We can easily update them whenever glibc has an update. I think it's ready to go. If so, could you help me push this since I currently do not have write after approval access? I have locally checked that this patch does not conflict with the current master branch. Thanks in advance, Yuao
[PATCH] fortran: add optional lower arg to c_f_pointer
Hi all, This patch implements the optional lower argument for the c_f_pointer intrinsic, as specified in the Fortran 2023 standard. I've also included documentation and tests, and the regression tests on aarch64-linux passed. Please take a look when you have a chance. Thanks, YuaoFrom 74eba4dbd7601ca4c854b0e6138b6bffc727222a Mon Sep 17 00:00:00 2001 From: Yuao Ma Date: Tue, 5 Aug 2025 23:33:16 +0800 Subject: [PATCH] fortran: add optional lower arg to c_f_pointer This patch adds support for the optional lower argument in intrinsic c_f_pointer specified in Fortran 2023. Test cases and documentation have also been updated. gcc/fortran/ChangeLog: * check.cc (gfc_check_c_f_pointer): Check lower arg legitimacy. * intrinsic.cc (add_subroutines): Teach c_f_pointer about lower arg. * intrinsic.h (gfc_check_c_f_pointer): Add lower arg. * intrinsic.texi: Update lower arg for c_f_pointer. * trans-intrinsic.cc (conv_isocbinding_subroutine): Add logic handle lower. gcc/testsuite/ChangeLog: * gfortran.dg/c_f_pointer_shape_tests_3.f03: Check rank & type for lower. * gfortran.dg/c_f_pointer_shape_tests_7.f90: New test. Signed-off-by: Yuao Ma --- gcc/fortran/check.cc | 35 +- gcc/fortran/intrinsic.cc | 5 +- gcc/fortran/intrinsic.h | 2 +- gcc/fortran/intrinsic.texi| 12 ++- gcc/fortran/trans-intrinsic.cc| 100 -- .../gfortran.dg/c_f_pointer_shape_tests_3.f03 | 29 +++-- .../gfortran.dg/c_f_pointer_shape_tests_7.f90 | 34 ++ 7 files changed, 171 insertions(+), 46 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/c_f_pointer_shape_tests_7.f90 diff --git a/gcc/fortran/check.cc b/gcc/fortran/check.cc index 862652683a7..3c4594be9e7 100644 --- a/gcc/fortran/check.cc +++ b/gcc/fortran/check.cc @@ -6081,7 +6081,8 @@ gfc_check_c_associated (gfc_expr *c_ptr_1, gfc_expr *c_ptr_2) bool -gfc_check_c_f_pointer (gfc_expr *cptr, gfc_expr *fptr, gfc_expr *shape) +gfc_check_c_f_pointer (gfc_expr *cptr, gfc_expr *fptr, gfc_expr *shape, + gfc_expr *lower) { symbol_attribute attr; const char *msg; @@ -6156,6 +6157,38 @@ gfc_check_c_f_pointer (gfc_expr *cptr, gfc_expr *fptr, gfc_expr *shape) } } + if (!shape && lower) +{ + gfc_error ("Unexpected LOWER argument at %L to C_F_POINTER " +"with scalar FPTR", +&fptr->where); + return false; +} + + if (lower && !rank_check (lower, 3, 1)) +return false; + + if (lower && !type_check (lower, 3, BT_INTEGER)) +return false; + + if (lower) +{ + mpz_t size; + if (gfc_array_size (lower, &size)) + { + if (mpz_cmp_ui (size, fptr->rank) != 0) + { + mpz_clear (size); + gfc_error ( + "LOWER argument at %L to C_F_POINTER must have the same " + "size as the RANK of FPTR", + &lower->where); + return false; + } + mpz_clear (size); + } +} + if (fptr->ts.type == BT_CLASS) { gfc_error ("Polymorphic FPTR at %L to C_F_POINTER", &fptr->where); diff --git a/gcc/fortran/intrinsic.cc b/gcc/fortran/intrinsic.cc index c99a7a86aea..e2847f08daa 100644 --- a/gcc/fortran/intrinsic.cc +++ b/gcc/fortran/intrinsic.cc @@ -3943,11 +3943,12 @@ add_subroutines (void) /* The following subroutines are part of ISO_C_BINDING. */ - add_sym_3s ("c_f_pointer", GFC_ISYM_C_F_POINTER, CLASS_IMPURE, BT_UNKNOWN, 0, + add_sym_4s ("c_f_pointer", GFC_ISYM_C_F_POINTER, CLASS_IMPURE, BT_UNKNOWN, 0, GFC_STD_F2003, gfc_check_c_f_pointer, NULL, NULL, "cptr", BT_VOID, 0, REQUIRED, INTENT_IN, "fptr", BT_UNKNOWN, 0, REQUIRED, INTENT_OUT, - "shape", BT_INTEGER, di, OPTIONAL, INTENT_IN); + "shape", BT_INTEGER, di, OPTIONAL, INTENT_IN, + "lower", BT_INTEGER, di, OPTIONAL, INTENT_IN); make_from_module(); add_sym_2s ("c_f_procpointer", GFC_ISYM_C_F_PROCPOINTER, CLASS_IMPURE, diff --git a/gcc/fortran/intrinsic.h b/gcc/fortran/intrinsic.h index 8a0ab935e1f..048196d65c3 100644 --- a/gcc/fortran/intrinsic.h +++ b/gcc/fortran/intrinsic.h @@ -165,7 +165,7 @@ bool gfc_check_sign (gfc_expr *, gfc_expr *); bool gfc_check_signal (gfc_expr *, gfc_expr *); bool gfc_check_sizeof (gfc_expr *); bool gfc_check_c_associated (gfc_expr *, gfc_expr *); -bool gfc_check_c_f_pointer (gfc_expr *, gfc_expr *, gfc_expr *); +bool gfc_check_c_f_pointer (gfc_expr *, gfc_expr *, gfc_expr *, gfc_expr *); bool gfc_check_c_f_procpointer (gfc_expr *, gfc_expr *); bool gfc_check_c_funloc (gfc_expr *); bool gfc_check_c_loc (gfc
[PATCH] fortran: implement conditional expression for fortran 2023
Hi all, This patch introduces support for conditional expressions (also known as ternary operators in some languages) to Fortran. I decided to implement this feature after discovering that this common functionality, widely used in C/C++, wasn't available until the Fortran 2023 standard. This was also a great opportunity to learn more about gfortran's internals, as it required touching both the front-end and back-end. For the front-end, I manually parsed the right-associative conditional expression, which required some special logic. On the back-end, I simply forwarded it to COND_EXPR. I also added support for tools like the parse-tree dump. I'm not sure if we need to handle constant folding ourselves, or if COND_EXPR will take care of it. I plan to add more tests and polish the patch. However, I want to get some early feedback on the general approach. Please take a look when you have a moment. Thanks in advance, YuaoFrom 8b0312442ade17f64ae7c8059daa3af46a0bceda Mon Sep 17 00:00:00 2001 From: Yuao Ma Date: Wed, 30 Jul 2025 22:38:57 +0800 Subject: [PATCH] fortran: implement conditional expression for fortran 2023 TBD gcc/fortran/ChangeLog: * dump-parse-tree.cc (show_expr): * expr.cc (gfc_get_conditional_expr): (gfc_copy_expr): (free_expr0): * gfortran.h (enum expr_t): (gfc_get_operator_expr): (gfc_get_conditional_expr): * matchexp.cc (gfc_match_expr): (match_binary): * resolve.cc (resolve_conditional): (gfc_resolve_expr): * trans-expr.cc (gfc_conv_conditional_expr): (gfc_conv_expr): gcc/testsuite/ChangeLog: * gfortran.dg/conditional_1.f90: New test. --- gcc/fortran/dump-parse-tree.cc | 10 gcc/fortran/expr.cc | 34 + gcc/fortran/gfortran.h | 30 +-- gcc/fortran/matchexp.cc | 55 +++-- gcc/fortran/resolve.cc | 39 +++ gcc/fortran/trans-expr.cc | 35 + gcc/testsuite/gfortran.dg/conditional_1.f90 | 12 + 7 files changed, 206 insertions(+), 9 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/conditional_1.f90 diff --git a/gcc/fortran/dump-parse-tree.cc b/gcc/fortran/dump-parse-tree.cc index 3cd2eeef11a..eda0659d6e2 100644 --- a/gcc/fortran/dump-parse-tree.cc +++ b/gcc/fortran/dump-parse-tree.cc @@ -767,6 +767,16 @@ show_expr (gfc_expr *p) break; +case EXPR_CONDITIONAL: + fputc ('(', dumpfile); + show_expr (p->value.conditional.condition); + fputs (" ? ", dumpfile); + show_expr (p->value.conditional.true_expr); + fputs (" : ", dumpfile); + show_expr (p->value.conditional.false_expr); + fputc (')', dumpfile); + break; + case EXPR_COMPCALL: show_compcall (p); break; diff --git a/gcc/fortran/expr.cc b/gcc/fortran/expr.cc index b8d04ff6f36..2ccd8248a8a 100644 --- a/gcc/fortran/expr.cc +++ b/gcc/fortran/expr.cc @@ -116,6 +116,25 @@ gfc_get_operator_expr (locus *where, gfc_intrinsic_op op, return e; } +/* Get a new expression node that is an conditional expression node. */ + +gfc_expr * +gfc_get_conditional_expr (locus *where, gfc_expr *condition, + gfc_expr *true_expr, gfc_expr *false_expr) +{ + gfc_expr *e; + + e = gfc_get_expr (); + e->expr_type = EXPR_CONDITIONAL; + e->value.conditional.condition = condition; + e->value.conditional.true_expr = true_expr; + e->value.conditional.false_expr = false_expr; + + if (where) +e->where = *where; + + return e; +} /* Get a new expression node that is an structure constructor of given type and kind. */ @@ -393,6 +412,15 @@ gfc_copy_expr (gfc_expr *p) break; +case EXPR_CONDITIONAL: + q->value.conditional.condition + = gfc_copy_expr (p->value.conditional.condition); + q->value.conditional.true_expr + = gfc_copy_expr (p->value.conditional.true_expr); + q->value.conditional.false_expr + = gfc_copy_expr (p->value.conditional.false_expr); + break; + case EXPR_FUNCTION: q->value.function.actual = gfc_copy_actual_arglist (p->value.function.actual); @@ -502,6 +530,12 @@ free_expr0 (gfc_expr *e) gfc_free_expr (e->value.op.op2); break; +case EXPR_CONDITIONAL: + gfc_free_expr (e->value.conditional.condition); + gfc_free_expr (e->value.conditional.true_expr); + gfc_free_expr (e->value.conditional.false_expr); + break; + case EXPR_FUNCTION: gfc_free_actual_arglist (e->value.function.actual); break; diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h index d9dcd1b504f..dde892d235a 100644 --- a/gcc/fortran/gfortran.h +++ b/gcc/fortran/gfortran.h @@ -176,8 +176,19 @@ enum gfc_sour
Re: [PATCH] fortran: implment split for fortran 2023
On 7/27/2025 8:51 PM, Mikael Morin wrote: Le 27/07/2025 à 13:46, Yuao Ma a écrit : On 7/27/2025 7:14 PM, Mikael Morin wrote: Le 27/07/2025 à 11:37, Yuao Ma a écrit : On 7/27/2025 5:19 PM, Mikael Morin wrote: +gfc_charlen_type +string_split (gfc_charlen_type stringlen, const CHARTYPE *string, + gfc_charlen_type setlen, const CHARTYPE *set, + gfc_charlen_type pos, GFC_LOGICAL_4 back) +{ + gfc_charlen_type i, j; + + if (!back) + { + if (pos > stringlen) + runtime_error ("If BACK is present with the value false, the value of " + "POS shall be in the range [0, LEN (STRING)]"); + The condition doesn't check pos >= 0; I think the case pos < 0 doesn't work. The variable pos is an unsigned type, so checking for < 0 is unnecessary. (I was not initially aware of this, but the compiler warning alerted me.) Mmmh, good point. But POS is a user variable, so a signed type originally. Can you check the following: integer(kind=1) :: my_pos = -1 character(len=300) :: my_str = "" call split(my_str, " ", my_pos) Is it rejected at runtime? I expect the unsigned conversion to convert -1 to 255 and then the call would be (wrongly) accepted. Yes, it's rejected at runtime. The trick lies in how the pos is handled: it's first converted to gfc_charlen_type (an unsigned long long). So, even if kind=1 and the input is -1, it wraps around to a very large positive value (specifically, ULLONG_MAX - 1). This value far exceeds 300, leading to a runtime error. + pos_for_call = fold_convert (gfc_charlen_type_node, pos); Ok, according to the dump the conversion is to integer(kind=8) which is signed. Anyway, I think it works for any sensible case. So the case above is rejected, but now the value of pos printed is not what the user specified. This is what I get: Fortran runtime error: If BACK is present with the value false, the value of POS shall be in the range [0, LEN (STRING)], where POS = 18446744073709551615 and LEN (STRING) = 300 Good point. Since the conversion is always to integer(kind=8), we can simply use %ld instead of %lu as the format argument. Fixed. Another thing I noticed: + gfc_add_expr_to_block (&block, call); + gfc_add_modify (&block, pos, fold_convert (TREE_TYPE (pos), call)); You are using the call twice; this results in two calls to the function split in the dump: _gfortran_string_split (300, &my_str, 1, &" "[1]{lb: 1 sz: 1}, (integer(kind=8)) my_pos, 0); my_pos = (integer(kind=1)) _gfortran_string_split (300, &my_str, 1, &" "[1]{lb: 1 sz: 1}, (integer(kind=8)) my_pos, 0); The gfc_add_expr_to_block call appears to be redundant. Fixed. I'm fine with this version. I'll leave one more day for someone else to comment further and then I'll push it for you. Thanks for the patch. Gentle ping~
Re: [PATCH] fortran: implment split for fortran 2023
On 7/27/2025 7:14 PM, Mikael Morin wrote: Le 27/07/2025 à 11:37, Yuao Ma a écrit : On 7/27/2025 5:19 PM, Mikael Morin wrote: +gfc_charlen_type +string_split (gfc_charlen_type stringlen, const CHARTYPE *string, + gfc_charlen_type setlen, const CHARTYPE *set, + gfc_charlen_type pos, GFC_LOGICAL_4 back) +{ + gfc_charlen_type i, j; + + if (!back) + { + if (pos > stringlen) + runtime_error ("If BACK is present with the value false, the value of " + "POS shall be in the range [0, LEN (STRING)]"); + The condition doesn't check pos >= 0; I think the case pos < 0 doesn't work. The variable pos is an unsigned type, so checking for < 0 is unnecessary. (I was not initially aware of this, but the compiler warning alerted me.) Mmmh, good point. But POS is a user variable, so a signed type originally. Can you check the following: integer(kind=1) :: my_pos = -1 character(len=300) :: my_str = "" call split(my_str, " ", my_pos) Is it rejected at runtime? I expect the unsigned conversion to convert -1 to 255 and then the call would be (wrongly) accepted. Yes, it's rejected at runtime. The trick lies in how the pos is handled: it's first converted to gfc_charlen_type (an unsigned long long). So, even if kind=1 and the input is -1, it wraps around to a very large positive value (specifically, ULLONG_MAX - 1). This value far exceeds 300, leading to a runtime error. + pos_for_call = fold_convert (gfc_charlen_type_node, pos); Ok, according to the dump the conversion is to integer(kind=8) which is signed. Anyway, I think it works for any sensible case. So the case above is rejected, but now the value of pos printed is not what the user specified. This is what I get: Fortran runtime error: If BACK is present with the value false, the value of POS shall be in the range [0, LEN (STRING)], where POS = 18446744073709551615 and LEN (STRING) = 300 Good point. Since the conversion is always to integer(kind=8), we can simply use %ld instead of %lu as the format argument. Fixed. Another thing I noticed: + gfc_add_expr_to_block (&block, call); + gfc_add_modify (&block, pos, fold_convert (TREE_TYPE (pos), call)); You are using the call twice; this results in two calls to the function split in the dump: _gfortran_string_split (300, &my_str, 1, &" "[1]{lb: 1 sz: 1}, (integer(kind=8)) my_pos, 0); my_pos = (integer(kind=1)) _gfortran_string_split (300, &my_str, 1, &" "[1]{lb: 1 sz: 1}, (integer(kind=8)) my_pos, 0); The gfc_add_expr_to_block call appears to be redundant. Fixed.From 8e867ea3593f85cdec4dd1dfe5b6a8729612b8eb Mon Sep 17 00:00:00 2001 From: Yuao Ma Date: Sun, 27 Jul 2025 19:41:25 +0800 Subject: [PATCH] fortran: implment split for fortran 2023 This patch includes the implementation, documentation, and test case for SPLIT. gcc/fortran/ChangeLog: * check.cc (gfc_check_split): Argument check for SPLIT. * gfortran.h (enum gfc_isym_id): Define GFC_ISYM_SPLIT. * intrinsic.cc (add_subroutines): Register SPLIT intrinsic. * intrinsic.h (gfc_check_split): New decl. (gfc_resolve_split): Ditto. * intrinsic.texi: SPLIT documentation. * iresolve.cc (gfc_resolve_split): Add resolved_sym for SPLIT. * trans-decl.cc (gfc_build_intrinsic_function_decls): Add decl for SPLIT in libgfortran. * trans-intrinsic.cc (conv_intrinsic_split): SPLIT codegen. (gfc_conv_intrinsic_subroutine): Handle SPLIT case. * trans.h (GTY): Declare gfor_fndecl_string_split{, _char4}. libgfortran/ChangeLog: * gfortran.map: Add split symbol. * intrinsics/string_intrinsics_inc.c (string_split): Runtime support for SPLIT. gcc/testsuite/ChangeLog: * gfortran.dg/split_1.f90: New test. * gfortran.dg/split_2.f90: New test. * gfortran.dg/split_3.f90: New test. * gfortran.dg/split_4.f90: New test. Signed-off-by: Yuao Ma --- gcc/fortran/check.cc | 21 ++ gcc/fortran/gfortran.h| 2 + gcc/fortran/intrinsic.cc | 8 +++ gcc/fortran/intrinsic.h | 2 + gcc/fortran/intrinsic.texi| 64 + gcc/fortran/iresolve.cc | 13 gcc/fortran/trans-decl.cc | 14 gcc/fortran/trans-intrinsic.cc| 72 +++ gcc/fortran/trans.h | 2 + gcc/testsuite/gfortran.dg/split_1.f90 | 28 gcc/testsuite/gfortran.dg/split_2.f90 | 22 ++ gcc/testsuite/gfortran.dg/split_3.f90 | 11 +++ gcc/testsuite/gfortran.dg/split_4.f90 | 11 +++ libgfortran/gfortran.map | 6 ++ .../intrinsics/string_intrinsics_inc.c| 52 +
Re: [PATCH] fortran: cleanup duplicate tests for c_f_pointer_shape_driver; nfc
On 8/6/2025 5:20 PM, Tobias Burnus wrote: Hi Yuao, thanks for the patch. — I have one nit: Yuao Ma wrote: This patch cleans up a duplicate test driver. Regression-tested. OK for trunk? ... Subject: [PATCH] fortran: cleanup duplicate tests for c_f_pointer_shape_driver; nfc '; nfc'? tests_2_driver and tests_4_driver are identical, and tests_4_driver is not used at all. This patch clean this up, and format the driver with gcc style. ... * gfortran.dg/c_f_pointer_shape_tests_2_driver.c: Removed. * gfortran.dg/c_f_pointer_shape_tests_4_driver.c: Removed. * gfortran.dg/c_f_pointer_shape_tests_driver.c: New test. This sounds as if c_f_pointer_shape_tests_driver.c is new. But it is just the renamed _2_driver plus some formatting. (As can be gathered from the wording above the ChangeLog). I have bluntly updated the ChangeLog entry to * gfortran.dg/c_f_pointer_shape_tests_2_driver.c: Renamed to ... * gfortran.dg/c_f_pointer_shape_tests_driver.c: ... this; format with gcc style. Thank you for polishing the ChangeLog! It is much clearer now. Yuao
Re: [PATCH] fortran: add optional lower arg to c_f_pointer
Hi Tobias, On 8/7/2025 8:59 PM, Tobias Burnus wrote: Yuao Ma wrote: Given the "Fortran 2023:" prefix, I wonder whether the wording shouldn't be tweaked: + call c_f_pointer(x, ptr_2d, shape=myshape_2d, lower=mylower_2d) ! { dg-error "Fortran 2023: Unexpected LOWER argument at" } It reads a bit as if with Fortran 2023, it is unexpected - but it is unexpected only with older versions. I wonder whether it is clearer using: "Error: Fortran 2023: LOWER argument at" Yes I agree. Done. * * * * I have a minor documentation nit; current wording is at https://gcc.gnu.org/onlinedocs/gfortran/C_005fF_005fPOINTER.html Namely, ... Yuao Ma wrote: --- a/gcc/fortran/intrinsic.texi +++ b/gcc/fortran/intrinsic.texi @@ -3368,10 +3368,10 @@ Fortran 2003 and later @table @asis @item @emph{Synopsis}: -@code{CALL C_F_POINTER(CPTR, FPTR[, SHAPE])} +@code{CALL C_F_POINTER(CPTR, FPTR[, SHAPE, LOWER])} @item @emph{Description}: -@code{C_F_POINTER(CPTR, FPTR[, SHAPE])} assigns the target of the C pointer +@code{C_F_POINTER(CPTR, FPTR[, SHAPE, LOWER])} assigns the target of the C pointer @var{CPTR} to the Fortran pointer @var{FPTR} and specifies its shape. I think some wording like the following is missing: "For an array @var{FPTR}, the lower bounds are specified by @var{LOWER} if present and otherwise equal to 1." Done. Actually, I wonder whether this wording is better placed at the end of "Description:" instead of under "Arguments:" for 'LOWER' See https://gcc.gnu.org/onlinedocs/gfortran/C_005fF_005fPOINTER.html for how it currently looks. Do you concur? Both seem good to me. Done. PS: FYI – contrib/download_prerequisites now downloads MPFR 4.2.2, i.e. it will have the 'pi' support. Great to hear that. Thank you for your effort! PPS: I am aware of other emails, esp. related to libquadmath, which someone (me?) should be take care of … That would be great, thanks! I think the libquadmath patch is ready to go from the current policy. For future optimizations, we should always handle them in glibc first. The Fortran part of libquadmath is only needed for systems with older versions of glibc, so I believe importing from glibc will be sufficient for now. (It's worth noting that even the core-math project doesn't have trig-pi functions for binary80/128). In addition to the libquadmath patch, there are two others: * The libgfortran fallback: Joseph suggested we can learn from the glibc implementation to achieve higher precision here. I'll find some time to work on this. * The conditional expression patch: It only supports basic types and doesn't work with strings, arrays, etc. Adding a new expression type is proving more difficult than I initially thought, as it requires careful handling every time we switch against an expression type. I'm hoping to implement this incrementally to make life easier. And: PS: Eventually, we should update https://gcc.gnu.org/gcc-16/ changes.html for the accumulated Fortran changes … [That's the https://gcc.gnu.org/ about.html#git ] Yes, we could summarize the work done for Fortran 2023, similar to how the Flang documentation(https://flang.llvm.org/docs/ FortranStandardsSupport.html#fortran-2023) does. I think that's two separate documents: One is about the news (release news, changes), i.e. gcc.gnu.org/gcc-16/changes.html for the current pending changes, https://gcc.gnu.org/gcc-15/changes.html#fortran is what we had last year. That's somewhat easy as we only need to add the new stuff and it is just unfortunate but not wrong to miss some features. This is useful & has a low effort. The other is to have a supported-feature list. That's quite nice, but requires more maintenance work. We have this kind of lists already in GCC: For C:https://gcc.gnu.org/projects/c-status.html For C++:https://gcc.gnu.org/projects/cxx-status.html For the C++ standard library:https://gcc.gnu.org/onlinedocs/libstdc++/manual/status.html For OpenMP - by version:https://gcc.gnu.org/projects/gomp/#implementation-status and for a specific version (here mainline) https://gcc.gnu.org/onlinedocs/libgomp/OpenMP-Implementation-Status.html (in the .texi file, i.e. it also ships with GCC). For gfortran, we have in the wiki →https://gcc.gnu.org/wiki/GFortran namely: https://gcc.gnu.org/wiki/Fortran2003Status https://gcc.gnu.org/wiki/Fortran2008Status https://gcc.gnu.org/wiki/Fortran2018Status https://gcc.gnu.org/wiki/TS29113Status I think this has bitrotted a bit and a Fortran2023 version is missing. And some statuses are outdated, like move_alloc being marked as unsupported. Thanks for the review and explanation! YuaoFrom 590cce1b335c781355d8518ad9c14502b127130a Mon Sep 17 00:00:00 2001 From: Yuao Ma Date: Thu, 7 Aug 2025 22:35:17 +0800 Subject: [PATCH] fortran: add optional lower arg to c
Re: [PATCH] libgfortran: add fallback for trigonometric pi-based functions
On 8/5/2025 4:05 AM, Joseph Myers wrote: On Sun, 3 Aug 2025, Yuao Ma wrote: By the way, could you please take another look at the libquadmath update? https://inbox.sourceware.org/fortran/kl1pr0601mb4291e1457dc09fe3aa6652c884...@kl1pr0601mb4291.apcprd06.prod.outlook.com/ Using the script is what's expected for such a libquadmath update. Thanks for the clarification! I'm wondering if this patch needs more refinement or if it's ready to go. The seven trig-pi functions are needed to implement a fallback for trig-pi functions in gfortran. We can easily update them whenever glibc has an update. Thanks, Yuao
[PATCH] fortran: map atand(y, x) to atan2d(y, x) [PR113413]
Hi Tobias, Following up on your review comments, I have updated the patch. Specifically, I have: * Added the PR number to the subject line. * Used the -b and -p options when running git gcc-commit-mklog. * Updated the intrinsic documentation as requested. Could you please take another look when you have a moment? Thanks again for your review, Yuao 0001-fortran-map-atand-y-x-to-atan2d-y-x-PR113413.patch Description: 0001-fortran-map-atand-y-x-to-atan2d-y-x-PR113413.patch
[PATCH] gcc: add trigonometric pi-based functions as gcc builtins
Hi Joseph, I have updated the patch based on your review comments. I added the newly introduced builtin to extend.texi and mentioned the PR in the commit message. Could you please take another look when you have a moment? Yuao From: Joseph Myers Sent: Thursday, May 15, 2025 0:47 To: Yuao Ma Cc: gcc-patches@gcc.gnu.org ; fort...@gcc.gnu.org ; tbur...@baylibre.com Subject: Re: [PATCH] gcc: add trigonometric pi-based functions as gcc builtins On Wed, 14 May 2025, Yuao Ma wrote: > Hi all, > > This patch adds trigonometric pi-based functions as gcc builtins: acospi, > asinpi, atan2pi, > atanpi, cospi, sinpi, and tanpi. Latest glibc already provides support for > these functions, which we plan to leverage in future gfortran implementations. > > The patch includes two test cases to verify both correct code generation and > function definition. > > If approved, I suggest committing this foundational change first. Constant > folding for these builtins will be addressed in subsequent patches. Note that either this change, or a subsequent one that makes the built-in functions do something useful, should also update extend.texi, "Library Builtins", to mention the new functions. (The text there doesn't distinguish existing C23 built-in functions, such as exp10 or roundeven, from those that are pure extensions, but addressing that is independent of adding new functions to the list. Also, I'm not sure these sentences with very long lists of functions are really the optimal way of presenting the information about such built-in functions; maybe Sandra has better ideas about how to document this, but again that's independent of adding new functions.) The commit message should reference PR c/118592 (it's not a full fix, but it's partial progress towards the full set of built-in functions / constant folding). -- Joseph S. Myers josmy...@redhat.com 0001-gcc-add-trigonometric-pi-based-functions-as-gcc-buil.patch Description: 0001-gcc-add-trigonometric-pi-based-functions-as-gcc-buil.patch
[PATCH] gcc: add trigonometric pi-based functions as gcc builtins
Hi all, This patch adds trigonometric pi-based functions as gcc builtins: acospi, asinpi, atan2pi, atanpi, cospi, sinpi, and tanpi. Latest glibc already provides support for these functions, which we plan to leverage in future gfortran implementations. The patch includes two test cases to verify both correct code generation and function definition. If approved, I suggest committing this foundational change first. Constant folding for these builtins will be addressed in subsequent patches. Best regards, Yuao 0001-gcc-add-trigonometric-pi-based-functions-as-gcc-buil.patch Description: 0001-gcc-add-trigonometric-pi-based-functions-as-gcc-buil.patch
Re: [PATCH] gcc: add trigonometric pi-based functions as gcc builtins
Hi Jakub, Thank you for your suggestion. I actually learned from your earlier patch (https://gcc.gnu.org/cgit/gcc/commit?id=7f940822) and had already planned to update tree-call-cdce.cc when handling these builtins. Your guidance is much appreciated! Best regards, Yuao From: Jakub Jelinek Sent: Saturday, May 17, 2025 22:11 To: Yuao Ma Cc: gcc-patches@gcc.gnu.org ; fort...@gcc.gnu.org ; tbur...@baylibre.com ; j...@polyomino.org.uk Subject: Re: [PATCH] gcc: add trigonometric pi-based functions as gcc builtins On Wed, May 14, 2025 at 02:22:23PM +, Yuao Ma wrote: > If approved, I suggest committing this foundational change first. Constant > folding for these builtins will be addressed in subsequent patches. Note, not just constant folding is needed, but I think the builtins should be handled in tree-call-cdce.cc (can_test_argument_range, edom_only_function, get_no_error_domain). Jakub
[PATCH] fortran: add constant input support for trig functions with half-revolutions
Sorry, the previous patch had some issues with the test case. Please refer to the updated version, which resolves the problem. From: Yuao Ma Sent: Tuesday, May 20, 2025 23:54 To: gcc-patches@gcc.gnu.org ; GCC Fortran ; tbur...@baylibre.com Subject: [PATCH] fortran: add constant input support for trig functions with half-revolutions Hi all, This patch introduces constant input support for trigonometric functions, including those involving half-revolutions. Both valid and invalid inputs have been thoroughly tested, as have mpfr versions greater than or equal to 4.2 and less than 4.2. Inspired by Steve's previous work, this patch also fixes subtle bugs revealed by newly added test cases. If this patch is merged, I plan to work on middle-end optimization support for previously added GCC built-ins and libgfortran intrinsics. Best regards, Yuao 0001-fortran-add-constant-input-support-for-trig-function.patch Description: 0001-fortran-add-constant-input-support-for-trig-function.patch
[PATCH] fortran: add constant input support for trig functions with half-revolutions
Hi all, This patch introduces constant input support for trigonometric functions, including those involving half-revolutions. Both valid and invalid inputs have been thoroughly tested, as have mpfr versions greater than or equal to 4.2 and less than 4.2. Inspired by Steve's previous work, this patch also fixes subtle bugs revealed by newly added test cases. If this patch is merged, I plan to work on middle-end optimization support for previously added GCC built-ins and libgfortran intrinsics. Best regards, Yuao 0001-fortran-add-constant-input-support-for-trig-function.patch Description: 0001-fortran-add-constant-input-support-for-trig-function.patch
[PATCH] fortran: add constant input support for trig functions with half-revolutions
Hi Tobias, Thanks for the thorough code review! I'm pretty swamped for the next couple of days, and I'll get the patch updated as you requested this weekend. > I don't understand this change. First, I don't see any reason why this > should get modified. And by modifying it, a simple "git blame" will also > point to the "wrong" commit. > > But also otherwise: it does not save the on the number of lines and it > also breaks the current pattern: > > > First line – name plus classification. > > Second line – return value, Fortran standard > > Third line – functions pointers > > Fourth line – arguments > > > I have to admit that we haven't been super consistent and the first two > lines are often also in one, but mostly (but not always) the function > pointer are on a line of their own. > > > Thus, I think we can break this pattern - but, in particular, I wouldn't > do so for the existing code, unless there is a good reason for doing so. You're absolutely right that the best way to keep changes minimal is to just rename the `*resolve*` function. The challenge is I used clang-format (with the `contrib/clang-format` file) for formatting. I know it's just a recommendation, but it's super helpful for keeping the code style consistent, and doing that manually is a pain. For the current pattern section, I think it'd be better to refactor the `add_sym*` function and use a struct to group the arguments. clang-format only touched the lines I changed in my patch, so the rest is unaffected. Does that make sense? If you're really set on the style for that part, I can definitely make the change. Yuao
[PATCH] fortran: add constant input support for trig functions with half-revolutions
Hi all, I've reverted the recent format changes, as three reviewers indicated they caused more harm than good. Are there any functional problems I need to address? Thanks, Yuao 0001-fortran-add-constant-input-support-for-trig-function.patch Description: 0001-fortran-add-constant-input-support-for-trig-function.patch
[PATCH] fortran: add constant input support for trig functions with half-revolutions
Hi Steve, Thanks for your review! I've updated the patch. > this range_check() is unneeded. Done. > As a side note, the error message is slightly misleading > (although it will not be issued). Technically, x = -1 or 1 > are allowed values, and neither is **between** -1 and 1. You're right, the original message was a bit imprecise. I've updated this and five other similar error messages in the patch for better accuracy. Yuao 0001-fortran-add-constant-input-support-for-trig-function.patch Description: 0001-fortran-add-constant-input-support-for-trig-function.patch
[PATCH] fortran: add constant input support for trig functions with half-revolutions
Hi Tobias, > you will notice that the PR is not recognized. The format as mentioned before > is "PR component/number". Namely: Thanks for the reminder! I'll use `-p` to double-check PR numbers going forward. > The second part is not what you are doing, you are actually changing the > call from gfc_resolve_trigd{,2} to gfc_resolve_trig{,2}. Done. > > + gfc_error ("If first argument of ATAN2PI at %L is zero, then the " + > > "second argument must not be zero", + &y->where); > > > I am a non-native speaker, but I think there is a "the" missing before > "first". You're right, I've corrected this and the two existing instances. > BTW: If you have '(1)', you need to escape it with '\\(1\\)' or as the > (...) don't matter, just use '.1.' as pattern. For '[...]' you need to > make sure that [...] is not read as pattern range (such as '[a-z]'), > i.e. use '\\\[-1, 1\\\]' (albeit it also works with only two \\). Yeah, I forgot to use double escaping. Done. > BTW: You could also use "intrinsic :: acospi" - which tells the compiler > that the function is supposed to be an intrinsic. Done. This will make the test case much cleaner! Yuao 0001-fortran-add-constant-input-support-for-trig-function.patch Description: 0001-fortran-add-constant-input-support-for-trig-function.patch
[PATCH] gcc: middle-end opt for trigonometric pi-based functions builtins
Hi all, This patch has been updated to conditionally fold the specified math functions only when using MPFR version 4.2.0 or newer. To accompany this change, the test case now utilizes `__builtin_constant_p` to determine whether to execute the tests against the folded functions. Let me know if this patch needs further modifications! Best regards, Yuao 0001-gcc-middle-end-opt-for-trigonometric-pi-based-functi.patch Description: 0001-gcc-middle-end-opt-for-trigonometric-pi-based-functi.patch
[PATCH] fortran: add intrinsic doc for trig functions with half-revolutions
Hi all, This patch is a follow-up to commit r16-938-ge8fdd55ec90749. In this patch, we add intrinsic documentation for the newly added trig functions with half-revolutions. We also reorder the documentation for `atand` to place it in correct alphabetical order. BR, Yuao 0001-fortran-add-intrinsic-doc-for-trig-functions-with-ha.patch Description: 0001-fortran-add-intrinsic-doc-for-trig-functions-with-ha.patch
Re: [PATCH] fortran: add optional lower arg to c_f_pointer
Hi Tobias, I noticed that this patch was already committed as r16-3154-g587b8a62f50179! Thanks, Yuao
Re: [PATCH] fortran: add optional lower arg to c_f_pointer
Hi Tobias, On 8/6/2025 4:32 PM, Tobias Burnus wrote: Hi Yuao, thanks for your patch. I have two nits: * There is no diagnostic for -std=f2018 (or lower); can you add the 'gfc_notify_std (GFC_STD_F2023' ? Done. Testcase added. * I have a minor documentation nit; current wording is at https://gcc.gnu.org/onlinedocs/gfortran/C_005fF_005fPOINTER.html Namely, ... Yuao Ma wrote: --- a/gcc/fortran/intrinsic.texi +++ b/gcc/fortran/intrinsic.texi @@ -3368,10 +3368,10 @@ Fortran 2003 and later @table @asis @item @emph{Synopsis}: -@code{CALL C_F_POINTER(CPTR, FPTR[, SHAPE])} +@code{CALL C_F_POINTER(CPTR, FPTR[, SHAPE, LOWER])} @item @emph{Description}: -@code{C_F_POINTER(CPTR, FPTR[, SHAPE])} assigns the target of the C pointer +@code{C_F_POINTER(CPTR, FPTR[, SHAPE, LOWER])} assigns the target of the C pointer @var{CPTR} to the Fortran pointer @var{FPTR} and specifies its shape. I think some wording like the following is missing: "For an array @var{FPTR}, the lower bounds are specified by @var{LOWER} if present and otherwise equal to 1." Done. @item @emph{Standard}: Fortran 2003 and later I think we should append ", with @var{LOWER} argument Fortran 2023 and later" to "Standard". Done. PS: Eventually, we should update https://gcc.gnu.org/gcc-16/changes.html for the accumulated Fortran changes … [That's the https://gcc.gnu.org/ about.html#git ] Yes, we could summarize the work done for Fortran 2023, similar to how the Flang documentation(https://flang.llvm.org/docs/FortranStandardsSupport.html#fortran-2023) does. The new version of this patch also fixes two minor issues, the token location in check.cc and the removal of unnecessary braces for single statements after an else clause. Thanks, YuaoFrom e885ab884b15ef052a3faaefff679065bfe714d0 Mon Sep 17 00:00:00 2001 From: Yuao Ma Date: Wed, 6 Aug 2025 22:48:46 +0800 Subject: [PATCH] fortran: add optional lower arg to c_f_pointer This patch adds support for the optional lower argument in intrinsic c_f_pointer specified in Fortran 2023. Test cases and documentation have also been updated. gcc/fortran/ChangeLog: * check.cc (gfc_check_c_f_pointer): Check lower arg legitimacy. * intrinsic.cc (add_subroutines): Teach c_f_pointer about lower arg. * intrinsic.h (gfc_check_c_f_pointer): Add lower arg. * intrinsic.texi: Update lower arg for c_f_pointer. * trans-intrinsic.cc (conv_isocbinding_subroutine): Add logic handle lower. gcc/testsuite/ChangeLog: * gfortran.dg/c_f_pointer_shape_tests_7.f90: New test. * gfortran.dg/c_f_pointer_shape_tests_8.f03: New test. * gfortran.dg/c_f_pointer_shape_tests_9.f90: New test. Signed-off-by: Yuao Ma --- gcc/fortran/check.cc | 41 +++- gcc/fortran/intrinsic.cc | 5 +- gcc/fortran/intrinsic.h | 2 +- gcc/fortran/intrinsic.texi| 15 +-- gcc/fortran/trans-intrinsic.cc| 98 +-- .../gfortran.dg/c_f_pointer_shape_tests_7.f90 | 35 +++ .../gfortran.dg/c_f_pointer_shape_tests_8.f03 | 24 + .../gfortran.dg/c_f_pointer_shape_tests_9.f90 | 17 8 files changed, 195 insertions(+), 42 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/c_f_pointer_shape_tests_7.f90 create mode 100644 gcc/testsuite/gfortran.dg/c_f_pointer_shape_tests_8.f03 create mode 100644 gcc/testsuite/gfortran.dg/c_f_pointer_shape_tests_9.f90 diff --git a/gcc/fortran/check.cc b/gcc/fortran/check.cc index 862652683a7..e8bbf638d9f 100644 --- a/gcc/fortran/check.cc +++ b/gcc/fortran/check.cc @@ -6081,7 +6081,8 @@ gfc_check_c_associated (gfc_expr *c_ptr_1, gfc_expr *c_ptr_2) bool -gfc_check_c_f_pointer (gfc_expr *cptr, gfc_expr *fptr, gfc_expr *shape) +gfc_check_c_f_pointer (gfc_expr *cptr, gfc_expr *fptr, gfc_expr *shape, + gfc_expr *lower) { symbol_attribute attr; const char *msg; @@ -6156,6 +6157,44 @@ gfc_check_c_f_pointer (gfc_expr *cptr, gfc_expr *fptr, gfc_expr *shape) } } + if (lower + && !gfc_notify_std (GFC_STD_F2023, + "Unexpected LOWER argument at %L to C_F_POINTER", + &lower->where)) +return false; + + if (!shape && lower) +{ + gfc_error ("Unexpected LOWER argument at %L to C_F_POINTER " +"with scalar FPTR", +&lower->where); + return false; +} + + if (lower && !rank_check (lower, 3, 1)) +return false; + + if (lower && !type_check (lower, 3, BT_INTEGER)) +return false; + + if (lower) +{ + mpz_t size; + if (gfc_array_size (lower, &size)) + { + if (mpz_cmp_ui (size, fptr->rank) != 0) + { + mpz_clear (size); + gfc_error ( + &qu
Re: [PATCH] fortran: add optional lower arg to c_f_pointer
Oops, looks like I mess up a testcase name. Fixed in this version. On 8/6/2025 10:57 PM, Yuao Ma wrote: Hi Tobias, On 8/6/2025 4:32 PM, Tobias Burnus wrote: Hi Yuao, thanks for your patch. I have two nits: * There is no diagnostic for -std=f2018 (or lower); can you add the 'gfc_notify_std (GFC_STD_F2023' ? Done. Testcase added. * I have a minor documentation nit; current wording is at https://gcc.gnu.org/onlinedocs/gfortran/C_005fF_005fPOINTER.html Namely, ... Yuao Ma wrote: --- a/gcc/fortran/intrinsic.texi +++ b/gcc/fortran/intrinsic.texi @@ -3368,10 +3368,10 @@ Fortran 2003 and later @table @asis @item @emph{Synopsis}: -@code{CALL C_F_POINTER(CPTR, FPTR[, SHAPE])} +@code{CALL C_F_POINTER(CPTR, FPTR[, SHAPE, LOWER])} @item @emph{Description}: -@code{C_F_POINTER(CPTR, FPTR[, SHAPE])} assigns the target of the C pointer +@code{C_F_POINTER(CPTR, FPTR[, SHAPE, LOWER])} assigns the target of the C pointer @var{CPTR} to the Fortran pointer @var{FPTR} and specifies its shape. I think some wording like the following is missing: "For an array @var{FPTR}, the lower bounds are specified by @var{LOWER} if present and otherwise equal to 1." Done. @item @emph{Standard}: Fortran 2003 and later I think we should append ", with @var{LOWER} argument Fortran 2023 and later" to "Standard". Done. PS: Eventually, we should update https://gcc.gnu.org/gcc-16/ changes.html for the accumulated Fortran changes … [That's the https://gcc.gnu.org/ about.html#git ] Yes, we could summarize the work done for Fortran 2023, similar to how the Flang documentation(https://flang.llvm.org/docs/ FortranStandardsSupport.html#fortran-2023) does. The new version of this patch also fixes two minor issues, the token location in check.cc and the removal of unnecessary braces for single statements after an else clause. Thanks, Yuao From 41c6172ddc053c4c902e81d60e061bd6901a4e31 Mon Sep 17 00:00:00 2001 From: Yuao Ma Date: Wed, 6 Aug 2025 23:01:14 +0800 Subject: [PATCH] fortran: add optional lower arg to c_f_pointer This patch adds support for the optional lower argument in intrinsic c_f_pointer specified in Fortran 2023. Test cases and documentation have also been updated. gcc/fortran/ChangeLog: * check.cc (gfc_check_c_f_pointer): Check lower arg legitimacy. * intrinsic.cc (add_subroutines): Teach c_f_pointer about lower arg. * intrinsic.h (gfc_check_c_f_pointer): Add lower arg. * intrinsic.texi: Update lower arg for c_f_pointer. * trans-intrinsic.cc (conv_isocbinding_subroutine): Add logic handle lower. gcc/testsuite/ChangeLog: * gfortran.dg/c_f_pointer_shape_tests_7.f90: New test. * gfortran.dg/c_f_pointer_shape_tests_8.f90: New test. * gfortran.dg/c_f_pointer_shape_tests_9.f90: New test. Signed-off-by: Yuao Ma --- gcc/fortran/check.cc | 41 +++- gcc/fortran/intrinsic.cc | 5 +- gcc/fortran/intrinsic.h | 2 +- gcc/fortran/intrinsic.texi| 15 +-- gcc/fortran/trans-intrinsic.cc| 98 +-- .../gfortran.dg/c_f_pointer_shape_tests_7.f90 | 35 +++ .../gfortran.dg/c_f_pointer_shape_tests_8.f90 | 24 + .../gfortran.dg/c_f_pointer_shape_tests_9.f90 | 17 8 files changed, 195 insertions(+), 42 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/c_f_pointer_shape_tests_7.f90 create mode 100644 gcc/testsuite/gfortran.dg/c_f_pointer_shape_tests_8.f90 create mode 100644 gcc/testsuite/gfortran.dg/c_f_pointer_shape_tests_9.f90 diff --git a/gcc/fortran/check.cc b/gcc/fortran/check.cc index 862652683a7..e8bbf638d9f 100644 --- a/gcc/fortran/check.cc +++ b/gcc/fortran/check.cc @@ -6081,7 +6081,8 @@ gfc_check_c_associated (gfc_expr *c_ptr_1, gfc_expr *c_ptr_2) bool -gfc_check_c_f_pointer (gfc_expr *cptr, gfc_expr *fptr, gfc_expr *shape) +gfc_check_c_f_pointer (gfc_expr *cptr, gfc_expr *fptr, gfc_expr *shape, + gfc_expr *lower) { symbol_attribute attr; const char *msg; @@ -6156,6 +6157,44 @@ gfc_check_c_f_pointer (gfc_expr *cptr, gfc_expr *fptr, gfc_expr *shape) } } + if (lower + && !gfc_notify_std (GFC_STD_F2023, + "Unexpected LOWER argument at %L to C_F_POINTER", + &lower->where)) +return false; + + if (!shape && lower) +{ + gfc_error ("Unexpected LOWER argument at %L to C_F_POINTER " +"with scalar FPTR", +&lower->where); + return false; +} + + if (lower && !rank_check (lower, 3, 1)) +return false; + + if (lower && !type_check (lower, 3, BT_INTEGER)) +return false; + + if (lower) +{ + mpz_t size; + if (gfc_array_size (lower, &size)) + { + if (mpz_cmp_ui (size, fptr-
Re: [PATCH] libgfortran: add fallback for trigonometric pi-based functions
Hi Joseph, On 7/31/2025 1:45 AM, Joseph Myers wrote: On Sun, 6 Jul 2025, Yuao Ma wrote: +#ifndef HAVE_COSPI +#define HAVE_COSPI 1 +double cospi (double); + +double +cospi (double x) +{ + return cos (x * pihi_d + x * pilo_d); For reasonable results for large x you should first reduce mod 2 to the range [-1, 1] (or reduce mod 1 and keep track of the parity of the integer you subtracted, etc.), before multiplying. And for reasonable results for x near a half-integer, you need to adjust to calling sin in such a case; cospi (0.5) should be exactly 0, which you won't get from calling cos on an argument close to pi/2. See the type-generic templates in glibc for example logic. Similar issues apply to all of cospi, sinpi, tanpi, for all floating-point formats. I have improved the patch using type-generic templates from the glibc source code. The precision has been enhanced. Thanks, YuaoFrom a3d6e00f3c3a2268ea1af1ef57c381f3ed43dcae Mon Sep 17 00:00:00 2001 From: Yuao Ma Date: Wed, 20 Aug 2025 01:03:47 +0800 Subject: [PATCH] libgfortran: add fallback for trigonometric pi-based functions This patch introduces a fallback implementation for pi-based trigonometric functions, ensuring broader compatibility and robustness. The new implementation supports float, double, and long double data types. Accordingly, the test cases for r4 and r8 have been revised to reflect these changes. libgfortran/ChangeLog: * Makefile.am: Add c23_functions.c to Makefile. * Makefile.in: Regenerate. * config.h.in: Regenerate. * configure: Regenerate. * configure.ac: Check if trig-pi functions exist. * gfortran.map: Add a section for c23 functions. * libgfortran.h: Include c23 proto file. * c23_protos.h: New file. * intrinsics/c23_functions.c: New file. * intrinsics/trigpi_inc.c: New file. gcc/testsuite/ChangeLog: * gfortran.dg/dec_math_5.f90: Revise test to use fallback. --- gcc/testsuite/gfortran.dg/dec_math_5.f90 | 36 +- libgfortran/Makefile.am |4 + libgfortran/Makefile.in |8 + libgfortran/c23_protos.h | 133 +++ libgfortran/config.h.in | 63 ++ libgfortran/configure| 1010 ++ libgfortran/configure.ac | 23 + libgfortran/gfortran.map | 25 + libgfortran/intrinsics/c23_functions.c | 160 libgfortran/intrinsics/trigpi_inc.c | 147 libgfortran/libgfortran.h|1 + 11 files changed, 1592 insertions(+), 18 deletions(-) create mode 100644 libgfortran/c23_protos.h create mode 100644 libgfortran/intrinsics/c23_functions.c create mode 100644 libgfortran/intrinsics/trigpi_inc.c diff --git a/gcc/testsuite/gfortran.dg/dec_math_5.f90 b/gcc/testsuite/gfortran.dg/dec_math_5.f90 index a7ff3275236..7bcf07fce67 100644 --- a/gcc/testsuite/gfortran.dg/dec_math_5.f90 +++ b/gcc/testsuite/gfortran.dg/dec_math_5.f90 @@ -102,26 +102,26 @@ program p if (abs(c1 - 0.5) > e3) stop 39 if (abs(d1 - 0.5) > e4) stop 40 - a1 = acospi(0.5) - b1 = acospi(-0.5) + a1 = 0.5; a1 = acospi(a1) + b1 = -0.5; b1 = acospi(b1) c1 = acospi(0.5) d1 = acospi(-0.5) - if (abs(a1 - 1.0 / 3) > e1) stop 41 - if (abs(b1 - 2.0 / 3) > e2) stop 42 + if (abs(a1 - 1._4 / 3) > e1) stop 41 + if (abs(b1 - 2._8 / 3) > e2) stop 42 if (abs(c1 - 1.0 / 3) > e3) stop 43 if (abs(d1 - 2.0 / 3) > e4) stop 44 - a1 = asinpi(0.5) - b1 = asinpi(-0.5) + a1 = 0.5; a1 = asinpi(a1) + b1 = -0.5; b1 = asinpi(b1) c1 = asinpi(0.5) d1 = asinpi(-0.5) - if (abs(a1 - 1.0 / 6) > e1) stop 45 - if (abs(b1 + 1.0 / 6) > e2) stop 46 + if (abs(a1 - 1._4 / 6) > e1) stop 45 + if (abs(b1 + 1._8 / 6) > e2) stop 46 if (abs(c1 - 1.0 / 6) > e3) stop 47 if (abs(d1 + 1.0 / 6) > e4) stop 48 - a1 = atanpi(1.0) - b1 = atanpi(-1.0) + a1 = 1.0; a1 = atanpi(a1) + b1 = -1.0; b1 = atanpi(b1) c1 = atanpi(1.0) d1 = atanpi(-1.0) if (abs(a1 - 0.25) > e1) stop 49 @@ -129,8 +129,8 @@ program p if (abs(c1 - 0.25) > e3) stop 51 if (abs(d1 + 0.25) > e4) stop 52 - a1 = atan2pi(1.0, 1.0) - b1 = atan2pi(1.0, 1.0) + a1 = 1.0; a1 = atan2pi(a1, a1) + b1 = 1.0; b1 = atan2pi(b1, b1) c1 = atan2pi(1.0, 1.0) d1 = atan2pi(1.0, 1.0) if (abs(a1 - 0.25) > e1) stop 53 @@ -138,8 +138,8 @@ program p if (abs(c1 - 0.25) > e3) stop 55 if (abs(d1 - 0.25) > e4) stop 56 - a1 = cospi(1._4 / 3) - b1 = cospi(-1._8 / 3) + a1 = 1._4 / 3; a1 = cospi(a1) + b1 = -1._8 / 3; b1 = cospi(b1) c1 = cospi(4._ep / 3) d1 = cospi(-4._16 / 3) if (abs(a1 - 0.5) > e1) stop 57 @@ -147,8 +147,8 @@ program p if (abs(c1 + 0.5) > e3) stop 59 if (abs(d1 + 0.5) > e4) stop 60 - a1 = sinpi(1._4 / 6) - b1 = sinpi(-1._8 / 6) + a1 = 1._4 / 6; a1 = sinpi(a1) + b1 = -1._8 / 6; b1 = sinpi(b1) c1 = sinpi(5._ep / 6)
Re: [PATCH] fortran: implement conditional expression for fortran 2023
Hi Tobias, On Fri, Sep 5, 2025 at 11:57 PM Tobias Burnus wrote: > > Hi Yuao, > > Yuao Ma wrote: > > This time I use git format-patch. Hopefully fix the issue. > > Seem so :-) > > Do I read you patch correctly that you still want to improve it > before committing the first version? I assume so because the > changelog part of the patch is largely lacking (except for > the summary line and the mklog.sh template). > I'll improve the existing functionality and add .nil. support in later patches. I've updated the commit message. (It turns out git format-patch doesn't automatically wrap lines... I'll manually wrap the commit message to 80 characters per line in the next update.) > * * * > > > @@ -163,20 +221,22 @@ match_primary (gfc_expr **result) > ... > > + m = match_conditional (&e); > > + if (m != MATCH_YES) > > +{ > > + return m; > > +} > > Superfluous curly braces. > Fixed. > * * * > > > --- a/gcc/fortran/module.cc > > +++ b/gcc/fortran/module.cc > > @@ -3843,6 +3843,11 @@ mio_expr (gfc_expr **ep) > > > > break; > > > > +case EXPR_CONDITIONAL: > > + mio_expr (&e->value.conditional.true_expr); > > + mio_expr (&e->value.conditional.false_expr); > > + break; > > + > Shouldn't you also handle e->value.conditional.condition ? > Fixed. > I think a use case for modules would be something like: > > module m > contains > function f(n) result(str) >integer, value :: n >character(len= (n > 5 ? n : 5)) :: str >str = "" >str(1:5) = "abcef" > end > end > > Not extremely useful, but valid [using 'max(n,5)' is more > readable + compatible with <= F2018]. Still, I bet that some > real-world use case will pop up, eventually. > > Additional issue: This gives an internal compiler error, > which either should be fixed (preferred) or a 'sorry' > should be added. That's for the code path: > gfc_match_char_spec → char_len_param_value → gfc_expr_check_typed > → gfc_traverse_expr > > And the latter does not handle EXPR_CONDITIONAL. > Thanks for the tip! It turns out that not only does gfc_traverse_expr fail to handle conditional expressions, but check_restricted and gfc_check_init_expr don't either. I've added all the necessary fixes, and the test case is now in conditional_7.f90. (Not sure why some of these weren't caught by -Wswitch) > * * * > > In matchexp.cc's match_conditional: > >m = gfc_match_char ('?'); > ... > > } > > >m = gfc_match_expr (&true_expr); > >if (m != MATCH_YES) > > { > >gfc_free_expr (condition); > >return m; > > } > > > >m = gfc_match_char (':'); > >if (m != MATCH_YES) > > { > ... > >gfc_gobble_whitespace (); > > > m = match_conditional (&false_expr); > > I would add a 'gfc_gobble_whitespace ();' at '', i.e. before matching > the expression. Fixed. > Ignoring the off-by-one issue and suboptimal diagnostic in general, > cf. https://gcc.gnu.org/PR121750 > > I think it makes it a bit clearer when there are many spaced or even line > breaks as in: > > logical :: cond1, cond2 > cond1 = .false. > cond2 = .false. > print *, (cond1 ? 5 : cond2 ? & >6 : & >5.0) > end > > * * * > > > + if (true_expr->ts.type != false_expr->ts.type) > > +{ > > + gfc_error ("true_expr at %L and false_expr at %L in conditional " > > +"expression must have the same declared type", > > +&true_expr->where, &false_expr->where); > > + return false; > > +} > > + > > + if (true_expr->ts.kind != false_expr->ts.kind) > > +{ > > + gfc_error ("true_expr at %L and false_expr at %L in conditional " > > +"expression must have the same kind parameter", > > +&true_expr->where, &false_expr->where); > > + return false; > > +} > > + > > + if (true_expr->rank != false_expr->rank) > > +{ > > + gfc_error ("true_expr at %L and false_expr at %L in conditional " > > +"expression must have the same rank", > > +&true_expr->where, &false_expr->where); > > + return false; > > +} > > I wonder whether
Re: [PATCH] fortran: implement conditional expression for fortran 2023
Hi Tobias, On Fri, Sep 12, 2025 at 9:08 PM Tobias Burnus wrote: > I am unsure if this is the most appropriate place to simplify. > > I am very much in favor of deferring it. Plus: I am wondering > whether it is generally okay to just simplify it here – or whether > it has to be delayed to a later resolving stage. For constants, when > used in the executable part, it is probably okay – but in the > specification part, it likely breaks. > > If we need to defer it, then the check would be later in > resolve_omp_clauses, but we then have to save the expression > (gfc_expr*) instead of a flag ('bool', 'int …:1'). > > In any case, that's something unrelated to the patch itself. > I've got a quick question about the collapse clause in OpenMP. I've been looking at the standard, specifically section 6.4.5, and it says that the value of 'n' can't be greater than the loop nest depth. But it doesn't seem to specify what kind of expression you can use for 'n' in C++ or Fortran. >From a compiler writer's perspective, how should we handle this? Maybe use any valid integer expression that resolves correctly? Or are there other rules I might've missed? > * * * > > Subject: [PATCH] fortran: implement conditional expression for fortran 2023 > > This patch adds support for conditional expressions in Fortran 2023 for a > limited set of types (logical, numerical), and also includes limited support > for conditional arguments without `.nil.` support. > > LGTM. Thanks! > Thank you for the thorough review! I think the patch is okay for trunk now(Bootstrapped and regression tested on an x86_64 Linux)? I will wait a day or so for any additional comments before committing it. In the meantime, I will start working on the nil part. Cheers, Yuao
Re: [PATCH] fortran: implement conditional expression for fortran 2023
On Mon, Sep 1, 2025 at 6:58 PM Tobias Burnus wrote: > > Yuao Ma wrote: > > BTW, The current trunk seems to have some problems with diagnostic > > location: https://godbolt.org/z/bcrvn9xo4 > > In the last days, there were some diagnostic changes; possibly > those caused an intermittent issue? > > Otherwise, it looks ok. Namely: > > The '1' points to the space after '='in 'square = "42"', > which seems to be a reasonable location for the error: > "Error: Cannot convert CHARACTER(2) to INTEGER(4) at (1)". > > A nicer output would be '~~~1–' with '1' under '=' > and '~~~' going from 'square...' to '...42"', but the current > one seems to be sufficient. > I mostly agree it looks okay. While I'm not that meticulous about diagnostics, I believe it should point to the '=' sign or the first character of the expression. Pointing to the middle, however, seems a bit odd. When I started fixing the problem, mostly with cond-expr, I've noticed that gfc_gobble_whitespace is called in many places, such as when parsing level_1 expression or when matching ' %e'. I haven't been able to figure out which part is causing the 'where' to point to the leading whitespace of the expression. When it comes to the diagnostic printing, I don't quite understand why %C needs an offset of +1 but %L doesn't. In the first email of the series, I posted the diagnostics for the conditional expression; do those look acceptable? If not, I'll continue working on them. Aside from the diagnostics and cond-arg support, I think the patch itself is kind of complete now : ) > > PS: I think there is room for improvement in gfortran in general > regarding the column data - both were '1' points to - and adding > more ranges. Admittedly, range support is relatively new in gfortran. > > Additionally, parsing support is old and we have not always > ensured that the location points to the most sensible place > instead of just somewhere in the expression. But we should at least > try to improve both old output and when adding new features. > > Note: in dg-error/dg-warning, if you start the reg-expr string with, > e.g., '12:' it means that the '1' is expected in column 12, > i.e. the column shown for ':[.]:'. That's often > not needed but if you want to make sure that the column position > is correct, you can use it in the testcase. > > We could also add fancier testcases, checking for the '1~' > but that's more effort, which in general it not worthwhile. >
Re: [PATCH] fortran: implement conditional expression for fortran 2023
Hi Tobias, On Tue, Sep 2, 2025 at 4:57 PM Tobias Burnus wrote: > BTW: I notice '(Stripping trailing CRs from patch; use --binary to disable.)' > when applying the patch. - Spurious '\r' in committed patches should be > avoided, albeit testcases with '\r\n' are fine as we also need to support > them on Windows. (Hence, testsuite pattern that check for linebreaks also > must handle them.) This time I use git format-patch. Hopefully fix the issue. > >> Based on our previous discussion, we > >> don't need to necessarily handle this in the current patch; I just > >> wanted to highlight it. I will investigate how the argument-passing > >> logic works. > > > > Yes, as long as there is a 'sorry' instead of producing wrong code, > > it is fine. But I would like to avoid seeing runtime fails. > > BTW: When thinking about the item below, I think the following > needs to have a condition: > >/* Match an expression in parentheses. */ >if (gfc_match_char ('(') != MATCH_YES) > ... >m = match_conditional (&e); > ... >m = gfc_match_char (')'); > ... >/* Now we have the expression inside the parentheses, build the > expression pointing to it. By 7.1.7.2, any expression in > parentheses shall be treated as a data entity. */ >*result = gfc_get_parentheses (e); > > Namely, if 'e->expr_type == EXPR_CONDITIONAL, the extra parentheses > should not be added. > Agreed. Fixed. > * * * > > I think something like the following would be useful during > parsing: > > + gfc_gobble_whitespace (); > + locus saved_loc = gfc_current_locus; > m = gfc_match_expr (&true_expr); > + if (m != MATCH_YES && (m = gfc_match (".nil.")) == MATCH_YES) > +true_expr = gfc_get_..._expr (saved_loc, ... ) > if (m != MATCH_YES) > > To parse the .nil.; I think in the first round, we should reject > it unconditionally during resolving. > > I am not completely sure how to store it, but I was wondering > whether it should be added to operator (EXPR_OP), which is contrary > to the others does not take any expression. As it should only be > reachable by resolving the conditional operation, it shouldn't > harm elsewhere but needs to be handled everywhere where > conditional is processed. During parsing, it should be parsed after > other options failed while during resolution/dumping, it needs to > be handled before as, e.g., resolve_operator does not know about it > (nor does it need to, IMHO). > > * * * > > In any case, we later have to handle the three cases: > - accept it for actual arguments to OPTIONAL dummy arguments > - reject it for dummy arguments that are not OPTIONAL > - reject it when not used as actual argument. > > I wonder whether we can just reject it unconditionally, > except when explicitly processed as actual argument. > Thus, in resolve_conditional, we could rejected in general. > > When processing actual arguments, only expr == EXPR_CONDITIONAL > with either argument being .NIL. makes sense (or the first/second > being again a EXPR_CONDITIONAL). Thus, those could be explicitly > handled for the OPTIONAL check. To mark the .NIL. and to be not > diagnosed, I think we can just set the expr->do_not_resolve_again = 1 > in the argument handling - and check for it with a comment in > resolve_conditional, skipping the .NIL. diagnostic only > in that case. > > The only caveat is that there are many ways to call procedures > (functions and subroutines, specific or generic, type-bound or > procedure pointers), including intrinsic procedures, which are > resolved slightly differently. - I am not sure how many call paths > remain that need to be checked, hopefully not many! > In several cases, the normal diagnostic will do as not all take > optional arguments, but some – including intrinsic procedures - > do. > > In any case, I think we should soon handle .NIL., but first reject > it unconditionally. > Thanks for the thorough analysis! I haven't had a chance to look into the nil part yet (I'll probably get to it tomorrow), specifically from the parsing to the semantic analysis phase. Now using nil in a conditional expression would result in an immediate error. Some kind of coherent lol. > If you add a 'VALUE' (pass by value) and, possibly, remove OPTIONAL – it > should work. That enforces pass by value; if you use pass by reference > semantic, there is an issue. I think if the expression needs a pointer, > you could just move the pointer-ness on, i.e. instead of '&(cond ? a : > b)' you produce '(cond ? &a : &b)' to use pseudo-C. I wonder whether > playing with se.want_pointer is enough or whether you need more. That approach works. My current method is to handle EXPR_CONDITIONAL in gfc_conv_expr_reference by setting want_pointer=1. Then, in trans_expr, I use this want_pointer value to determine if the true_expr and false_expr need to be pointers. I've added a test case, conditional_6.f90. > I think there is no reason to not permit BT_COMPLEX as well; all of those > are numerical types, which would
Re: [PATCH] fortran: implement conditional expression for fortran 2023
Hi Tobias, I addressed some review comments and included additional feedback. On 8/22/2025 11:22 PM, Tobias Burnus wrote: Hi Yuao, Yuao Ma wrote: Hi Paul, On 7/31/2025 6:02 AM, Paul Richard Thomas wrote: That's exactly how I had a mind to do it. You beat me to it 🙁 Just get on, polish the patch and add more tests. I've updated the patch with improvements to both the functionality and test cases. It should now work well for simple scalar types. However, I've found that the issue is more complex than I initially thought. The current implementation causes an ICE with character and array types. It seems that we need to handle EXPR_CONDITIONAL every time we switch against an expr's type, and a quick search shows that there are many instances of this. I'm wondering if we could create separate, incremental patches for this. For example, in this patch, we could deny other complex types in the resolution process and gradually relax that constraint in future patches. IMHO that's fine. We use "gfc_error ("Sorry, … is not yet supported")" in gfortran for this. We might want to file Bugzilla problem report in order to track this to avoid that it falls through the cracks, unless the follow up patches are expected to arrive soon. * * * Build fails with: fortran/module.cc: In function ‘void mio_expr(gfc_expr**)’: fortran/module.cc:3773:10: error: enumeration value ‘EXPR_CONDITIONAL’ not handled in switch [-Werror=switch] 3773 | switch (e->expr_type) | ^ fortran/trans-expr.cc: In function ‘void gfc_apply_interface_mapping_to_expr(gfc_interface_mapping*, gfc_expr*)’: fortran/trans-expr.cc:5338:10: error: enumeration value ‘EXPR_CONDITIONAL’ not handled in switch [-Werror=switch] 5338 | switch (expr->expr_type) * * * But there are more, e.g. gfc_walk_subexpr Yes I am aware of that, which is why I'm a bit fear of implementing it. Every time we switch an expression type, we have to consider whether a conditional expression should be a no-op or require special handling. This would, however, be a great opportunity to dive deep into the gfortran codebase. * * * +static bool +simplify_conditional (gfc_expr *p, int type) +{ … + p->value.conditional.condition = NULL; + p->value.conditional.true_expr = NULL; + p->value.conditional.false_expr = NULL; + + if (condition->value.logical) + gfc_replace_expr (p, true_expr); + else + gfc_replace_expr (p, false_expr); This looks like a memory leak. I think you need to call gfc_free_expr for condition and for either false_expr or true_expr. Fixed. * * * Ignoring the issue of the array (segfault in scalarizer), a check that the condition is a scalar (logical) expression is missing: implicit none logical :: ll(2) integer :: A(1) integer :: B(2,3) print *, (ll ? A : B(:,1)) Possibly a copy and paste error? Because you check twice for BT_LOGICAL. Fixed and testcase added. * * * Regarding the diagnostic: 6 | print *, (ll ? A : .true. ? B(:,1) : 5) | 1 Error: True and false expressions in conditional expression must have the same rank at (1) Talking about the location, I wonder whether the expr->where should be not the beginning of the expr, i.e. 'll' for the outer and the leftmost ':' for the inner conditional, but at the (respective) '?'. Additionally, I wonder whether in + gfc_error ("True and false expressions in conditional expression " + "must have the same declared type at %L", + &expr->where); the '&expr->where' should be replaced by: gfc_get_location_range (NULL, 0, &true_expr->where,0, &false_expr- >where, 0) [gfc_get_location_range can also be used in general, which C++ does; albeit there is a small overhead doing so. The output is "~(1)~" where ~ extends from begin to end and (1) it placed at the caret position.] * * * Regarding: resolve_conditional There needs to be also a check for parameterized derived types (PDT) that their kind argument is the same. I guess this can be done later, but that would be another SORRY case. And, in general, for derived types ('TYPE', 'CLASS'), it must be the same derived, type. I think for the beginning, rejecting those are fine. It seems as we also need to check for a matching cor I am not very familiar with PDT in Fortran. I will learn about it and add the relevant parts. * * * print *, 2 > 3 ? 3 : 2 I believe this is invalid but it is currently accepted. Fortran seems to require the enclosing "(...)" in this case: Thanks for pointing that out; that was my oversight. I obviously assumed conditional expressions don't require surrounding parentheses, unlike in C++. To parse this form, maybe we need to handle it in `match_primary`? I
[pushed] MAINTAINERS: add myself to write after approval
Hi, I've added myself to the MAINTAINERS file. I also ran contrib/check-MAINTAINERS.py to confirm everything looks good. Thanks, Yuao 0001-MAINTAINERS-add-myself-to-write-after-approval.patch Description: Binary data
[PATCH] fortran: implement conditional expression for fortran 2023
Hi Tobias, I have some updates about this patch. First, some good news: 1. The patch has been bootstrapped and tested with no regressions. This was achieved by limiting the type with only one 'sorry' case. 2. The frontend parsing now considers outer parentheses. And we also have some unresolved problems. 1. First is the use of a conditional expression as an argument. Consider the following code example: it will compile but produce an incorrect result. The reason is that the current implementation passes the address of a local output to the function, rather than the address of the true_expr or false_expr. Based on our previous discussion, we don't need to necessarily handle this in the current patch; I just wanted to highlight it. I will investigate how the argument-passing logic works. program conditional_arg implicit none call five((a < 5 ? a : b)) contains subroutine five(x) integer, optional :: x if (present(x)) then x = 5 end if end subroutine five end program conditional_arg 2. The diagnostic location appears to have an off-by-one error. Consider the diagnostic for conditional_4.f90. Ideally, I'd like points (1) and (2) to reference the true_expr and false_expr, respectively. However, the current output is odd: (1) points to the '?' and (2) points to the ':', which is not what I expected. /gcc/gcc/testsuite/gfortran.dg/conditional_4.f90:13:7: 13 | i = (l1 ? 1 : -1) ! { dg-error "Condition in conditional expression must be a scalar logical" } | 1 Error: Condition in conditional expression must be a scalar logical at (1) /gcc/gcc/testsuite/gfortran.dg/conditional_4.f90:14:7: 14 | i = (i ? 1 : -1) ! { dg-error "Condition in conditional expression must be a scalar logical" } | 1 Error: Condition in conditional expression must be a scalar logical at (1) /gcc/gcc/testsuite/gfortran.dg/conditional_4.f90:15:15-19: 15 | i = (i /= 0 ? 1 : "oh no") ! { dg-error "must have the same declared type" } | 1 2 Error: true_expr at (1) and false_expr at (2) in conditional expression must have the same declared type /gcc/gcc/testsuite/gfortran.dg/conditional_4.f90:16:15-20: 16 | i = (i /= 0 ? k1 : k4) ! { dg-error "must have the same kind parameter" } | 12 Error: true_expr at (1) and false_expr at (2) in conditional expression must have the same kind parameter /gcc/gcc/testsuite/gfortran.dg/conditional_4.f90:17:15-22: 17 | i = (i /= 0 ? a_1d : a_2d) ! { dg-error "must have the same rank" } | 1 2 Error: true_expr at (1) and false_expr at (2) in conditional expression must have the same rank /gcc/gcc/testsuite/gfortran.dg/conditional_4.f90:18:8: 18 | k1 = (i /= 0 ? k1 : k1) ! { dg-error "currently only support integer, logical, and real types" } |1 Error: Sorry, conditional expression at (1) currently only support integer, logical, and real types I will investigate these issues and look forward to your further suggestions. Thanks, Yuao conditional.diff Description: Binary data
Re: [PATCH] fortran: implement conditional expression for fortran 2023
On Sat, Aug 30, 2025 at 2:02 AM Yuao Ma wrote: > 2. The diagnostic location appears to have an off-by-one error. > Consider the diagnostic for conditional_4.f90. Ideally, I'd like > points (1) and (2) to reference the true_expr and false_expr, > respectively. However, the current output is odd: (1) points to the > '?' and (2) points to the ':', which is not what I expected. BTW, The current trunk seems to have some problems with diagnostic location: https://godbolt.org/z/bcrvn9xo4
[PATCH] fortran: allow character in conditional expression
Hi all, The first cond-expr patch has been committed as r16-3848-gaf53cfeb8352b1. This patch inspired me to work on an even clearer feature: allowing cond-expr to use characters. The change is small; we just need to conditionally fill the string_length. I'd like to implement this before moving on to the .nil. part. This patch also includes a new TODO for the array "sorry case" that was missing before. Maybe OK for trunk? Thanks, Yuao 0001-fortran-allow-character-in-conditional-expression.patch Description: Binary data
Re: [PATCH] fortran: allow character in conditional expression
Hi Tobias, On Wed, Sep 17, 2025 at 3:33 AM Tobias Burnus wrote: > > Hi Yuao, > > Yuao Ma wrote: > > On Tue, Sep 16, 2025 at 5:11 PM Tobias Burnus wrote: > > PS: Already with the current code, we may run into the issue of passing > an actual argument like '(cond ? "abc" : "cdfg")' to 'class(*)' – and I > am not sure whether we handle this correctly or not. > > That is a great test case, and I apologize for not testing against the > cond-arg part. The current patch doesn't work well with cond-arg, > whether or not class(*) is involved. To avoid introducing new ICE, > I'll hold off on this patch until we can properly use character types > in both cond-expr and cond-arg. Part of the root of the problem seems > to be here: > https://github.com/gcc-mirror/gcc/blob/857c742e7bb8b24a05180e1cfee62efa417a48fe/gcc/fortran/trans-expr.cc#L10652. > When the expression's type is found to be a character, it uses > gfc_conv_string_parameter and returns immediately. Handling > EXPR_CONDITIONAL first would solve part of the issue, allowing two > character variables to be passed without a problem. However, passing > two STRING_CST still causes an ICE, failing in fold_convert_loc within > GIMPLE. I'm currently investigating why this is happening. > > If you want me to have a look as well, it helps if you sent a minimal patch > plus example. Otherwise, I quickly acknowledge that there is an issue – > without knowing much more nor digging into it. > > Regarding the const, I wonder whether something gets wrong with the type > declaration? > My apologies for not providing the full context earlier! The good news is, I've managed to figure it out : ) I've made two key updates in this version: 1. In gfc_conv_expr_reference, the previous logic handled BT_CHARACTER first. This caused a failure when we had a cond-arg with a character type, as the cond-expr logic wasn't being handled correctly. I've simply reordered the check case to handle the cond-expr first. 2. The previous gfc_conv_constant function didn't handle want_pointer, whereas gfc_conv_variable did. This explains why two string variables worked correctly, but two string constants resulted in an ICE. Both of these issues are now fixed, and I've added test cases to confirm the fixes. Thanks, Yuao 0001-fortran-allow-character-in-conditional-expression.patch Description: Binary data
Re: [PATCH] fortran: allow character in conditional expression
Hi Tobias, On Tue, Sep 16, 2025 at 5:11 PM Tobias Burnus wrote: > PS: Already with the current code, we may run into the issue of passing > an actual argument like '(cond ? "abc" : "cdfg")' to 'class(*)' – and I > am not sure whether we handle this correctly or not. That is a great test case, and I apologize for not testing against the cond-arg part. The current patch doesn't work well with cond-arg, whether or not class(*) is involved. To avoid introducing new ICE, I'll hold off on this patch until we can properly use character types in both cond-expr and cond-arg. Part of the root of the problem seems to be here: https://github.com/gcc-mirror/gcc/blob/857c742e7bb8b24a05180e1cfee62efa417a48fe/gcc/fortran/trans-expr.cc#L10652. When the expression's type is found to be a character, it uses gfc_conv_string_parameter and returns immediately. Handling EXPR_CONDITIONAL first would solve part of the issue, allowing two character variables to be passed without a problem. However, passing two STRING_CST still causes an ICE, failing in fold_convert_loc within GIMPLE. I'm currently investigating why this is happening. > > For BT_DERIVED: > > (i) The type needs to be the same – or compatible ('SEQUENCE' attribute) > > I was referring to: "7.5.2.4 Determination of derived types": > > "Data entities also have the same type if they are declared with > reference to different derived-type definitions that specify the same > type name, all have the SEQUENCE attribute or all have the BIND attribute." > > But as they need to have the same name, it is formally correct, but has > no practical effect for checking. > > Thus, feel free to ignore this mumbling. > > * * * > > > (ii) If there are kind arguments, it needs to be the same. > > gfortran names parameterized derived types (PDT) as > 'Pdt' + type name + '_ – thus for > 'type t(x); integer, kind :: x' – the names associated > with type(t(x=5)) and type(t(6)) are 'Pdtt_5' and 'Pdtt_6', > respectively. - Which implies a name check. > > However, it seems as if the simplest is to call: > gfc_compare_types and use gfc_typename in the diagnostic. > > As discussed before, I think rejecting BT_CLASS for now > makes sense, but there is no reason why BT_DERIVED including > PDT can't be enabled. > > > type t(kind,dim) >integer, kind :: kind = 4, dim = 10 >real(kind=kind) :: array(dim) > end type t > ... > type(t(8)) :: x > type(t(dim=10)) :: y > type(t(8,24)) :: z > > would be a use for the *kind type parameter* ('kind'); The kind is > known at compile time and, hence, does not actually need to be stored > in the type. The 'len' type parameter also exists. > > * * * > > BT_CLASS: > > As the declare type needs to be the same, > > subroutine sub(var, var2) >type(t) :: var >class(t) :: var2 >... (cond ? var : var2) > > seems to be fine – and would be accepted by the gfc_compare_types > check. But there are presumably further adjustments required, > Including, possibly the optimization whether BT_CLASS should > be converted to BT_DERIVED or BT_DERIVED to BT_CLASS, depending > how the conditional-expr / conditional-arg is used. Erring on > BT_CLASS is probably best, but having some optimizations might > still make sense, at least when used as dummy argument. > > * * * > > And regarding procedure/procedure pointers: > > While 'actual-arg' (R1524) permits procedure-name and proc-component-ref, > the 'conditional-arg' constitutent, i.e. 'consequent-arg' (R1528) only > permits 'expr' or 'variable' - i.e. no procedures are permitted > > * * * > > Talking about NIL, I noticed that (cond ? .nil. : .nil.) is invalid, > cf. first sentence of: > > C1540 At least one consequent in a conditional-arg shall be a consequent-arg. > If the corresponding dummy argument is not optional, .NIL. shall not appear. > I'm on the same page. When we ship cond-arg with nil, I think we need to ensure the following diagnostics are supported: 1: Reject (cond ? nil : nil) This case is straightforward and has already been implemented. We should reject any cond-expr where both branches are nil. 2: Disallow nil for Non-Optional Dummy Arguments If a dummy argument isn't declared with the OPTIONAL attribute, nil shouldn't be allowed as an actual argument. I think the ideal place for this check is in compare_actual_formal, where we can raise an error if the formal argument lacks the OPTIONAL attribute and the actual argument is nil. The challenge is figuring out how to store the information that an inner cond-expr contains a nil branch. I haven't found a clean solution yet. Perhaps adding a new field to the AST could work? You previously mentioned using do_not_resolve_again to move the check to the optional checking part. Could you elaborate more on this? 3: Restrict nil only in cond-arg nil shouldn't appear in a regular cond-expr outside of an argument list. The problem is differentiating between when gfc_resolve_expr is resolving a function argument versus an ordinary expression. The former is ca
Re: [PATCH] fortran: implement conditional expression for fortran 2023
Hi Tobias, On Mon, Sep 8, 2025 at 10:54 PM Tobias Burnus wrote: > * * * > > >>> + if (true_expr->rank != false_expr->rank) > >>> +{ > >>> + gfc_error ("true_expr at %L and false_expr at %L in conditional " > >>> +"expression must have the same rank", > >>> +&true_expr->where, &false_expr->where); > >>> + return false; > >>> +} > >> I wonder whether 'true_expr' / 'false_expr' are the best wording; Fortran > >> itself just has: > >> > >> "R1002 conditional-expr is ( scalar-logical-expr ? expr > >> [ : scalar-logical-expr ? expr ]... : expr ) > >> "C1004 Each expr of a conditional-expr shall have the same declared type, > >> kind > >> type parameters, and rank." > >> > >> and > >> > >> "Evaluation of a conditional-expr evaluates each scalar-logical-expr in > >>order, until the value of a scalar-logical-expr is true, or there are > >>no more scalar-logical-exprs. If the value of a scalar-logical-expr is > >>true, its subsequent expr is chosen; otherwise, the last expr of the > >>conditional-expr is chosen. The chosen expr is evaluated, and its > >>value is the value of the conditional expression." > >> > >> I wonder whether we shouldn't just use 'expr' without true_/false_ > >> prefix; I am especially stumbling over the underscore but also > >> "true/false expr" sounds a bit odd. > > I'm open to different naming. In the GCC C frontend, the > > build_conditional_expr function uses op1 and op2, while the Clang AST > > uses an expression array with COND, LHS, and RHS > > Note: I am not talking about the internal name - I am totally fine with > true_expr and false_expr. > > I am talking about the wording presented to the Fortran user, who > will see the compiler error message. > > Thus, "+ if (true_expr->rank != false_expr->rank)" is perfectly fine > with me. > > But instead of: > > Error: true_expr at (1) and false_expr at (2) in conditional expression must > have the same rank > > I wonder whether it would be more Fortran like (without loosing > information) to say: > > Error: expr at (1) and expr at (2) in conditional expression must have the > same rank > > > which avoids the underscore and true/false prefix. > Yeah, that makes sense. Fixed. > * * * > > BTW, just thinking about it: When simplifying, I wonder > whether we want to have a special case for >cond ? expr1 : expr1 > > Namely, condition is only known at runtime but both expressions are > identically? I think gfc_dep_compare_expr (...) == 0 does this. > (If you add it, please re-check that my claim is correct.) > It seems like a good optimization to me. However, our current implementation already seems to have this capability. When I attempt to use `i > 0 ? j : j` and view the output using dump-tree-original, I only see `i = j`. My initial assumption is that COND_EXPR will perform this transformation for simple scalar types that we currently support. I believe we will need this optimization for future complex types, so perhaps we should hold this until we actually need this part. For gfc_dep_compare_expr, yes returning 0 indicates that the two expressions are identical. > * * * > > > I'm not sure if I'm understanding you correctly here. Are you > > suggesting we treat the conditional expression as a special case for > > EXPR_OP (the ternary operator)? > > My initial thought, which isn't fully developed, is to store a nullptr > > for the nil in the AST. However, this would require a null check every > > time we use the true/false expr... > > Not as ternery operator, only for .NIL.: > > I was wondering about .NIL. as an operator that takes 0 expressions > with it - contrary to .NOT. that takes one (unary) and .AND. that takes > two (binary). - I got the idea because of the '..' it looks > very much like an operator and fits into the parser for operators. > > But I have not thought whether that has additional implications or > how many code changes it requires. > > Your idea of using nullptr is also interesting - and at a first glance, > should work. On the second thought, wouldn't this have issues (a) for > the diagnostic if gfc_expr == nullptr as there is also no expr->where > in that case. Additionally, (b) condition is known at compile time, > the might simplify to nullptr - which has issues. Having a special > case for EXPR_CONDITIONAL is fine - as it is localized, but adding > it elsewhere is a problem, especially as we already have several > null values in Fortran: > > At least null(), c_null_ptr, and c_null_funptr come into my mind. > And '(cond ? c_null_ptr : .nil.)' looks like a valid expression, > depending on the associated dummy argument. > > If you can get it working, having a nullptr for .NIL. works for me, > but I fear it won't work out and (too) many hacks are required. > > > But I am not sure whether there is a better third solution or whether > we can get one of the two ideas (.NIL. as EXPR_OP or as nullptr worki
Re: [PATCH] fortran: implement conditional expression for fortran 2023
Hi Tobias, On Wed, Sep 10, 2025 at 5:48 PM Tobias Burnus wrote: > * * * > > Regarding the patch: > > > This patch adds support for conditional expressions in Fortran 2023 for > > limited > > types (logical, numerical), and also includes limited support for > > conditional > > arguments without `.nil.` support. > > 'limited types' sounds odd – in what sense is a type such as 'real' limited? > I think it should be either 'for some types' or 'for a limited set of types'. > Fixed. > > > gcc/fortran/ChangeLog: > > > > * dump-parse-tree.cc (show_expr): Add support for EXPR_CONDITIONAL. > > * expr.cc (gfc_get_conditional_expr): Add cond-expr constructor. > > (gfc_copy_expr): Add support for EXPR_CONDITIONAL. > > (free_expr0): Ditto. > > (gfc_is_constant_expr): Ditto. > > (simplify_conditional): Ditto. > > (gfc_simplify_expr): Ditto. > > (gfc_check_init_expr): Ditto. > > (check_restricted): Ditto. > > (gfc_traverse_expr): Ditto. > > Note that you can combine all function names in a single function, i.e.: > > * expr.cc (gfc_get_conditional_expr): Add cond-expr constructor. > (gfc_copy_expr, free_expr0, gfc_is_constant_expr, > simplify_conditional, gfc_simplify_expr, gfc_check_init_expr, > check_restricted, gfc_traverse_expr): Add support for > EXPR_CONDITIONAL. > > avoiding the long list of 'Ditto.'. You still need a ditto/likewise > when you apply the same change to multiple files as in ... > > > * module.cc (mio_expr): Add support for EXPR_CONDITIONAL. > > * resolve.cc (resolve_conditional): Ditto. > > (It also makes sense to use it, when you apply the same change as > the previous entry to a function - but want to document some > other change in addition.) > Fixed. > * * * > > All in all, I think the patch is nearly ready to go. > It handles most cases fine, there are still some corner > case issues - and a wide swath of code unsupported but > rejected with an error. > > My idea would be: > > (a) To add a sorry for arrays (i.e. expr->rank > 0) - those > are a pretty common and will currently fail with an ICE. > > (b) Possibly, fix one or the other issue of the examples > below [or all of them] > > (c) Defer the rest. > > [For (b), I think the se.pre/se.post issue should really be > handled now, while the rest could be deferred.] > > > Then, in follow-up work (one or multiple patches), address: > > (A) .NIL. as a feature > > (B) Handle the remaining non-array issues of below (if any) > > Once done, you can look at adding array, character, > derived-type or polymophism support - or do a short detour > to other topics and return. > > But now to the examples that illustrate some issues: > > * * * > > The following seems to require some additional check > to yield a sorry: > > internal compiler error: in gfc_conv_variable, at fortran/trans-expr.cc:3197 > > I wonder whether we should just disallow any array for now? > I bet there are some special cases where it might work, > but so far all array examples, I tried, fail. > > > implicit none > logical :: cond > integer :: array(4) > array=[1,2,3,4] > > cond = .true. > > print *, (cond ? array : array + f() ) ) ! ICE > call sub( (cond ? array : array + f() ) ) ! ICE > contains >integer function f() > f = 1 > error stop "should not be called" >end >subroutine sub(x) > integer :: x(:) > if (any (x /= [1,2,3,4])) error stop "invalid value" > print *, x >end > end > Fixed. > * * * > > Note: Even when rejecting arrays, gfortran might produce questionable code: > > implicit none > integer, allocatable :: aa(:), bb(:) > aa = [1,2] > > print *, (aa(1) > 0 ? aa(2) : f(bb(::2))) > contains > integer function f(x) >integer :: x(*) >f = 42 > end > end > > However, it seems to work most of the time... > > * * * > > But, the following fails ('error stop'): > > implicit none > integer :: aa(2) > aa = [1,2] > > print *, (aa(1) > 0 ? aa(2) : g()) > contains > integer function g() >allocatable :: g >error stop "should not be called" >g = 3 > end > end > > for the same reason. > > > I think the solution for the two is: > > In gfc_conv_conditional_expr: > > if (true_se.pre || flase_se.pre) > gfc_add_expr_to_block (&se->pre, > fold_build3_loc (input_location, COND_EXPR, void_type_node, >condition, true_se.pre ? gfc_finish_block (&true_se.pre) : > build_empty_stmt (input_location)), > (likewise for false_se) > > And also the same for {true,false}_se.post. > You're righ - the current implementation only uses COND_EXPR for the value. But to properly handle side effects, we also need to handle the pre/post blocks. Fixed now and added a test case: conditional_8.f90. > * * * > > implicit none > integer :: i,j > > do concurrent (i=j: 5) local(j) ! Error: Variable ‘j’ referenced in > concurrent-header at (1) must not appear in LOCAL locality-spec > end do > > do concurrent (i=(j > 1 ? j : 1) : 5) loc
Re: [PATCH] fortran: allow character in conditional expression
Hi Tobias, On Mon, Sep 22, 2025 at 3:47 PM Tobias Burnus wrote: > Thanks for the updated patch. Glancing at the code, I wondered about: > > + if (expr->ts.type == BT_CHARACTER && !gfc_is_proc_ptr_comp (expr)) > > Namely: Why are procedure-pointer components handled differently? I must admit that I borrowed this code snippet from gfc_conv_variable. I am unsure if gfc_is_proc_ptr_comp is appropriate here, so I included it to keep them consistent. > I have still to understand that part, but I created a testcase that > fails in "force_constant_size, at gimplify.cc:809" > > -- > implicit none (type, external) > > character(len=:), allocatable :: str > integer :: i > > do i = 1, 3 > str = (i > 1 ? f() : "abcd") > print '(*(g0))', len(str), " >",str,"<" > end do > > contains > function f() >character(:), allocatable :: f >if (i == 1) then > f = "12345" >else > f = "" >end if > end > end > -- > > The following looks odd: > > D.4706 = (void *) &(D.4698 ? pstr.1 : "abcd"); > > The '&' looks wrong; at least 'pstr.1' is already a pointer, > for "abcd", I am currently sure how it should look in the > dump (in C/C++ is clear). gfortran produces without a > conditional: > &"abcd"[1] > > > And the following testcase fails elsewhere during > gimplification: in create_tmp_var, at gimple-expr.cc:484 > > -- > implicit none (type, external) > > character(len=:), allocatable :: str > integer :: i > character(len=5) :: str2 = "ABCDE" > > str = (i > 1 ? "abcde" : str2(1:3)) > end > -- > > Actually, while the error message is different, the > dump shows the same issue: > D.4683 = (void *) &(D.4678 ? "abcde" : &str2); > > why is there an '&' before '('? > Thanks for providing those test cases! They helped me see the real issue. The problem is that most of the existing character arguments are handled directly within gfc_conv_string_parameter. This means even when I reordered the expr types in gfc_conv_expr_reference, I was still missing cases like the ones you found. So, I've taken a different approach: for character types, I'm now handling the cond-expr directly inside gfc_conv_string_parameter. This involves recursively calling the function for both the lse and rse. The failed test cases now work with this patch. > > BTW: Using "abc"(1:i) or str(i:5) similar substrings, it is possible > to combine a constant string or const-length variable with not knowing > the length address of the in the expression - including also changing > the first character picked from the string. I have tested "abc"(1:i) pattern in my latest patch and it works as expected. Yuao PS: Sorry for the late reply! I had some urgent stuff come up that ate most of my time. : ( 0001-fortran-allow-character-in-conditional-expression.patch Description: Binary data
Re: [PATCH] libstdc++: Implement P2835R7 Expose std::atomic_ref's object address
On Fri, Oct 10, 2025 at 11:53 PM Tomasz Kaminski wrote: > On Fri, Oct 10, 2025 at 5:21 PM Yuao Ma wrote: >> >> Hi Tomasz, >> >> On Fri, Oct 10, 2025 at 10:52 PM Tomasz Kaminski wrote: >> > >> > Hi, >> > >> > Firstly, just to confirm, do you have a copyright assignment for >> > GCC in place (or are covered by a corporate assignment)? >> > >> > If not, please complete that process, or contribute under the DCO >> > terms, see https://gcc.gnu.org/contribute.html#legal >> > >> > If the patch is being contributed under the DCO, please resubmit it >> > with a Signed-off-by tag as per the first link :) >> > >> >> Actually, I'm already a committer - you can find my DCO in the >> MAINTAINERS file : ) >> I mostly contributed to gfortran as part of my GSOC project before >> this, this is my first time working on libstdc++. > > Thanks, no need for the Signed-off-by tag. >> >> >> > Secondly: >> > We will need to have two overload of address function, to preserve >> > the qualification: >> > In atomic_ref_bse, the existing one should return const _Tp* >> > #if __glibcxx_atomic_ref >= 202411L >> > _GLIBCXX_ALWAYS_INLINE constexpr const _Tp* >> > address() const noexcept >> > { return _M_ptr; } >> > #endif // __glibcxx_atomic_ref >= 202411L >> > >> > And we need to also define in in atomic_ref_base<_Tp>, this will hide one >> > from ato >> > #if __glibcxx_atomic_ref >= 202411L >> > _GLIBCXX_ALWAYS_INLINE constexpr _Tp* >> > address() const noexcept >> > { return _M_ptr; } >> > #endif // __glibcxx_atomic_ref >= 202411L >> > >> >> That was indeed my oversight. It's fixed in the new patch. >> >> > For the test I would split them into two levels: >> > template >> > void test() >> > { >> > T x(T(42)); >> > const std::atomic_ref a(x); >> > >> > static_assert(noexcept(a.address())); >> > VERIFY( std::addressof(x) == a.address() ); >> > // I would also add a test that decltype(a.address()) is T*: >> > // that will detect above issue. >> > static_assert( std::is_same_v ); >> > } >> > >> > >> > template >> > void test_cv() >> > { >> > test(); >> > test(); >> > test(); >> > test(); >> > } >> > >> > And then in main, you could just use: >> > test; >> > >> >> This really cleans up the test case! Done. > > LGTM. This still needs to be reviewed and approved by Jonathan, before it can > be merged. Thank you for the prompt review! Hi Jonathan, can you please take a look at this patch as well? Thanks! Yuao