Re: [PUSHED] nvptx: Build libgfortran with '-mfake-ptx-alloca' [PR107635]
Hi Harald, my "rant" was more about "Why would one spend time with a library meant for testing only." I totally agree that the one code base approach is one fine way to go. I didn't not want to insult anyone and apologize, if I did. Finally this discussion made me think, what it would need to have a coarray library that makes use of a GPU, i.e. to really formulate the parallel code for nvptx with coarrays and how the expressions would need to look like. Any thoughts about that? Regards, Andre Andre Vehreschild * ve...@gmx.de Am 28. Februar 2025 20:40:40 schrieb Harald Anlauf : Am 28.02.25 um 08:24 schrieb Andre Vehreschild: Hi Thomas, are you really telling me, that gfortran's coarray test library is compiled for offloading to GPU (or other SIMD processors)? Because that's what NVPTX is used for most, right? In my opinion that makes no sense, because coarrays in Fortran are used for SISD style accesses. Although the new style now extracts "kernels" for the access (which are tiny; and therefore should not perform on any GPU), I'd rather expect the caf_single library to be not compiled for NVPTX-offloading. Are there different opinions? Why not? Some people develop code using a single-source approach, so they can use (Fortran standard) coarrays, as well as directive-driven offloading. I don't see anything wrong with that. Some other compiler (NAG) enables coarrays by default(!). So is there any reason *not* to require that the caf_single library is - at least in principle - available on *all* targets? Cheers, Harald Sorry, for disturbing the NVPTX build. I wasn't aware, that it was done for caf_*. Regards, Andre
Re: [PUSHED] nvptx: Build libgfortran with '-mfake-ptx-alloca' [PR107635]
Am 28.02.25 um 08:24 schrieb Andre Vehreschild: Hi Thomas, are you really telling me, that gfortran's coarray test library is compiled for offloading to GPU (or other SIMD processors)? Because that's what NVPTX is used for most, right? In my opinion that makes no sense, because coarrays in Fortran are used for SISD style accesses. Although the new style now extracts "kernels" for the access (which are tiny; and therefore should not perform on any GPU), I'd rather expect the caf_single library to be not compiled for NVPTX-offloading. Are there different opinions? Why not? Some people develop code using a single-source approach, so they can use (Fortran standard) coarrays, as well as directive-driven offloading. I don't see anything wrong with that. Some other compiler (NAG) enables coarrays by default(!). So is there any reason *not* to require that the caf_single library is - at least in principle - available on *all* targets? Cheers, Harald Sorry, for disturbing the NVPTX build. I wasn't aware, that it was done for caf_*. Regards, Andre
Re: [Fortran, Patch, PR118730, v1] Ensure user-finalized type is referenced
Hi Harald, thanks for the review. Committed as gcc-15-7747-gc1606e383a3. Thanks again, Andre On Thu, 27 Feb 2025 21:13:08 +0100 Harald Anlauf wrote: > Hi Andre, > > Am 27.02.25 um 18:36 schrieb Andre Vehreschild: > > Hi all, > > > > attached patch fixes user defined finalizers in derived (class) types not > > getting called, when the variable declared of that type was not used in the > > current block. The patch ensures calling the finalizer by marking the > > variable referenced, if it has not been. > > > > Additionally I had to patch three testcases, because their tree-dump-scans > > did not fit anymore. In one case a variable was not used in the two others > > the counts did not match any more. > > > > Regstests ok on x86_64-pc-linux-gnu / F41. Ok for mainline? > > this LGTM. > > Thanks for the patch! > > Harald > > > Regards, > > Andre > > -- > > Andre Vehreschild * Email: vehre ad gmx dot de > -- Andre Vehreschild * Email: vehre ad gmx dot de
Re: [PATCH] Fortran: fix check for non-optional arrays passed to elemental
No problem, created PR119054 with a reproducer and some details. Cheers, Peter On Thu, 27 Feb 2025 at 20:45, Jerry D wrote: > > On 2/27/25 12:33 PM, Peter Hill wrote: > > On Thu, 27 Feb 2025 at 18:09, Jerry D wrote: > >> > >> On 2/27/25 7:38 AM, Peter Hill wrote: > >>> Dear all, > >>> > >>> The attached patch fixes an ICE in gfc_resolve_code when passing an > >>> optional array to an elemental procedure with `-pedantic` enabled. > >>> PR95446 added the original check, this patch fixes the case where the > >>> other actual argument is an array literal (or something else other > >>> than a variable). The ICE is present since 11.1, so this could be > >>> backported? > >>> > >>> Cheers, > >>> Peter > >>> > >> > >> Hi Peter, was there a PR associated with this one? > >> > >> Jerry > >> > >> --- snip --- > > > > Hi Jerry, > > > > Nope, I couldn't find one -- should I have created one first? > > > > Cheers, > > Peter > > Yes please with a testcase that illustrates the problem. This way we > capture some hsitory,if you dont mind. > > Jerry
[Fortran, Patch, PR118747, v1] Prevent double free alloc. comp. in derived type function results
Hi all,
on this regression I had to chew a longer time. Assume this Fortran:
type T
integer, allocatable:: a
end type T
result(type T) function bar()
allocate(bar%a)
end function
call foo([bar()])
That Fortran fragment was translated to something like (pseudo code):
T temp;
T arr[];
temp = bar();
arr[0]= temp;
foo(arr);
if (temp.a) { free(temp.a); temp.a= NULL;}
for (i in size(arr))
if (arr[i].a) { free(arr[i].a]; <-- double free here
arr[i].a = NULL;
}
I.e., when the derived type result of a function was used in an array
constructor that was used a function argument, then the temporary used to
evaluate the function only ones was declared to be of value. When the derived
type now had allocatable components, freeing those would be done on the value
typed temporary (here temp). But later on the array would also be freed. Now a
doulbe free occured, because the temporary variable was already freed. The
patch fixes this, by preventing the temporary when not necessary, or using a
temporary that is reference into the array, i.e., the memory freed (and marked
as such) is stored at the same location.
So after the patch this looks like this:
T *temp; // Now a pointer!
T arr[];
arr[0] = bar();
temp = &arr[0];
... Now we're safe, because freeing temp->a sets arr[0].a to NULL and the
following loop is safe.
Regtests ok on x86_64-pc-linux-gnu / F41. Ok for mainline?
Regards,
Andre
--
Andre Vehreschild * Email: vehre ad gmx dot de
From e9fd144ed6b72ddeb37c629a710bebbfba918e19 Mon Sep 17 00:00:00 2001
From: Andre Vehreschild
Date: Wed, 26 Feb 2025 14:30:13 +0100
Subject: [PATCH] Fortran: Fix regression on double free on elemental function
[PR118747]
Fix a regression were adding a temporary variable inserted a copy of the
argument to the elemental function. That copy was then later used to
free allocated memory, but the freeing was not tracked in the source
array correctly.
PR fortran/118747
gcc/fortran/ChangeLog:
* trans-array.cc (gfc_trans_array_ctor_element): Remove copy to
temporary variable.
* trans-expr.cc (gfc_conv_procedure_call): Use references to
array members instead of copies when freeing after use.
Formatting fix.
gcc/testsuite/ChangeLog:
* gfortran.dg/alloc_comp_auto_array_4.f90: New test.
---
gcc/fortran/trans-array.cc| 11 +++-
gcc/fortran/trans-expr.cc | 13 ++---
.../gfortran.dg/alloc_comp_auto_array_4.f90 | 27 +++
3 files changed, 41 insertions(+), 10 deletions(-)
create mode 100644 gcc/testsuite/gfortran.dg/alloc_comp_auto_array_4.f90
diff --git a/gcc/fortran/trans-array.cc b/gcc/fortran/trans-array.cc
index 8f76870b286..6a00d26cb2f 100644
--- a/gcc/fortran/trans-array.cc
+++ b/gcc/fortran/trans-array.cc
@@ -2002,13 +2002,10 @@ gfc_trans_array_ctor_element (stmtblock_t * pblock, tree desc,
if (expr->expr_type == EXPR_FUNCTION && expr->ts.type == BT_DERIVED
&& expr->ts.u.derived->attr.alloc_comp)
-{
- if (!VAR_P (se->expr))
- se->expr = gfc_evaluate_now (se->expr, &se->pre);
- gfc_add_expr_to_block (&se->finalblock,
- gfc_deallocate_alloc_comp_no_caf (
- expr->ts.u.derived, se->expr, expr->rank, true));
-}
+gfc_add_expr_to_block (&se->finalblock,
+ gfc_deallocate_alloc_comp_no_caf (expr->ts.u.derived,
+ tmp, expr->rank,
+ true));
if (expr->ts.type == BT_CHARACTER)
{
diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc
index ab55940638e..e619013f261 100644
--- a/gcc/fortran/trans-expr.cc
+++ b/gcc/fortran/trans-expr.cc
@@ -6999,6 +6999,12 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym,
if ((fsym && fsym->attr.value)
|| (ulim_copy && (argc == 2 || argc == 3)))
gfc_conv_expr (&parmse, e);
+ else if (e->expr_type == EXPR_ARRAY)
+ {
+ gfc_conv_expr (&parmse, e);
+ if (e->ts.type != BT_CHARACTER)
+ parmse.expr = gfc_build_addr_expr (NULL_TREE, parmse.expr);
+ }
else
gfc_conv_expr_reference (&parmse, e);
@@ -7930,11 +7936,11 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym,
/* It is known the e returns a structure type with at least one
allocatable component. When e is a function, ensure that the
function is called once only by using a temporary variable. */
- if (!DECL_P (parmse.expr))
+ if (!DECL_P (parmse.expr) && e->expr_type == EXPR_FUNCTION)
parmse.expr = gfc_evaluate_now_loc (input_location,
parmse.expr, &se->pre);
- if (fsym && fsym->attr.value)
+ if ((fsym && fsym->attr.value) || e->expr_type == EXPR_ARRAY)
tmp = parmse.expr;
else
tmp = build_fold_indirect_ref_loc (input_location,
@@ -7993,7 +7999,8 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym,
/* Scalars passed to an assumed rank argument are converted to
a descriptor. Obtain the data field before deallocating any
allocatable components. */
- if (parm_rank == 0 && GFC_DESCRIPTO
Re: [PATCH 0/4] Fortran: Improve flow of intrinsics/library documentation [PR47928]
Hi Sandra,
thanks for taking on the laborious task. I have browsed over the changes and
found:
Patch 3 in intrinsic.texi:
@@ -2071,6 +2071,9 @@ end program atomic
@cindex Atomic subroutine, ADD with fetch
@table @asis
+@item @emph{Synopsis}:
+@code{CALL ATOMIC_FETCH_ADD (ATOM, VALUE, old [, STAT])}
+
`old` should be uppercase here, too, for consistency.
Yes, I know, that is nothing you changed. I just stumbled over it and while we
are at it, let's address it.
Same for:
@@ -3074,6 +3074,9 @@ end program test_btest
@cindex pointer, C association status
@table @asis
+@item @emph{Synopsis}:
+@code{RESULT = C_ASSOCIATED(c_ptr_1[, c_ptr_2])}
With uppercasing in the following paragraph needed, too. And I vote for using
CPTR1 and CPTR2 instead.
Same here:
@@ -3177,6 +3177,9 @@ end program main
@cindex pointer, C address of pointers
@table @asis
+@item @emph{Synopsis}:
+@code{CALL C_F_PROCPOINTER(cptr, fptr)}
and here:
@@ -3235,6 +3235,9 @@ end program main
@cindex pointer, C address of procedures
@table @asis
+@item @emph{Synopsis}:
+@code{RESULT = C_FUNLOC(x)}
+
I'd say: "Ok, I'll stop." here, but that is the list of changes needed to get
the description in intrinsic.texi neat.
In part 4 of your patch, can you rephrase:
@@ -1118,6 +1114,10 @@ program test_allocated
if (.not. allocated(x)) allocate(x(i))
end program test_allocated
@end smallexample
+
+@item @emph{Standard}:
+Fortran 90 and later. Note, the @code{SCALAR=} keyword and allocatable
+scalar entities are available in Fortran 2003 and later.
@end table
to
+Fortran 90 and later; for @code{SCALAR=} keyword and allocatable
+scalar entities Fortran 2003 and later.
Just for consistency.
With these changes, ok for mainline.
Thank you very much for taking on that laborious task. My deepest respect!
Regards and thanks,
Andre
On Tue, 25 Feb 2025 20:16:51 -0700
Sandra Loosemore wrote:
> This series addresses PR 47928, a Fortran documentation issue filed back
> in 2011. Quoting from the issue:
>
> "IMHO the order of paragraphs in the intrinsics chapter of the manual
> is a bit illogical. For instance, the description comes before the
> (strangely named) syntax paragraph, so when the description refers to
> arguments it's a bit backwards. Similarly, is the version of the
> standard where the intrinsic was introduced really the second most
> important thing a user needs to know?"
>
> I'd also thought that the ordering of the syntax/description
> subheadings was confusing even before I found this issue, so I figured
> this was a worthwhile documentation fix for me to tackle.
>
> Only the first part involved manual editing; the other three patches
> were purely mechanical: search-and-replace for part 2, Emacs keyboard
> macros for parts 3 and 4 (it did take me a few attempts before I got
> the latter right). I did confirm that part 2 consisted of nothing but
> whitespace changes and that parts 3 and 4 did not add or lose any
> lines, only change their ordering. I skimmed both the patches and the
> final PDF output but I admit I didn't do a line-by-line review of the
> entire patch series.
>
> Both parts 3 and 4 are too big for the mailing list so I have attached
> the whole set as a tarball instead of posting the pieces individually.
> I'll hold off on pushing this patch series for a few days in case
> anybody else does want to review it.
>
> I can see that the documentation for the intrinsics and library
> functions still needs a lot of cleanup in addition to this patch.
> There are problems all over the place with inconsistent use of @var
> markup to describe arguments, some instances of missing @code
> markup on symbolic constants, poor formatting of the syntax/synopsis
> in the library function documentation with line breaks in random
> places, places where using a table for formatting would greatly
> improve readability (e.g. _gfortran_set_options), etc. But those are
> separate from the issue addressed by this patch series and would need
> to be addressed individually by hand.
>
> -Sandra
>
> Sandra Loosemore (4):
>Fortran: Tidy subheadings in Fortran documentation [PR47928]
>Fortran: Whitespace cleanup in documentation [PR47928]
>Fortran: Rename/move "Syntax" subheading in documentation [PR47928]
>Fortran: Move "Standard" subheading in documentation [PR47928]
>
> gcc/fortran/gfortran.texi | 387 ++--
> gcc/fortran/intrinsic.texi | 3912 ++--
> 2 files changed, 2147 insertions(+), 2152 deletions(-)
>
--
Andre Vehreschild * Email: vehre ad gmx dot de
