[PATCH, fortran] Fix simple typo in libgfortran
Hi Tobias, Thanks for your time and guidance on this GSOC project. > * This is a trivial patch but for larger patches, you need >grant the right to use your patch. There are two ways of doing so: >(a) The simple one, called "Developer Certificate of Origin" (DCO). >This requires a "Signed-off-by:" line (as added by 'git commit -s'. >(b) A copyright assignment with the Free Software Foundation (FSF). You're right. I found the information on the website stating that minor patches can omit the DCO. I have now added the "Signed-off-by:" line in the new patch. > (a) FYI - if you run for the last commit > ./contrib/gcc-changelog/git_check_commit.py -v -p or for some commit > ./contrib/gcc-changelog/git_check_commit.py -v -p Then you > see how the resulting ChangeLog will look like - it also shows errors > and warnings. (That's also used on the server.) (b) Small nit picking: Thanks for the tip! I ran the git_check_commit.py script, and it reported "OK." However, when I regenerated the ChangeLog, the function name wasn't included automatically. I've manually added the function name to the ChangeLog for now and will investigate the script's behavior later. The updated patch is attached for your review. Yuao 0001-fortran-fix-simple-typo-in-libgfortran.patch Description: 0001-fortran-fix-simple-typo-in-libgfortran.patch
[PATCH] fortran: map atand(y, x) to atan2d(y, x)
Hi all, According to the Fortran standard, atand(y, x) is equivalent to atan2d(y, x). However, the current atand(y, x) function produces an error. This patch includes the necessary intrinsic mapping, related test, and intrinsic documentation. The minor comment change in intrinsic.cc is cherry-picked from Steve's previous work. Best regards, Yuao 0001-fortran-map-atand-y-x-to-atan2d-y-x.patch Description: 0001-fortran-map-atand-y-x-to-atan2d-y-x.patch
[PATCH, fortran] Fix simple typo in libgfortran
Hi GCC Maintainers, I'm writing to express my sincere gratitude for the opportunity to participate in Google Summer of Code with GCC this year. I am very enthusiastic about this program and fully committed to making a valuable contribution and fulfilling my responsibilities. As I am relatively new to submitting patches via mailing lists, I would like to send a very simple patch primarily to familiarize myself with the correct procedure. I have reviewed the contribution guidelines on the GCC website and have tried to follow them closely. Please let me know if this approach is acceptable. I'm eager to learn and ensure my future contributions align with the project's standards. Thank you for your time and guidance. Best regards, Yuao 0001-Fix-simple-typo-in-libgfortran.patch Description: 0001-Fix-simple-typo-in-libgfortran.patch
[PATCH, fortran] Fix simple typo in libgfortran
Hi Thomas, Thanks for your quick response. > There should be a ChangeLog entry. You can generate a template by > running your patch through contrib/mklog.py, which you can then fill > out with the details. I think the ChangeLog entry is already in the patch. I used git gcc-commit-mklog, and it produced the following: ``` libgfortran/ChangeLog: * io/read.c: typo fix, explict -> explicit ``` Is this sufficient, or does it need more detail? Yuao 0001-Fix-simple-typo-in-libgfortran.patch Description: 0001-Fix-simple-typo-in-libgfortran.patch
[PATCH] fortran: map atand(y, x) to atan2d(y, x) [PR113413]
Hi Tobias, Following up on your review comments, I have updated the patch. Specifically, I have: * Added the PR number to the subject line. * Used the -b and -p options when running git gcc-commit-mklog. * Updated the intrinsic documentation as requested. Could you please take another look when you have a moment? Thanks again for your review, Yuao 0001-fortran-map-atand-y-x-to-atan2d-y-x-PR113413.patch Description: 0001-fortran-map-atand-y-x-to-atan2d-y-x-PR113413.patch
[PATCH] gcc: add trigonometric pi-based functions as gcc builtins
Hi Joseph, I have updated the patch based on your review comments. I added the newly introduced builtin to extend.texi and mentioned the PR in the commit message. Could you please take another look when you have a moment? Yuao From: Joseph Myers Sent: Thursday, May 15, 2025 0:47 To: Yuao Ma Cc: gcc-patches@gcc.gnu.org ; fort...@gcc.gnu.org ; tbur...@baylibre.com Subject: Re: [PATCH] gcc: add trigonometric pi-based functions as gcc builtins On Wed, 14 May 2025, Yuao Ma wrote: > Hi all, > > This patch adds trigonometric pi-based functions as gcc builtins: acospi, > asinpi, atan2pi, > atanpi, cospi, sinpi, and tanpi. Latest glibc already provides support for > these functions, which we plan to leverage in future gfortran implementations. > > The patch includes two test cases to verify both correct code generation and > function definition. > > If approved, I suggest committing this foundational change first. Constant > folding for these builtins will be addressed in subsequent patches. Note that either this change, or a subsequent one that makes the built-in functions do something useful, should also update extend.texi, "Library Builtins", to mention the new functions. (The text there doesn't distinguish existing C23 built-in functions, such as exp10 or roundeven, from those that are pure extensions, but addressing that is independent of adding new functions to the list. Also, I'm not sure these sentences with very long lists of functions are really the optimal way of presenting the information about such built-in functions; maybe Sandra has better ideas about how to document this, but again that's independent of adding new functions.) The commit message should reference PR c/118592 (it's not a full fix, but it's partial progress towards the full set of built-in functions / constant folding). -- Joseph S. Myers josmy...@redhat.com 0001-gcc-add-trigonometric-pi-based-functions-as-gcc-buil.patch Description: 0001-gcc-add-trigonometric-pi-based-functions-as-gcc-buil.patch
[PATCH] gcc: add trigonometric pi-based functions as gcc builtins
Hi all, This patch adds trigonometric pi-based functions as gcc builtins: acospi, asinpi, atan2pi, atanpi, cospi, sinpi, and tanpi. Latest glibc already provides support for these functions, which we plan to leverage in future gfortran implementations. The patch includes two test cases to verify both correct code generation and function definition. If approved, I suggest committing this foundational change first. Constant folding for these builtins will be addressed in subsequent patches. Best regards, Yuao 0001-gcc-add-trigonometric-pi-based-functions-as-gcc-buil.patch Description: 0001-gcc-add-trigonometric-pi-based-functions-as-gcc-buil.patch
Re: [PATCH] gcc: add trigonometric pi-based functions as gcc builtins
Hi Jakub, Thank you for your suggestion. I actually learned from your earlier patch (https://gcc.gnu.org/cgit/gcc/commit?id=7f940822) and had already planned to update tree-call-cdce.cc when handling these builtins. Your guidance is much appreciated! Best regards, Yuao From: Jakub Jelinek Sent: Saturday, May 17, 2025 22:11 To: Yuao Ma Cc: gcc-patches@gcc.gnu.org ; fort...@gcc.gnu.org ; tbur...@baylibre.com ; j...@polyomino.org.uk Subject: Re: [PATCH] gcc: add trigonometric pi-based functions as gcc builtins On Wed, May 14, 2025 at 02:22:23PM +, Yuao Ma wrote: > If approved, I suggest committing this foundational change first. Constant > folding for these builtins will be addressed in subsequent patches. Note, not just constant folding is needed, but I think the builtins should be handled in tree-call-cdce.cc (can_test_argument_range, edom_only_function, get_no_error_domain). Jakub
[PATCH] fortran: add constant input support for trig functions with half-revolutions
Sorry, the previous patch had some issues with the test case. Please refer to the updated version, which resolves the problem. From: Yuao Ma Sent: Tuesday, May 20, 2025 23:54 To: gcc-patches@gcc.gnu.org ; GCC Fortran ; tbur...@baylibre.com Subject: [PATCH] fortran: add constant input support for trig functions with half-revolutions Hi all, This patch introduces constant input support for trigonometric functions, including those involving half-revolutions. Both valid and invalid inputs have been thoroughly tested, as have mpfr versions greater than or equal to 4.2 and less than 4.2. Inspired by Steve's previous work, this patch also fixes subtle bugs revealed by newly added test cases. If this patch is merged, I plan to work on middle-end optimization support for previously added GCC built-ins and libgfortran intrinsics. Best regards, Yuao 0001-fortran-add-constant-input-support-for-trig-function.patch Description: 0001-fortran-add-constant-input-support-for-trig-function.patch
[PATCH] fortran: add constant input support for trig functions with half-revolutions
Hi all, This patch introduces constant input support for trigonometric functions, including those involving half-revolutions. Both valid and invalid inputs have been thoroughly tested, as have mpfr versions greater than or equal to 4.2 and less than 4.2. Inspired by Steve's previous work, this patch also fixes subtle bugs revealed by newly added test cases. If this patch is merged, I plan to work on middle-end optimization support for previously added GCC built-ins and libgfortran intrinsics. Best regards, Yuao 0001-fortran-add-constant-input-support-for-trig-function.patch Description: 0001-fortran-add-constant-input-support-for-trig-function.patch
[PATCH] fortran: add constant input support for trig functions with half-revolutions
Hi Tobias, Thanks for the thorough code review! I'm pretty swamped for the next couple of days, and I'll get the patch updated as you requested this weekend. > I don't understand this change. First, I don't see any reason why this > should get modified. And by modifying it, a simple "git blame" will also > point to the "wrong" commit. > > But also otherwise: it does not save the on the number of lines and it > also breaks the current pattern: > > > First line – name plus classification. > > Second line – return value, Fortran standard > > Third line – functions pointers > > Fourth line – arguments > > > I have to admit that we haven't been super consistent and the first two > lines are often also in one, but mostly (but not always) the function > pointer are on a line of their own. > > > Thus, I think we can break this pattern - but, in particular, I wouldn't > do so for the existing code, unless there is a good reason for doing so. You're absolutely right that the best way to keep changes minimal is to just rename the `*resolve*` function. The challenge is I used clang-format (with the `contrib/clang-format` file) for formatting. I know it's just a recommendation, but it's super helpful for keeping the code style consistent, and doing that manually is a pain. For the current pattern section, I think it'd be better to refactor the `add_sym*` function and use a struct to group the arguments. clang-format only touched the lines I changed in my patch, so the rest is unaffected. Does that make sense? If you're really set on the style for that part, I can definitely make the change. Yuao
[PATCH] fortran: add constant input support for trig functions with half-revolutions
Hi all, I've reverted the recent format changes, as three reviewers indicated they caused more harm than good. Are there any functional problems I need to address? Thanks, Yuao 0001-fortran-add-constant-input-support-for-trig-function.patch Description: 0001-fortran-add-constant-input-support-for-trig-function.patch
[PATCH] fortran: add constant input support for trig functions with half-revolutions
Hi Steve, Thanks for your review! I've updated the patch. > this range_check() is unneeded. Done. > As a side note, the error message is slightly misleading > (although it will not be issued). Technically, x = -1 or 1 > are allowed values, and neither is **between** -1 and 1. You're right, the original message was a bit imprecise. I've updated this and five other similar error messages in the patch for better accuracy. Yuao 0001-fortran-add-constant-input-support-for-trig-function.patch Description: 0001-fortran-add-constant-input-support-for-trig-function.patch
[PATCH] fortran: add constant input support for trig functions with half-revolutions
Hi Tobias,
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: add constant input support for trig functions with half-revolutions
Hi Tobias,
> you will notice that the PR is not recognized. The format as mentioned before
> is "PR component/number". Namely:
Thanks for the reminder! I'll use `-p` to double-check PR numbers going
forward.
> The second part is not what you are doing, you are actually changing the
> call from gfc_resolve_trigd{,2} to gfc_resolve_trig{,2}.
Done.
> > + gfc_error ("If first argument of ATAN2PI at %L is zero, then the " +
> > "second argument must not be zero", + &y->where);
> >
> I am a non-native speaker, but I think there is a "the" missing before
> "first".
You're right, I've corrected this and the two existing instances.
> BTW: If you have '(1)', you need to escape it with '\\(1\\)' or as the
> (...) don't matter, just use '.1.' as pattern. For '[...]' you need to
> make sure that [...] is not read as pattern range (such as '[a-z]'),
> i.e. use '\\\[-1, 1\\\]' (albeit it also works with only two \\).
Yeah, I forgot to use double escaping. Done.
> BTW: You could also use "intrinsic :: acospi" - which tells the compiler
> that the function is supposed to be an intrinsic.
Done. This will make the test case much cleaner!
Yuao
0001-fortran-add-constant-input-support-for-trig-function.patch
Description: 0001-fortran-add-constant-input-support-for-trig-function.patch
[PATCH] gcc: middle-end opt for trigonometric pi-based functions builtins
Hi all, This patch has been updated to conditionally fold the specified math functions only when using MPFR version 4.2.0 or newer. To accompany this change, the test case now utilizes `__builtin_constant_p` to determine whether to execute the tests against the folded functions. Let me know if this patch needs further modifications! Best regards, Yuao 0001-gcc-middle-end-opt-for-trigonometric-pi-based-functi.patch Description: 0001-gcc-middle-end-opt-for-trigonometric-pi-based-functi.patch
[PATCH] fortran: add intrinsic doc for trig functions with half-revolutions
Hi all, This patch is a follow-up to commit r16-938-ge8fdd55ec90749. In this patch, we add intrinsic documentation for the newly added trig functions with half-revolutions. We also reorder the documentation for `atand` to place it in correct alphabetical order. BR, Yuao 0001-fortran-add-intrinsic-doc-for-trig-functions-with-ha.patch Description: 0001-fortran-add-intrinsic-doc-for-trig-functions-with-ha.patch
[Patch] Fortran: gfc_simplify_{cospi,sinpi} - fix for MPFR < 4.2.0 (was: [PATCH] fortran: add constant input support for trig functions with half-revolutions)
Hi Tobias and Harald, I sincerely apologize for the breakage! I did test both preprocessor branches, but I tested against the same MPFR version (4.2.2, which is the latest on Arch Linux). Going forward, I will test against versions both above and below 4.2.0 to ensure this type of breakage doesn't happen again. Best regards, Yuao
[PATCH] gcc: middle-end opt for trigonometric pi-based functions builtins
Hi Joseph, > I don't see tests for the various special cases from Annex F (for example, > "tanpi(n) returns +0, for positive even and negative odd integers n." and > "tanpi(n) returns -0, for positive odd and negative even integers n."). > In such cases the sign of zero would need to be checked specifically (via > copysign or signbit). Thanks for the suggestions! The tests are now added; please check the attached patch. > Similar considerations may apply to other functions as well - and would be > avoided by not trying to do the optimizations with older MPFR versions. While conditionally optimizing these built-in functions based on the available MPFR version seems beneficial, it poses a testing challenge. Specifically, test cases would need a mechanism to dynamically determine the MPFR version in use. Yuao 0001-gcc-middle-end-opt-for-trigonometric-pi-based-functi.patch Description: 0001-gcc-middle-end-opt-for-trigonometric-pi-based-functi.patch
[PATCH] gcc: middle-end opt for trigonometric pi-based functions builtins
Hello world, This patch partially handled PR118592. This patch builds upon r16-710-g591d3d02664c7b and r16-711-g89935d56f768b4. It introduces middle-end optimizations, such as constant folding, for our trigonometric pi-based function built-ins. For MPFR versions older than 4.2.0, we've included our own folding functions. I've also added tests to ensure everything works as expected. Best regards, Yuao 0001-gcc-middle-end-opt-for-trigonometric-pi-based-functi.patch Description: 0001-gcc-middle-end-opt-for-trigonometric-pi-based-functi.patch
[PATCH] gcc: remove atan from edom_only_function
Hi Richard, > OK. > > Thanks, > Richard. Thanks for the quick review! Could you please help me merge this patch? I'll post the rest of the original patch soon. Thanks, Yuao
[PATCH] gcc: remove atan from edom_only_function
Hi all, This patch addresses previous review feedback by splitting the atan handling into a separate patch. This patch is part of https://gcc.gnu.org/pipermail/fortran/attachments/20250607/4a4a9cb6/attachment.obj Please take a look when you are available. Thanks! Best regards, Yuao 0001-gcc-remove-atan-from-edom_only_function.patch Description: 0001-gcc-remove-atan-from-edom_only_function.patch
[PATCH] gcc: middle-end opt for trigonometric pi-based functions builtins
Hi Dave, > but the testcases don't seem to be conditionalized on this. Would the > new tests fail if gcc is built against an insufficiently recent version > of mpfr, and is/should there be some kind of dg-requires for this, so > that the new tests gracefully are "UNSUPPORTED" on such configurations? The test case is indeed conditionalized, though in a different manner than you might expect. The condition depends on the version of MPFR we're using, and unfortunately, I haven't found a predefined macro that indicates which MPFR version GCC is linked against. I tried `gcc -E -dM - < /dev/null`, but didn't find any relevant macros. My current approach uses `__builtin_constant_p(acospi(0.5))`. If we're using a newer MPFR version, acospi will be constant-folded, causing the condition to evaluate to true and enabling the rest of the test. Otherwise, the condition will be false, and the entire test case will be omitted. Do you see any other parts of the patch that require further revision? Thanks, Yuao
[PATCH] gcc: middle-end opt for trigonometric pi-based functions builtins
Hi all, This patch, a follow-up to r16-1652-g0606d2b979f401, implements middle-end optimizations (e.g., constant folding) for our trigonometric pi-based function built-ins. This patch is part of https://gcc.gnu.org/pipermail/fortran/attachments/20250607/4a4a9cb6/attachment.obj Please take a look when you are available. Thanks! Please disregard the previous email as I forgot to attach the patch... Best regards, Yuao 0001-gcc-middle-end-opt-for-trigonometric-pi-based-functi.patch Description: 0001-gcc-middle-end-opt-for-trigonometric-pi-based-functi.patch
[PATCH] gcc: middle-end opt for trigonometric pi-based functions builtins
Hi all, This patch, a follow-up to r16-1652-g0606d2b979f401, implements middle-end optimizations (e.g., constant folding) for our trigonometric pi-based function built-ins. This patch is part of https://gcc.gnu.org/pipermail/fortran/attachments/20250607/4a4a9cb6/attachment.obj Please take a look when you are available. Thanks! Best regards, Yuao
[PATCH] gcc: middle-end opt for trigonometric pi-based functions builtins
Hi Jakub, > I think the __builtin_constant_p(acospi(0.5)) approach is usable, but would > be much better done on the lib/target-supports.exp side. > So, have foldable_pi_based_trigonometry effective target, which would test > if __builtin_constant_p(acospi(0.5)) is 1. Thanks again for your helpful advice. I've added the foldable_pi_based_trigonometry effective target and removed the conditional branch in the test case. The test results look good. Thanks, Yuao 0001-gcc-middle-end-opt-for-trigonometric-pi-based-functi.patch Description: 0001-gcc-middle-end-opt-for-trigonometric-pi-based-functi.patch
[PATCH] gcc: middle-end opt for trigonometric pi-based functions builtins
Hi Jakub, > Please don't include math.h here. Done. > And instead of this line use __builtin_acospi (0.5). > and, in dejagnu for runtime tests we prefer __builtin_abort on failure, so Done. Yuao 0001-gcc-middle-end-opt-for-trigonometric-pi-based-functi.patch Description: 0001-gcc-middle-end-opt-for-trigonometric-pi-based-functi.patch
