Re: Testing for prototypes generated from Fortran
On Sun, May 11, 2025 at 8:38 PM Harald Anlauf wrote:
>
> Hi Thomas,
>
> Am 11.05.25 um 12:51 schrieb Thomas Koenig via Gcc:
> > Hi Harald,
> >
> >> Hi Thomas,
> >>
> >> On 5/11/25 10:34, Thomas Koenig via Gcc wrote:
> >>> As PR120139 has shown (again), it is too easy to create regressions
> >>> for dumping C prototypes from Fortran. The main problem
> >>> is that there is currently no test in the testsuite.
> >>>
> >
> >> for something along this variant you can try multiline-output
> >> as in the attached sample.
> >>
> >> There may be better ways...
> >
> > Hm, this could work. This would have the disadvantage that any
> > change to the generated file would show up as "regression" on
> > testing, even if it was not relevant to the test.
> >
> > Is there maybe something along the lines of "match the compiler
> > output for a certain pattern and discard everything else" in
> > dejagnu? I tried finding it in the docs, but I didn't find anything
> > that would work.
>
> the only thing that I am aware is writing multiple dg-output's with
> suitable regexes. This allow to check that something is *present*,
> but I did not find the negation, i.e. absence of something not wanted.
I would suggest to try writing some new dejagnu harness that has
a two-stage compilation, generating the C prototypes from fortran
part of a testcase and compiling a C part of a testcase using the
prototypes. I'm not sure whether eventually you can achieve this
with
! { dg-options "-fc-prototypes" }
! { dg-additional-sources "foo.c" }
aka, whether foo.c is then reliably compiled _after_ the fortran source
Richard.
> Good luck,
> Harald
>
> > Best regards
> >
> > Thomas
> >
> >
>
[PATCH] fortran, v2: Fix up minloc/maxloc lowering [PR120191]
On Sat, May 10, 2025 at 11:21:19AM +0200, Tobias Burnus wrote:
> Namely: Similar to above, we should be able to just do:
>
> if (dim_arg->expr)
>
> I think the comment should be also updated and we
> can also get rid of the 'actual' variable for cleanup.
>
> Namely, something like the following on top of your patch (untested):
Here is an updated patch including your incremental changes.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Trying to write a testcase I've run into further issues but seems they
are on the library side, so I'll post it incrementally.
2025-05-12 Jakub Jelinek
Daniil Kochergin
Tobias Burnus
PR fortran/120191
* trans-intrinsic.cc (strip_kind_from_actual): Remove.
(gfc_conv_intrinsic_minmaxloc): Don't call strip_kind_from_actual.
Free and clear kind_arg->expr if non-NULL. Set back_arg->name to
"%VAL" instead of a loop looking for last argument. Remove actual
variable, use array_arg instead. Free and clear dim_arg->expr if
non-NULL for BT_CHARACTER cases instead of using a loop.
* gfortran.dg/pr120191_1.f90: New test.
--- gcc/fortran/trans-intrinsic.cc.jj 2025-04-22 21:26:15.772920190 +0200
+++ gcc/fortran/trans-intrinsic.cc 2025-05-10 21:25:24.541308686 +0200
@@ -4715,22 +4715,6 @@ maybe_absent_optional_variable (gfc_expr
}
-/* Remove unneeded kind= argument from actual argument list when the
- result conversion is dealt with in a different place. */
-
-static void
-strip_kind_from_actual (gfc_actual_arglist * actual)
-{
- for (gfc_actual_arglist *a = actual; a; a = a->next)
-{
- if (a && a->name && strcmp (a->name, "kind") == 0)
- {
- gfc_free_expr (a->expr);
- a->expr = NULL;
- }
-}
-}
-
/* Emit code for minloc or maxloc intrinsic. There are many different cases
we need to handle. For performance reasons we sometimes create two
loops instead of one, where the second one is much simpler.
@@ -4925,7 +4909,7 @@ gfc_conv_intrinsic_minmaxloc (gfc_se * s
tree b_if, b_else;
tree back;
gfc_loopinfo loop, *ploop;
- gfc_actual_arglist *actual, *array_arg, *dim_arg, *mask_arg, *kind_arg;
+ gfc_actual_arglist *array_arg, *dim_arg, *mask_arg, *kind_arg;
gfc_actual_arglist *back_arg;
gfc_ss *arrayss = nullptr;
gfc_ss *maskss = nullptr;
@@ -4944,8 +4928,7 @@ gfc_conv_intrinsic_minmaxloc (gfc_se * s
int n;
bool optional_mask;
- actual = expr->value.function.actual;
- array_arg = actual;
+ array_arg = expr->value.function.actual;
dim_arg = array_arg->next;
mask_arg = dim_arg->next;
kind_arg = mask_arg->next;
@@ -4954,14 +4937,16 @@ gfc_conv_intrinsic_minmaxloc (gfc_se * s
bool dim_present = dim_arg->expr != nullptr;
bool nested_loop = dim_present && expr->rank > 0;
- /* The last argument, BACK, is passed by value. Ensure that
- by setting its name to %VAL. */
- for (gfc_actual_arglist *a = actual; a; a = a->next)
+ /* Remove kind. */
+ if (kind_arg->expr)
{
- if (a->next == NULL)
- a->name = "%VAL";
+ gfc_free_expr (kind_arg->expr);
+ kind_arg->expr = NULL;
}
+ /* Pass BACK argument by value. */
+ back_arg->name = "%VAL";
+
if (se->ss)
{
if (se->ss->info->useflags)
@@ -4983,25 +4968,19 @@ gfc_conv_intrinsic_minmaxloc (gfc_se * s
}
}
- arrayexpr = actual->expr;
+ arrayexpr = array_arg->expr;
- /* Special case for character maxloc. Remove unneeded actual
- arguments, then call a library function. */
+ /* Special case for character maxloc. Remove unneeded "dim" actual
+ argument, then call a library function. */
if (arrayexpr->ts.type == BT_CHARACTER)
{
gcc_assert (expr->rank == 0);
- gfc_actual_arglist *a = actual;
- strip_kind_from_actual (a);
- while (a)
+ if (dim_arg->expr)
{
- if (a->name && strcmp (a->name, "dim") == 0)
- {
- gfc_free_expr (a->expr);
- a->expr = NULL;
- }
- a = a->next;
+ gfc_free_expr (dim_arg->expr);
+ dim_arg->expr = NULL;
}
gfc_conv_intrinsic_funcall (se, expr);
return;
--- gcc/testsuite/gfortran.dg/pr120191_1.f90.jj 2025-05-09 17:19:07.905018604
+0200
+++ gcc/testsuite/gfortran.dg/pr120191_1.f902025-05-09 17:19:07.905018604
+0200
@@ -0,0 +1,614 @@
+! PR fortran/120191
+! { dg-do run }
+
+ integer(kind=1) :: a1(10, 10, 10), b1(10)
+ integer(kind=2) :: a2(10, 10, 10), b2(10)
+ integer(kind=4) :: a4(10, 10, 10), b4(10)
+ integer(kind=8) :: a8(10, 10, 10), b8(10)
+ real(kind=4) :: r4(10, 10, 10), s4(10)
+ real(kind=8) :: r8(10, 10, 10), s8(10)
+ logical :: l1(10, 10, 10), l2(10), l3
+ l1 = .true.
+ l2 = .true.
+ l3 = .true.
+ a1 = 0
+ if (any (maxloc (a1) .ne. 1)) stop 1
+ if (any (maxloc (a1, back=.false.) .ne. 1)) stop 2
+ if (any (maxloc (a1, back=.true.) .ne. 10)) stop 3
+ if (a
[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
Re: [PATCH] fortran: map atand(y, x) to atan2d(y, x)
Hi all, hi Yuao, first, thanks for your patch - you are awesome! I believe it fixes the issue reported by Steven in problem report (PR) 113414, https://gcc.gnu.org/PR113413 Thus: * * * [Linking PR numbers] In order to correlate commits to issued (and get them automatically linked), the commit log should contain a reference. The syntax is + "PR " + space + + "/" + number, i.e. here: (tab) PR fortran/113413. However, when using mklog.py, it takes already care of the syntax: * Use '-b' to specify the bug number or * If in the first few lines of a (test) file, a PRxxx or PR comp/xxx appears, mklog.py assumes that's a PR number. Unless you already use 'PR comp/123", this doesn't automatically add the component to the commit changelog. However, if you invoke mklog.py with '-p', it queries Bugzilla – and fills in the component. Additionally, '-p' includes the bug title(s) in the template; that's a good way to cross check the number as it is easily to mistype a longer number. [PR number in the commit/email subject] Consider to add the PR number to the mail – most common is to append [PR1234] at the end (i.e. without component). However, there are no firm rules about this. Sometimes, leaving it out completely make most sense (e.g. if several bugs are fixed) or if the subject line already very long; additionally, different persons have different styles - and use a different style. * * * (Pre-existing documentation issue, but should be fixed alongside:) Looking at the documentation and comparing ATAND to ATAN, https://gcc.gnu.org/onlinedocs/gfortran/ATAND.html and https://gcc.gnu.org/onlinedocs/gfortran/ATAN.html * Synopsis should also show the two-argument version * "ifY is present,X shall be REAL" does no make sense as ATAND (contrary to ATAN) does not permit complex values * Something like "If Y is present, the result is identical to ATAN2D(Y,X). Otherwise," is missing. * "range -90 \leq \Re \atand(x) \leq 90": the 'Re' IMHO doesn't make sense as the argument must always be real. * * * [I have still to look at the rest of the patch, but at a glance it looks fine.] Thanks again for your patch! Tobias
Re: Testing for prototypes generated from Fortran
Hi Harald, Hi Thomas, On 5/11/25 10:34, Thomas Koenig via Gcc wrote: As PR120139 has shown (again), it is too easy to create regressions for dumping C prototypes from Fortran. The main problem is that there is currently no test in the testsuite. for something along this variant you can try multiline-output as in the attached sample. There may be better ways... Hm, this could work. This would have the disadvantage that any change to the generated file would show up as "regression" on testing, even if it was not relevant to the test. Is there maybe something along the lines of "match the compiler output for a certain pattern and discard everything else" in dejagnu? I tried finding it in the docs, but I didn't find anything that would work. Best regards Thomas
Testing for prototypes generated from Fortran
As PR120139 has shown (again), it is too easy to create regressions for dumping C prototypes from Fortran. The main problem is that there is currently no test in the testsuite. So, what to do? I see several possibilities: 1a) Change the relevant options so that they optionally create a file (something like -fc-prototypes=filename.h) and then scan for the presence/absence of patterns in that file. (Is it OK just to open and close a file anywhere in the front end?). The generated files should then be removed afterwards (how?). 1b) Instead of checking for patterns in the output create a header file which is then included in a C file for testing. Same questions as above, with the additional question on how to ensure that compilation of the C file only happens after the header file has been generated. 2) Dump to standard output and check for the presence of certain regexps, ignoring anything else. Again, this is something I don't know how to do. This is really something I would like to have fixed for gcc-16. Other ideas? Comments? Suggestions? Best regards Thomas
Re: Testing for prototypes generated from Fortran
Hi Thomas,
On 5/11/25 10:34, Thomas Koenig via Gcc wrote:
As PR120139 has shown (again), it is too easy to create regressions
for dumping C prototypes from Fortran. The main problem
is that there is currently no test in the testsuite.
So, what to do? I see several possibilities:
1a) Change the relevant options so that they optionally
create a file (something like -fc-prototypes=filename.h)
and then scan for the presence/absence of patterns in that
file. (Is it OK just to open and close a file anywhere
in the front end?). The generated files should then be
removed afterwards (how?).
1b) Instead of checking for patterns in the output create a header file
which is then included in a C file for testing. Same questions
as above, with the additional question on how to ensure that
compilation of the C file only happens after the header file
has been generated.
2) Dump to standard output and check for the presence of certain
regexps, ignoring anything else. Again, this is something I don't
know how to do.
for something along this variant you can try multiline-output
as in the attached sample.
There may be better ways...
Cheers,
Harald
This is really something I would like to have fixed for gcc-16.
Other ideas? Comments? Suggestions?
Best regards
Thomas
! { dg-options "-fc-prototypes" }
subroutine foo (x, k) bind(c)
integer, value :: k
real :: x(k)
end
#if 0
{ dg-begin-multiline-output "" }
#include
#ifdef __cplusplus
#include
#define __GFORTRAN_FLOAT_COMPLEX std::complex
#define __GFORTRAN_DOUBLE_COMPLEX std::complex
#define __GFORTRAN_LONG_DOUBLE_COMPLEX std::complex
extern "C" {
#else
#define __GFORTRAN_FLOAT_COMPLEX float _Complex
#define __GFORTRAN_DOUBLE_COMPLEX double _Complex
#define __GFORTRAN_LONG_DOUBLE_COMPLEX long double _Complex
#endif
void foo (float x /* WARNING: non-interoperable KIND */ , int k /* WARNING: non-interoperable KIND */ );
#ifdef __cplusplus
}
#endif
{ dg-end-multiline-output "" }
#endif
Re: Testing for prototypes generated from Fortran
Hi Thomas, Am 11.05.25 um 12:51 schrieb Thomas Koenig via Gcc: Hi Harald, Hi Thomas, On 5/11/25 10:34, Thomas Koenig via Gcc wrote: As PR120139 has shown (again), it is too easy to create regressions for dumping C prototypes from Fortran. The main problem is that there is currently no test in the testsuite. for something along this variant you can try multiline-output as in the attached sample. There may be better ways... Hm, this could work. This would have the disadvantage that any change to the generated file would show up as "regression" on testing, even if it was not relevant to the test. Is there maybe something along the lines of "match the compiler output for a certain pattern and discard everything else" in dejagnu? I tried finding it in the docs, but I didn't find anything that would work. the only thing that I am aware is writing multiple dg-output's with suitable regexes. This allow to check that something is *present*, but I did not find the negation, i.e. absence of something not wanted. Good luck, Harald Best regards Thomas
Re: Testing for prototypes generated from Fortran
On Sun, May 11, 2025 at 10:34:11 +0200, Thomas Koenig via Gcc wrote: > 2) Dump to standard output and check for the presence of certain > regexps, ignoring anything else. Again, this is something I don't > know how to do. This is…fraught with peril and fiddliness. If the test can stomach some C++, you can use `` to `static_assert` on declaration properties. See this test as an example of how we ensure ABI stability as far as the header is concerned: https://gitlab.kitware.com/paraview/catalyst/-/blob/master/tests/abi_tests/test_catalyst_abi.cpp --Ben
