[PATCH, fortran] Fix simple typo in libgfortran

2025-05-10 Thread Yuao Ma
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)

2025-05-11 Thread Yuao Ma
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

2025-05-10 Thread Yuao Ma
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

2025-05-10 Thread Yuao Ma
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

2025-07-08 Thread Yuao Ma




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

2025-07-03 Thread Yuao Ma

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

2025-07-05 Thread Yuao Ma

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

2025-07-06 Thread Yuao Ma

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

2025-07-05 Thread Yuao Ma




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

2025-07-05 Thread Yuao Ma

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

2025-07-02 Thread Yuao Ma

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

2025-06-27 Thread Yuao Ma
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

2025-06-27 Thread Yuao Ma
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

2025-06-27 Thread Yuao Ma
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

2025-05-23 Thread Yuao Ma
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)

2025-05-28 Thread Yuao Ma
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

2025-06-02 Thread Yuao Ma
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

2025-06-01 Thread Yuao Ma
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

2025-06-24 Thread Yuao Ma
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

2025-06-23 Thread Yuao Ma
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

2025-06-26 Thread Yuao Ma
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

2025-06-26 Thread Yuao Ma
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

2025-06-26 Thread Yuao Ma
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

2025-07-26 Thread Yuao Ma

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

2025-07-26 Thread Yuao Ma

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

2025-07-26 Thread Yuao Ma
 ("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

2025-07-27 Thread Yuao Ma

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

2025-07-27 Thread Yuao Ma



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

2025-07-27 Thread Yuao Ma




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

2025-08-03 Thread Yuao Ma

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

2025-08-03 Thread Yuao Ma




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

2025-08-04 Thread Yuao Ma

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

2025-08-05 Thread Yuao Ma




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

2025-08-05 Thread Yuao Ma

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

2025-07-30 Thread Yuao Ma

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

2025-07-29 Thread Yuao Ma




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

2025-07-27 Thread Yuao Ma



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

2025-08-06 Thread Yuao Ma




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

2025-08-07 Thread Yuao Ma

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

2025-08-04 Thread Yuao Ma




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]

2025-05-12 Thread Yuao Ma
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

2025-05-14 Thread Yuao Ma
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

2025-05-14 Thread Yuao Ma
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

2025-05-18 Thread Yuao Ma
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

2025-05-20 Thread Yuao Ma
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

2025-05-20 Thread Yuao Ma
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

2025-05-21 Thread Yuao Ma
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

2025-05-27 Thread Yuao Ma
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

2025-05-24 Thread Yuao Ma
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

2025-05-28 Thread Yuao Ma
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

2025-06-07 Thread Yuao Ma
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

2025-06-06 Thread Yuao Ma
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

2025-08-14 Thread Yuao Ma

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

2025-08-06 Thread Yuao Ma

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

2025-08-06 Thread Yuao Ma

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

2025-08-19 Thread Yuao Ma

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

2025-09-06 Thread Yuao Ma
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

2025-09-13 Thread Yuao Ma
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

2025-09-03 Thread Yuao Ma
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

2025-09-03 Thread Yuao Ma
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

2025-08-25 Thread Yuao Ma

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

2025-08-28 Thread Yuao Ma
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

2025-08-29 Thread Yuao Ma
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

2025-08-30 Thread Yuao Ma
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

2025-09-15 Thread Yuao Ma
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

2025-09-17 Thread Yuao Ma
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

2025-09-18 Thread Yuao Ma
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

2025-09-17 Thread Yuao Ma
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

2025-09-20 Thread Yuao Ma
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

2025-09-29 Thread Yuao Ma
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

2025-10-11 Thread Yuao Ma
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