Re: [Patch, Fortran, committed] Allow ref'ing PDT's len() in parameter-initializer [PR102003]
Hi all, hi Harald,
thanks for the review. I choose to use gfc_replace_expr() and retested.
Everything went fine now.
Also thank you clarifying the pdt as a component in a derived type and that
that is still a bug and I didn't do it wrong.
I have pushed the attached patch seconds ago.
Thanks for your help,
Andre
On Tue, 11 Jul 2023 22:24:37 +0200
Harald Anlauf wrote:
> Hi Andre,
>
> this looks much better now!
>
> This looks mostly good to me, except for a typo in the testcase:
>
> + if (p% ci% len /= 42) stop 4
>
> There is no component "ci", only "c". The testsuite would fail.
>
> Regarding the memleak: replacing
>
>// TODO: Fix leaking expr tmp, when simplify is done twice.
>tmp = gfc_copy_expr (*newp);
>
> by
>
>if (inquiry->next)
> {
> gfc_free_expr (tmp);
> tmp = gfc_copy_expr (*newp);
> }
>
> or rather
>
>if (inquiry->next)
> gfc_replace_expr (tmp, *newp);
>
> at least shrinks the leak a bit. (Untested otherwise).
>
> OK with one of the above changes (provided it survives regtesting).
>
> Thanks for the patch!
>
> Harald
>
>
> Am 11.07.23 um 18:23 schrieb Andre Vehreschild via Gcc-patches:
> > Hi Harald,
> >
> > attached is a new version of the patch. This now also respects inquiry-LEN.
> > Btw, there is a potential memory leak in the simplify for inquiry
> > functions. I have added a note into the code.
> >
> > I tried to use a pdt within a derived type as a component. Is that not
> > allowed by the standard? I know, I could hunt in the standard for it, but
> > when someone knows out of his head, he could greatly help me out.
> >
> > Regtests ok on x86_64-linux-gnu/F37.
> >
> > Regards,
> > Andre
> >
> > On Mon, 10 Jul 2023 20:55:29 +0200
> > Harald Anlauf wrote:
> >
> >> Hi Andre,
> >>
> >> thanks for looking into this!
> >>
> >> While it fixes the original PR, here is a minor extension of the
> >> testcase that ICEs here with your patch:
> >>
> >> program pr102003
> >> type pdt(n)
> >>integer, len :: n = 8
> >>character(len=n) :: c
> >> end type pdt
> >> type(pdt(42)) :: p
> >> integer, parameter :: m = len (p% c)
> >> integer, parameter :: n = p% c% len
> >>
> >> if (m /= 42) stop 1
> >> if (len (p% c) /= 42) stop 2
> >> print *, p% c% len ! OK
> >> if (p% c% len /= 42) stop 3 ! OK
> >> print *, n ! ICE
> >> end
> >>
> >> I get:
> >>
> >> pdt_33.f03:14:27:
> >>
> >> 14 | integer, parameter :: n = p% c% len
> >> | 1
> >> Error: non-constant initialization expression at (1)
> >> pdt_33.f03:20:31:
> >>
> >> 20 | print *, n ! ICE
> >> | 1
> >> internal compiler error: tree check: expected record_type or union_type
> >> or qual_union_type, have integer_type in gfc_conv_component_ref, at
> >> fortran/trans-expr.cc:2757
> >> 0x84286c tree_check_failed(tree_node const*, char const*, int, char
> >> const*, ...)
> >> ../../gcc-trunk/gcc/tree.cc:8899
> >> 0xa6d6fb tree_check3(tree_node*, char const*, int, char const*,
> >> tree_code, tree_code, tree_code)
> >> ../../gcc-trunk/gcc/tree.h:3617
> >> 0xa90847 gfc_conv_component_ref(gfc_se*, gfc_ref*)
> >> ../../gcc-trunk/gcc/fortran/trans-expr.cc:2757
> >> 0xa91bbc gfc_conv_variable
> >> ../../gcc-trunk/gcc/fortran/trans-expr.cc:3137
> >> 0xaa8e9c gfc_conv_expr(gfc_se*, gfc_expr*)
> >> ../../gcc-trunk/gcc/fortran/trans-expr.cc:9594
> >> 0xaa92ae gfc_conv_expr_reference(gfc_se*, gfc_expr*)
> >> ../../gcc-trunk/gcc/fortran/trans-expr.cc:9713
> >> 0xad67f6 gfc_trans_transfer(gfc_code*)
> >> ../../gcc-trunk/gcc/fortran/trans-io.cc:2607
> >> 0xa43cb7 trans_code
> >> ../../gcc-trunk/gcc/fortran/trans.cc:2449
> >> 0xad37c6 build_dt
> >> ../../gcc-trunk/gcc/fortran/trans-io.cc:2051
> >> 0xa43cd7 trans_code
> >> ../../gcc-trunk/gcc/fortran/trans.cc:2421
> >> 0xa84711 gfc_generate_function_code(gfc_namespace*)
> >> ../../gcc-trunk/gcc/fortran/trans-decl.cc:7762
> >> 0x9d9ca7 translate_all_program_u
[Patch, Fortran] Allow ref'ing PDT's len() in parameter-initializer [PR102003]
Hi all,
while browsing the pdt meta-bug I came across 102003 and thought to myself:
Well, that one is easy. How foolish of me...
Anyway, the solution attached prevents a pdt_len (or pdt_kind) expression in a
function call (e.g. len() or kind()) to mark the whole expression as a pdt one.
The second part of the patch in simplify.cc then takes care of either generating
the correct component ref or when a constant expression (i.e.
gfc_init_expr_flag is set) is required to look this up from the actual symbol
(not from the type, because there the default value is stored).
Regtested ok on x86_64-linux-gnu/Fedora 37.
Regards,
Andre
--
Andre Vehreschild * Email: vehre ad gmx dot de
gcc/fortran/ChangeLog:
* expr.cc (gfc_match_init_expr): Prevent PDT analysis for function
calls.
* simplify.cc (gfc_simplify_len): Replace len() of PDT with pdt
component ref or constant.
gcc/testsuite/ChangeLog:
* gfortran.dg/pdt_33.f03: New test.
diff --git a/gcc/fortran/expr.cc b/gcc/fortran/expr.cc
index e418f1f3301..fb6eb76cda7 100644
--- a/gcc/fortran/expr.cc
+++ b/gcc/fortran/expr.cc
@@ -3229,7 +3229,7 @@ gfc_match_init_expr (gfc_expr **result)
return m;
}
- if (gfc_derived_parameter_expr (expr))
+ if (expr->expr_type != EXPR_FUNCTION && gfc_derived_parameter_expr (expr))
{
*result = expr;
gfc_init_expr_flag = false;
diff --git a/gcc/fortran/simplify.cc b/gcc/fortran/simplify.cc
index 81680117f70..8fb453d0a54 100644
--- a/gcc/fortran/simplify.cc
+++ b/gcc/fortran/simplify.cc
@@ -4580,19 +4580,54 @@ gfc_simplify_len (gfc_expr *e, gfc_expr *kind)
return range_check (result, "LEN");
}
else if (e->expr_type == EXPR_VARIABLE && e->ts.type == BT_CHARACTER
- && e->symtree->n.sym
- && e->symtree->n.sym->ts.type != BT_DERIVED
- && e->symtree->n.sym->assoc && e->symtree->n.sym->assoc->target
- && e->symtree->n.sym->assoc->target->ts.type == BT_DERIVED
- && e->symtree->n.sym->assoc->target->symtree->n.sym
- && UNLIMITED_POLY (e->symtree->n.sym->assoc->target->symtree->n.sym))
-
-/* The expression in assoc->target points to a ref to the _data component
- of the unlimited polymorphic entity. To get the _len component the last
- _data ref needs to be stripped and a ref to the _len component added. */
-return gfc_get_len_component (e->symtree->n.sym->assoc->target, k);
- else
-return NULL;
+ && e->symtree->n.sym)
+{
+ if (e->symtree->n.sym->ts.type != BT_DERIVED
+ && e->symtree->n.sym->assoc && e->symtree->n.sym->assoc->target
+ && e->symtree->n.sym->assoc->target->ts.type == BT_DERIVED
+ && e->symtree->n.sym->assoc->target->symtree->n.sym
+ && UNLIMITED_POLY (e->symtree->n.sym->assoc->target->symtree
+->n.sym))
+ /* The expression in assoc->target points to a ref to the _data
+ component of the unlimited polymorphic entity. To get the _len
+ component the last _data ref needs to be stripped and a ref to the
+ _len component added. */
+ return gfc_get_len_component (e->symtree->n.sym->assoc->target, k);
+ else if (e->symtree->n.sym->ts.type == BT_DERIVED
+ && e->ref && e->ref->type == REF_COMPONENT
+ && e->ref->u.c.component->attr.pdt_string
+ && e->ref->u.c.component->ts.type == BT_CHARACTER
+ && e->ref->u.c.component->ts.u.cl->length)
+ {
+ if (gfc_init_expr_flag)
+ {
+ /* The actual length of a pdt is in its components. In the
+ initializer of the current ref is only the default value.
+ Therefore traverse the chain of components and pick the correct
+ one's initializer expressions. */
+ for (gfc_component *comp = e->symtree->n.sym->ts.u.derived
+ ->components; comp != NULL; comp = comp->next)
+ {
+ if (!strcmp (comp->name, e->ref->u.c.component->ts.u.cl
+ ->length->symtree->name))
+ return gfc_copy_expr (comp->initializer);
+ }
+ }
+ else
+ {
+ gfc_expr *len_expr = gfc_copy_expr (e);
+ gfc_free_ref_list (len_expr->ref);
+ len_expr->ref = NULL;
+ gfc_find_component (len_expr->symtree->n.sym->ts.u.derived, e->ref
+ ->u.c.component->ts.u.cl->length->symtree
+ ->name,
+ false, true, &len_expr->ref);
+ len_expr->ts = len_expr->ref->u.c.component->ts;
+ return len_expr;
+ }
+ }
+}
+ return NULL;
}
diff --git a/gcc/testsuite/gfortran.dg/pdt_33.f03 b/gcc/testsuite/gfortran.dg/pdt_33.f03
new file mode 100644
index 000..c12bd9b411c
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/pdt_33.f03
@@ -0,0 +1,18 @@
+! { dg-do run }
+!
+! Test the fix for PR102003, where len parameters where not returned as constants.
+!
+! Contributed by Harald Anlauf
+!
+program pr102003
+ type pdt(n)
+ integer, len :: n = 8
+ character(len=n) :: c
+ end type pdt
+ type(pdt(42)) :: p
+ integer, parameter :: m = len (p% c)
+
+ if (m /= 42) stop 1
+ if (len (p% c) /= 42) stop 2
+end
+
Re: [Patch, Fortran] Allow ref'ing PDT's len() in parameter-initializer [PR102003]
Hi Harald, I do get why this happens. I still don't get why I have to do this 'optimization' manually. I mean, this rewriting of expressions is needed in more than one location and most probably already present somewhere. So who can point me in the right direction? Regards, Andre Andre Vehreschild
Re: [Patch, Fortran] Allow ref'ing PDT's len() in parameter-initializer [PR102003]
Hi Harald, attached is a new version of the patch. This now also respects inquiry-LEN. Btw, there is a potential memory leak in the simplify for inquiry functions. I have added a note into the code. I tried to use a pdt within a derived type as a component. Is that not allowed by the standard? I know, I could hunt in the standard for it, but when someone knows out of his head, he could greatly help me out. Regtests ok on x86_64-linux-gnu/F37. Regards, Andre On Mon, 10 Jul 2023 20:55:29 +0200 Harald Anlauf wrote: > Hi Andre, > > thanks for looking into this! > > While it fixes the original PR, here is a minor extension of the > testcase that ICEs here with your patch: > > program pr102003 >type pdt(n) > integer, len :: n = 8 > character(len=n) :: c >end type pdt >type(pdt(42)) :: p >integer, parameter :: m = len (p% c) >integer, parameter :: n = p% c% len > >if (m /= 42) stop 1 >if (len (p% c) /= 42) stop 2 >print *, p% c% len ! OK >if (p% c% len /= 42) stop 3 ! OK >print *, n ! ICE > end > > I get: > > pdt_33.f03:14:27: > > 14 | integer, parameter :: n = p% c% len >| 1 > Error: non-constant initialization expression at (1) > pdt_33.f03:20:31: > > 20 | print *, n ! ICE >| 1 > internal compiler error: tree check: expected record_type or union_type > or qual_union_type, have integer_type in gfc_conv_component_ref, at > fortran/trans-expr.cc:2757 > 0x84286c tree_check_failed(tree_node const*, char const*, int, char > const*, ...) > ../../gcc-trunk/gcc/tree.cc:8899 > 0xa6d6fb tree_check3(tree_node*, char const*, int, char const*, > tree_code, tree_code, tree_code) > ../../gcc-trunk/gcc/tree.h:3617 > 0xa90847 gfc_conv_component_ref(gfc_se*, gfc_ref*) > ../../gcc-trunk/gcc/fortran/trans-expr.cc:2757 > 0xa91bbc gfc_conv_variable > ../../gcc-trunk/gcc/fortran/trans-expr.cc:3137 > 0xaa8e9c gfc_conv_expr(gfc_se*, gfc_expr*) > ../../gcc-trunk/gcc/fortran/trans-expr.cc:9594 > 0xaa92ae gfc_conv_expr_reference(gfc_se*, gfc_expr*) > ../../gcc-trunk/gcc/fortran/trans-expr.cc:9713 > 0xad67f6 gfc_trans_transfer(gfc_code*) > ../../gcc-trunk/gcc/fortran/trans-io.cc:2607 > 0xa43cb7 trans_code > ../../gcc-trunk/gcc/fortran/trans.cc:2449 > 0xad37c6 build_dt > ../../gcc-trunk/gcc/fortran/trans-io.cc:2051 > 0xa43cd7 trans_code > ../../gcc-trunk/gcc/fortran/trans.cc:2421 > 0xa84711 gfc_generate_function_code(gfc_namespace*) > ../../gcc-trunk/gcc/fortran/trans-decl.cc:7762 > 0x9d9ca7 translate_all_program_units > ../../gcc-trunk/gcc/fortran/parse.cc:6929 > 0x9d9ca7 gfc_parse_file() > ../../gcc-trunk/gcc/fortran/parse.cc:7235 > 0xa40a1f gfc_be_parse_file > ../../gcc-trunk/gcc/fortran/f95-lang.cc:229 > > The fortran-dump confirms that n is not simplified to a constant. > So while you're at it, do you also see a solution to this variant? > > Harald > > > Am 10.07.23 um 17:48 schrieb Andre Vehreschild via Gcc-patches: > > Hi all, > > > > while browsing the pdt meta-bug I came across 102003 and thought to myself: > > Well, that one is easy. How foolish of me... > > > > Anyway, the solution attached prevents a pdt_len (or pdt_kind) expression > > in a function call (e.g. len() or kind()) to mark the whole expression as a > > pdt one. The second part of the patch in simplify.cc then takes care of > > either generating the correct component ref or when a constant expression > > (i.e. gfc_init_expr_flag is set) is required to look this up from the > > actual symbol (not from the type, because there the default value is > > stored). > > > > Regtested ok on x86_64-linux-gnu/Fedora 37. > > > > Regards, > > Andre > > -- > > Andre Vehreschild * Email: vehre ad gmx dot de > -- Andre Vehreschild * Email: vehre ad gmx dot de gcc/fortran/ChangeLog: * expr.cc (find_inquiry_ref): Replace len of pdt_string by constant. (gfc_match_init_expr): Prevent PDT analysis for function calls. (gfc_pdt_find_component_copy_initializer): Get the initializer value for given component. * gfortran.h (gfc_pdt_find_component_copy_initializer): New function. * simplify.cc (gfc_simplify_len): Replace len() of PDT with pdt component ref or constant. gcc/testsuite/ChangeLog: * gfortran.dg/pdt_33.f03: New test. diff --git a/gcc/fortran/expr.cc b/gcc/fortran/expr.cc index e418f1f3301..23324517ff2 100644 --- a/gcc/fortran/
[PR103970, Fortran, Coarray] Multi-image co_broadcast of derived type with allocatable components fails^
Hi all,
attached patch fixes wrong code generation when broadcasting a derived type
containing allocatable and non-allocatable scalars. Furthermore does it prevent
broadcasting of coarray-tokens, which are always local this_image. Thus having
them on a different image makes no sense.
Bootstrapped and regtested ok on x86_64-linux/F35.
Ok, for trunk and backport to 12 and 11-branch after decent time?
I perceived that 12 is closed for this kind of bugfix, therefore asking ok for
13.
Regards,
Andre
--
Andre Vehreschild * Email: vehre ad gmx dot de
gcc/fortran/ChangeLog:
2022-01-24 Andre Vehreschild
PR fortran/103790
* trans-array.cc (structure_alloc_comps): Prevent descriptor
stacking for non-array data; do not broadcast caf-tokens.
* trans-intrinsic.cc (conv_co_collective): Prevent generation
of unused descriptor.
gcc/testsuite/ChangeLog:
2022-01-24 Andre Vehreschild
PR fortran/103790
* gfortran.dg/coarray_collectives_18.f90: New test.
diff --git a/gcc/fortran/trans-array.cc b/gcc/fortran/trans-array.cc
index 2f0c8a4d412..1234932aaff 100644
--- a/gcc/fortran/trans-array.cc
+++ b/gcc/fortran/trans-array.cc
@@ -9102,6 +9102,10 @@ structure_alloc_comps (gfc_symbol * der_type, tree decl,
continue;
}
+ /* Do not broadcast a caf_token. These are local to the image. */
+ if (attr->caf_token)
+ continue;
+
add_when_allocated = NULL_TREE;
if (cmp_has_alloc_comps
&& !c->attr.pointer && !c->attr.proc_pointer)
@@ -9134,10 +9138,13 @@ structure_alloc_comps (gfc_symbol * der_type, tree decl,
if (attr->dimension)
{
tmp = gfc_get_element_type (TREE_TYPE (comp));
- ubound = gfc_full_array_size (&tmpblock, comp,
- c->ts.type == BT_CLASS
- ? CLASS_DATA (c)->as->rank
- : c->as->rank);
+ if (!GFC_DESCRIPTOR_TYPE_P (TREE_TYPE (comp)))
+ ubound = GFC_TYPE_ARRAY_SIZE (TREE_TYPE (comp));
+ else
+ ubound = gfc_full_array_size (&tmpblock, comp,
+ c->ts.type == BT_CLASS
+ ? CLASS_DATA (c)->as->rank
+ : c->as->rank);
}
else
{
@@ -9145,26 +9152,36 @@ structure_alloc_comps (gfc_symbol * der_type, tree decl,
ubound = build_int_cst (gfc_array_index_type, 1);
}
- cdesc = gfc_get_array_type_bounds (tmp, 1, 0, &gfc_index_one_node,
-&ubound, 1,
-GFC_ARRAY_ALLOCATABLE, false);
+ /* Treat strings like arrays. Or the other way around, do not
+ * generate an additional array layer for scalar components. */
+ if (attr->dimension || c->ts.type == BT_CHARACTER)
+ {
+ cdesc = gfc_get_array_type_bounds (tmp, 1, 0, &gfc_index_one_node,
+&ubound, 1,
+GFC_ARRAY_ALLOCATABLE, false);
- cdesc = gfc_create_var (cdesc, "cdesc");
- DECL_ARTIFICIAL (cdesc) = 1;
+ cdesc = gfc_create_var (cdesc, "cdesc");
+ DECL_ARTIFICIAL (cdesc) = 1;
- gfc_add_modify (&tmpblock, gfc_conv_descriptor_dtype (cdesc),
- gfc_get_dtype_rank_type (1, tmp));
- gfc_conv_descriptor_lbound_set (&tmpblock, cdesc,
- gfc_index_zero_node,
- gfc_index_one_node);
- gfc_conv_descriptor_stride_set (&tmpblock, cdesc,
- gfc_index_zero_node,
- gfc_index_one_node);
- gfc_conv_descriptor_ubound_set (&tmpblock, cdesc,
- gfc_index_zero_node, ubound);
+ gfc_add_modify (&tmpblock, gfc_conv_descriptor_dtype (cdesc),
+ gfc_get_dtype_rank_type (1, tmp));
+ gfc_conv_descriptor_lbound_set (&tmpblock, cdesc,
+ gfc_index_zero_node,
+ gfc_index_one_node);
+ gfc_conv_descriptor_stride_set (&tmpblock, cdesc,
+ gfc_index_zero_node,
+ gfc_index_one_node);
+ gfc_conv_descriptor_ubound_set (&tmpblock, cdesc,
+ gfc_index_zero_node, ubound);
+ }
if (attr->dimension)
- comp = gfc_conv_descriptor_data_get (comp);
+ {
+ if (GFC_DESCRIPTOR_TYPE_P (TREE_TYPE (comp)))
+ comp = gfc_conv_descriptor_data_get (comp);
+
[Submitted, PR103970, Fortran, Coarray] Multi-image co_broadcast of derived type with allocatable components fails^
Hi Harald, thanks for the fast review. I have submitted as c9c48ab7bad. Will wait for two weeks (reminder set :-)) before backporting to gcc-11. Thank you and regards, Andre On Tue, 25 Jan 2022 22:30:22 +0100 Harald Anlauf via Fortran wrote: > Hi Andre', > > Am 25.01.22 um 17:32 schrieb Andre Vehreschild via Fortran: > > Hi all, > > > > attached patch fixes wrong code generation when broadcasting a derived type > > containing allocatable and non-allocatable scalars. Furthermore does it > > prevent broadcasting of coarray-tokens, which are always local this_image. > > Thus having them on a different image makes no sense. > > > > Bootstrapped and regtested ok on x86_64-linux/F35. > > > > Ok, for trunk and backport to 12 and 11-branch after decent time? > > > > I perceived that 12 is closed for this kind of bugfix, therefore asking ok > > for 13. > > I do not think that 12 is closed for bugfixing, especially not for > fortran. And if my cursory reading of the patch is not misleading, > the impact of the patch is really limited to coarrays. > > You may want to wait for another 1-2 days for additional comments. > If not, it is OK from my side. > > Thanks for the patch! > > Harald > > > Regards, > > Andre > > -- > > Andre Vehreschild * Email: vehre ad gmx dot de > > -- Andre Vehreschild * Email: vehre ad gmx dot de
Re: [Submitted, PR103970, Fortran, Coarray] Multi-image co_broadcast of derived type with allocatable components fails^
Hi Tobias, ups, sorry, reverted immediately. Regards, Andre On Fri, 28 Jan 2022 10:27:26 +0100 Tobias Burnus wrote: > Hi Andre, > > your patch breaks bootstrapping: > > ../../repos/gcc/gcc/fortran/trans-array.cc: In function ‘tree_node* > structure_alloc_comps(gfc_symbol*, tree, tree, int, int, int, > gfc_co_subroutines_args*)’: > ../../repos/gcc/gcc/fortran/trans-array.cc:9200:42: error: ‘cdesc’ may be > used uninitialized [-Werror=maybe-uninitialized] 9200 | > gfc_conv_descriptor_data_set (&tmpblock, cdesc, comp); | > ~^~~~ > ../../repos/gcc/gcc/fortran/trans-array.cc:9082:16: note: ‘cdesc’ was > declared here 9082 | tree cdesc; |^ cc1plus: > all warnings being treated as errors make[3]: *** [Makefile:1143: > fortran/trans-array.o] Error 1 > > Tobias > > On 28.01.22 10:07, Andre Vehreschild via Fortran wrote: > > Hi Harald, > > > > thanks for the fast review. I have submitted as c9c48ab7bad. > > > > Will wait for two weeks (reminder set :-)) before backporting to gcc-11. > > > > Thank you and regards, > > Andre > > > > On Tue, 25 Jan 2022 22:30:22 +0100 > > Harald Anlauf via Fortran wrote: > > > >> Hi Andre', > >> > >> Am 25.01.22 um 17:32 schrieb Andre Vehreschild via Fortran: > >>> Hi all, > >>> > >>> attached patch fixes wrong code generation when broadcasting a derived > >>> type containing allocatable and non-allocatable scalars. Furthermore does > >>> it prevent broadcasting of coarray-tokens, which are always local > >>> this_image. Thus having them on a different image makes no sense. > >>> > >>> Bootstrapped and regtested ok on x86_64-linux/F35. > >>> > >>> Ok, for trunk and backport to 12 and 11-branch after decent time? > >>> > >>> I perceived that 12 is closed for this kind of bugfix, therefore asking ok > >>> for 13. > >> I do not think that 12 is closed for bugfixing, especially not for > >> fortran. And if my cursory reading of the patch is not misleading, > >> the impact of the patch is really limited to coarrays. > >> > >> You may want to wait for another 1-2 days for additional comments. > >> If not, it is OK from my side. > >> > >> Thanks for the patch! > >> > >> Harald > >> > >>> Regards, > >>> Andre > >>> -- > >>> Andre Vehreschild * Email: vehre ad gmx dot de > >> > > > > -- > > Andre Vehreschild * Email: vehre ad gmx dot de > - > Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 > München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas > Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht > München, HRB 106955 -- Andre Vehreschild * Email: vehre ad gmx dot de
Re: [Submitted, PR103970, Fortran, Coarray] Multi-image co_broadcast of derived type with allocatable components fails^
Hi Tobias, I don't know why that bootstrapped initially. I fixed the patch (naming a ``` else /* Prevent warning. */ cdesc = NULL_TREE; ``` obvious) and rerun bootstrap making sure to purge everything beforehand. It did not break bootstrap on x86_64-linux/f35. Hope it doesn't elsewhere with submit 26e237fb5b8. Thanks for pointing this out. Regards, Andre On Fri, 28 Jan 2022 10:36:23 +0100 Andre Vehreschild via Fortran wrote: > Hi Tobias, > > ups, sorry, reverted immediately. > > Regards, > Andre > > On Fri, 28 Jan 2022 10:27:26 +0100 > Tobias Burnus wrote: > > > Hi Andre, > > > > your patch breaks bootstrapping: > > > > ../../repos/gcc/gcc/fortran/trans-array.cc: In function ‘tree_node* > > structure_alloc_comps(gfc_symbol*, tree, tree, int, int, int, > > gfc_co_subroutines_args*)’: > > ../../repos/gcc/gcc/fortran/trans-array.cc:9200:42: error: ‘cdesc’ may be > > used uninitialized [-Werror=maybe-uninitialized] 9200 | > > gfc_conv_descriptor_data_set (&tmpblock, cdesc, comp); | > > ~^~~~ > > ../../repos/gcc/gcc/fortran/trans-array.cc:9082:16: note: ‘cdesc’ was > > declared here 9082 | tree cdesc; |^ cc1plus: > > all warnings being treated as errors make[3]: *** [Makefile:1143: > > fortran/trans-array.o] Error 1 > > > > Tobias > > > > On 28.01.22 10:07, Andre Vehreschild via Fortran wrote: > > > Hi Harald, > > > > > > thanks for the fast review. I have submitted as c9c48ab7bad. > > > > > > Will wait for two weeks (reminder set :-)) before backporting to gcc-11. > > > > > > Thank you and regards, > > > Andre > > > > > > On Tue, 25 Jan 2022 22:30:22 +0100 > > > Harald Anlauf via Fortran wrote: > > > > > >> Hi Andre', > > >> > > >> Am 25.01.22 um 17:32 schrieb Andre Vehreschild via Fortran: > > >>> Hi all, > > >>> > > >>> attached patch fixes wrong code generation when broadcasting a derived > > >>> type containing allocatable and non-allocatable scalars. Furthermore > > >>> does it prevent broadcasting of coarray-tokens, which are always local > > >>> this_image. Thus having them on a different image makes no sense. > > >>> > > >>> Bootstrapped and regtested ok on x86_64-linux/F35. > > >>> > > >>> Ok, for trunk and backport to 12 and 11-branch after decent time? > > >>> > > >>> I perceived that 12 is closed for this kind of bugfix, therefore asking > > >>> ok for 13. > > >> I do not think that 12 is closed for bugfixing, especially not for > > >> fortran. And if my cursory reading of the patch is not misleading, > > >> the impact of the patch is really limited to coarrays. > > >> > > >> You may want to wait for another 1-2 days for additional comments. > > >> If not, it is OK from my side. > > >> > > >> Thanks for the patch! > > >> > > >> Harald > > >> > > >>> Regards, > > >>> Andre > > >>> -- > > >>> Andre Vehreschild * Email: vehre ad gmx dot de > > >> > > > > > > -- > > > Andre Vehreschild * Email: vehre ad gmx dot de > > - > > Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, > > 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: > > Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; > > Registergericht München, HRB 106955 > > -- Andre Vehreschild * Email: vehre ad gmx dot de
Re: [Backport, committed, PR103970, Fortran, Coarray] Multi-image co_broadcast of derived type with allocatable components fails^
Hi all, two weeks have passed with no complains about the patch for PR103970. Therefore backported and pushed to gcc-11 as 680ee9c3332. Regards, Andre On Fri, 28 Jan 2022 12:39:17 +0100 Andre Vehreschild wrote: > Hi Tobias, > > I don't know why that bootstrapped initially. I fixed the patch (naming a > ``` > else > /* Prevent warning. */ > cdesc = NULL_TREE; > ``` > obvious) and rerun bootstrap making sure to purge everything beforehand. It > did not break bootstrap on x86_64-linux/f35. Hope it doesn't elsewhere with > submit 26e237fb5b8. > > Thanks for pointing this out. > > Regards, > Andre > > On Fri, 28 Jan 2022 10:36:23 +0100 > Andre Vehreschild via Fortran wrote: > > > Hi Tobias, > > > > ups, sorry, reverted immediately. > > > > Regards, > > Andre > > > > On Fri, 28 Jan 2022 10:27:26 +0100 > > Tobias Burnus wrote: > > > > > Hi Andre, > > > > > > your patch breaks bootstrapping: > > > > > > ../../repos/gcc/gcc/fortran/trans-array.cc: In function ‘tree_node* > > > structure_alloc_comps(gfc_symbol*, tree, tree, int, int, int, > > > gfc_co_subroutines_args*)’: > > > ../../repos/gcc/gcc/fortran/trans-array.cc:9200:42: error: ‘cdesc’ may be > > > used uninitialized [-Werror=maybe-uninitialized] 9200 | > > > gfc_conv_descriptor_data_set (&tmpblock, cdesc, comp); | > > > ~^~~~ > > > ../../repos/gcc/gcc/fortran/trans-array.cc:9082:16: note: ‘cdesc’ was > > > declared here 9082 | tree cdesc; |^ cc1plus: > > > all warnings being treated as errors make[3]: *** [Makefile:1143: > > > fortran/trans-array.o] Error 1 > > > > > > Tobias > > > > > > On 28.01.22 10:07, Andre Vehreschild via Fortran wrote: > > > > Hi Harald, > > > > > > > > thanks for the fast review. I have submitted as c9c48ab7bad. > > > > > > > > Will wait for two weeks (reminder set :-)) before backporting to gcc-11. > > > > > > > > Thank you and regards, > > > > Andre > > > > > > > > On Tue, 25 Jan 2022 22:30:22 +0100 > > > > Harald Anlauf via Fortran wrote: > > > > > > > >> Hi Andre', > > > >> > > > >> Am 25.01.22 um 17:32 schrieb Andre Vehreschild via Fortran: > > > >>> Hi all, > > > >>> > > > >>> attached patch fixes wrong code generation when broadcasting a derived > > > >>> type containing allocatable and non-allocatable scalars. Furthermore > > > >>> does it prevent broadcasting of coarray-tokens, which are always local > > > >>> this_image. Thus having them on a different image makes no sense. > > > >>> > > > >>> Bootstrapped and regtested ok on x86_64-linux/F35. > > > >>> > > > >>> Ok, for trunk and backport to 12 and 11-branch after decent time? > > > >>> > > > >>> I perceived that 12 is closed for this kind of bugfix, therefore > > > >>> asking ok for 13. > > > >> I do not think that 12 is closed for bugfixing, especially not for > > > >> fortran. And if my cursory reading of the patch is not misleading, > > > >> the impact of the patch is really limited to coarrays. > > > >> > > > >> You may want to wait for another 1-2 days for additional comments. > > > >> If not, it is OK from my side. > > > >> > > > >> Thanks for the patch! > > > >> > > > >> Harald > > > >> > > > >>> Regards, > > > >>> Andre > > > >>> -- > > > >>> Andre Vehreschild * Email: vehre ad gmx dot de > > > >> > > > > > > > > -- > > > > Andre Vehreschild * Email: vehre ad gmx dot de > > > - > > > Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, > > > 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: > > > Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; > > > Registergericht München, HRB 106955 > > > > > > -- Andre Vehreschild * Email: vehre ad gmx dot de
Re: [Backport gcc-11, Patch, Fortran] PR100337 Should be able to pass non-present optional arguments to CO_BROADCAST
Hi everyone, sorry for missing out on the gcc-11 backport, but better late than never. Committed backport as ae57aae60d1. Regards, Andre On Wed, 23 Jun 2021 11:21:45 +0200 Tobias Burnus wrote: > On 23.06.21 10:23, Andre Vehreschild wrote: > > > Will wait two weeks for any errors introduced by this patch before > > backporting to gcc-11, ok? > > Fine with me. > > Thanks again for the patch. > > Tobias > > - > Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München > Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank > Thürauf -- Andre Vehreschild * Email: vehre ad gmx dot de
Re: [Ping^2, Patch, Fortran] PR98301 Re: RANDOM_INIT() and coarray Fortran
Ping, ping!
Please find attached a rebased version of the patch for the RANDOM_INIT issue
with coarray Fortran. Nothing changed to the previous version, just rebased to
current master.
Regtested fine on x86_64-linux/f33. Ok for trunk?
- Andre
On Mon, 3 May 2021 08:20:36 -0700
Steve Kargl wrote:
> On Mon, May 03, 2021 at 11:21:10AM +0200, Andre Vehreschild wrote:
> > Ping!
> >
> > Ok for trunk?
> >
> > I have looked at other patches, but none was patching any location I have
> > worked on previously. Therefore I can't return the favor of reviewing any
> > currently open patches and have to ask for volunteers here.
> >
> > - Andre
> >
>
> I doubt I'm allowed to approve a patch, where I wrote a portion
> of it. However, if no one else steps forward in the next day
> or two, then I think you should commit.
>
--
Andre Vehreschild * Email: vehre ad gmx dot de
Steve Kargl
PR fortran/98301 - random_init() is broken
Correct implementation of random_init() when -fcoarray=lib is given.
gcc/fortran/ChangeLog:
PR fortran/98301
* trans-decl.c (gfc_build_builtin_function_decls): Move decl.
* trans-intrinsic.c (conv_intrinsic_random_init): Use bool for
lib-call of caf_random_init instead of logical (4-byte).
* trans.h: Add tree var for random_init.
libgfortran/ChangeLog:
PR fortran/98301
* caf/libcaf.h (_gfortran_caf_random_init): New function.
* caf/single.c (_gfortran_caf_random_init): New function.
* gfortran.map: Added fndecl.
* intrinsics/random_init.f90: Implement random_init.
diff --git a/gcc/fortran/trans-decl.c b/gcc/fortran/trans-decl.c
index 406b4aeb1d4..c32bd05bb1b 100644
--- a/gcc/fortran/trans-decl.c
+++ b/gcc/fortran/trans-decl.c
@@ -170,6 +170,7 @@ tree gfor_fndecl_co_min;
tree gfor_fndecl_co_reduce;
tree gfor_fndecl_co_sum;
tree gfor_fndecl_caf_is_present;
+tree gfor_fndecl_caf_random_init;
/* Math functions. Many other math functions are handled in
@@ -233,7 +234,7 @@ tree gfor_fndecl_cgemm;
tree gfor_fndecl_zgemm;
/* RANDOM_INIT function. */
-tree gfor_fndecl_random_init;
+tree gfor_fndecl_random_init; /* libgfortran, 1 image only. */
static void
gfc_add_decl_to_parent_function (tree decl)
@@ -3516,6 +3517,8 @@ gfc_build_intrinsic_function_decls (void)
void_type_node, 3, gfc_logical4_type_node, gfc_logical4_type_node,
gfc_int4_type_node);
+ // gfor_fndecl_caf_rand_init is defined in the lib-coarray section below.
+
gfor_fndecl_sc_kind = gfc_build_library_function_decl_with_spec (
get_identifier (PREFIX("selected_char_kind")), ". . R ",
gfc_int4_type_node, 2, gfc_charlen_type_node, pchar_type_node);
@@ -4081,6 +4084,10 @@ gfc_build_builtin_function_decls (void)
get_identifier (PREFIX("caf_is_present")), ". r . r ",
integer_type_node, 3, pvoid_type_node, integer_type_node,
pvoid_type_node);
+
+ gfor_fndecl_caf_random_init = gfc_build_library_function_decl (
+ get_identifier (PREFIX("caf_random_init")),
+ void_type_node, 2, logical_type_node, logical_type_node);
}
gfc_build_intrinsic_function_decls ();
diff --git a/gcc/fortran/trans-intrinsic.c b/gcc/fortran/trans-intrinsic.c
index 4d7451479d3..db9248c0043 100644
--- a/gcc/fortran/trans-intrinsic.c
+++ b/gcc/fortran/trans-intrinsic.c
@@ -3827,38 +3827,43 @@ conv_intrinsic_random_init (gfc_code *code)
{
stmtblock_t block;
gfc_se se;
- tree arg1, arg2, arg3, tmp;
- tree logical4_type_node = gfc_get_logical_type (4);
+ tree arg1, arg2, tmp;
+ /* On none coarray == lib compiles use LOGICAL(4) else regular LOGICAL. */
+ tree used_bool_type_node = flag_coarray == GFC_FCOARRAY_LIB
+ ? logical_type_node
+ : gfc_get_logical_type (4);
/* Make the function call. */
gfc_init_block (&block);
gfc_init_se (&se, NULL);
- /* Convert REPEATABLE to a LOGICAL(4) entity. */
+ /* Convert REPEATABLE to the desired LOGICAL entity. */
gfc_conv_expr (&se, code->ext.actual->expr);
gfc_add_block_to_block (&block, &se.pre);
- arg1 = fold_convert (logical4_type_node, gfc_evaluate_now (se.expr, &block));
+ arg1 = fold_convert (used_bool_type_node, gfc_evaluate_now (se.expr, &block));
gfc_add_block_to_block (&block, &se.post);
- /* Convert IMAGE_DISTINCT to a LOGICAL(4) entity. */
+ /* Convert IMAGE_DISTINCT to the desired LOGICAL entity. */
gfc_conv_expr (&se, code->ext.actual->next->expr);
gfc_add_block_to_block (&block, &se.pre);
- arg2 = fold_convert (logical4_type_node, gfc_evaluate_now (se.expr, &block));
+ arg2 = fold_convert (used_bool_type_node, gfc_evaluate_now (se.expr, &block));
gfc_add_block_to_block (&block, &se.post);
- /* Create the hidden argument. For non-coarray codes and -fcoarray=single,
- simply set this to 0. For -fcoarray=lib, generate a call to
- THIS_IMAGE() without arguments. */
- arg3 = build_int_cst (gfc_get_int_type (4), 0);
if (flag_coarray == GFC_FCOARRAY_LIB)
{
- arg3 = build_call_expr_loc (input_location, gfor_fndecl_caf_this_image,
- 1, arg3);
- se.expr = fold_c
[Patch, Fortran] PR100337 Should be able to pass non-present optional arguments to CO_BROADCAST
Hi,
the attached patch fixes an issue when calling CO_BROADCAST in -fcoarray=single
mode, where the optional but non-present (in the calling scope) stat variable
was assigned to before checking for it being not present.
Regtests fine on x86-64-linux/f33. Ok for trunk?
Regards,
Andre
--
Andre Vehreschild * Email: vehre ad gmx dot de
gcc/fortran/ChangeLog:
PR fortran/100337
* trans-intrinsic.c (conv_co_collective): Check stat for null ptr
before dereferrencing.
gcc/testsuite/ChangeLog:
PR fortran/100337
* gfortran.dg/coarray_collectives_17.f90: New test.
diff --git a/gcc/fortran/trans-intrinsic.c b/gcc/fortran/trans-intrinsic.c
index 4d7451479d3..03a38090051 100644
--- a/gcc/fortran/trans-intrinsic.c
+++ b/gcc/fortran/trans-intrinsic.c
@@ -11232,8 +11232,28 @@ conv_co_collective (gfc_code *code)
if (flag_coarray == GFC_FCOARRAY_SINGLE)
{
if (stat != NULL_TREE)
- gfc_add_modify (&block, stat,
- fold_convert (TREE_TYPE (stat), integer_zero_node));
+ {
+ /* For optional stats, check the pointer is valid before zero'ing. */
+ if (gfc_expr_attr (stat_expr).optional)
+ {
+ tree tmp;
+ stmtblock_t ass_block;
+ gfc_start_block (&ass_block);
+ gfc_add_modify (&ass_block, stat,
+ fold_convert (TREE_TYPE (stat),
+ integer_zero_node));
+ tmp = fold_build2 (NE_EXPR, logical_type_node,
+ gfc_build_addr_expr (NULL_TREE, stat),
+ null_pointer_node);
+ tmp = fold_build3 (COND_EXPR, void_type_node, tmp,
+ gfc_finish_block (&ass_block),
+ build_empty_stmt (input_location));
+ gfc_add_expr_to_block (&block, tmp);
+ }
+ else
+ gfc_add_modify (&block, stat,
+ fold_convert (TREE_TYPE (stat), integer_zero_node));
+ }
return gfc_finish_block (&block);
}
diff --git a/gcc/testsuite/gfortran.dg/coarray_collectives_17.f90 b/gcc/testsuite/gfortran.dg/coarray_collectives_17.f90
new file mode 100644
index 000..84a6645865e
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/coarray_collectives_17.f90
@@ -0,0 +1,42 @@
+! { dg-do run }
+! { dg-options "-fcoarray=single" }
+!
+! PR 100337
+! Test case inspired by code submitted by Brad Richardson
+
+program main
+implicit none
+
+integer, parameter :: MESSAGE = 42
+integer :: result
+
+call myco_broadcast(MESSAGE, result, 1)
+
+if (result /= MESSAGE) error stop 1
+contains
+subroutine myco_broadcast(m, r, source_image, stat, errmsg)
+integer, intent(in) :: m
+integer, intent(out) :: r
+integer, intent(in) :: source_image
+integer, intent(out), optional :: stat
+character(len=*), intent(inout), optional :: errmsg
+
+integer :: data_length
+
+data_length = 1
+
+call co_broadcast(data_length, source_image, stat, errmsg)
+
+if (present(stat)) then
+if (stat /= 0) return
+end if
+
+if (this_image() == source_image) then
+r = m
+end if
+
+call co_broadcast(r, source_image, stat, errmsg)
+end subroutine
+
+end program
+
Re: [Ping^2, Patch, Fortran] PR98301 Re: RANDOM_INIT() and coarray Fortran
Hi Steve and Jerry, thanks for the ok'ing. Committed as https://gcc.gnu.org/g:26ca6dbda23bc6dfab96ce07afa70ebacedfaf9c and https://gcc.gnu.org/g:c4771b3438a8cd9afcef1762957b763f8df3fa6e (for the missing changelog entries). - Andre On Fri, 21 May 2021 19:38:00 -0700 Jerry D wrote: > yes, please commit > > On 5/21/21 8:08 AM, Steve Kargl via Fortran wrote: > > On Fri, May 21, 2021 at 10:09:02AM +0200, Andre Vehreschild wrote: > >> Ping, ping! > >> > >> Please find attached a rebased version of the patch for the RANDOM_INIT > >> issue with coarray Fortran. Nothing changed to the previous version, just > >> rebased to current master. > >> > >> Regtested fine on x86_64-linux/f33. Ok for trunk? > >> > > I think you've down your due diligence with 2 pings. > > I would commit. > > > -- Andre Vehreschild * Email: vehre ad gmx dot de
Re: [Ping^2, Patch, Fortran] PR98301 Re: RANDOM_INIT() and coarray Fortran
Hi Martin, thanks for pointing that out. I haven't committed for quite some time now and could not find on the webpage how this works nowadays. I was thinking that the special gcc-git-command should have added the Changelog entries automagically and immediately. That they are added by a daily bump is new to me. Thanks for giving me more insight. So do I need to revert the commit c4771b3438a8cd9afcef1762957b763f8df3fa6e to fix the Changelogs or how do we proceed from here? Is there a webpage which describes the current state-of-art of committing to gcc for folks that do not follow every discussion on the mailing lists? Thanks for your help, Andre On Sat, 22 May 2021 19:58:57 +0200 Martin Liška wrote: > On 5/22/21 1:39 PM, Andre Vehreschild via Gcc-patches wrote: > > Hi Steve and Jerry, > > > > thanks for the ok'ing. > > > > Committed as https://gcc.gnu.org/g:26ca6dbda23bc6dfab96ce07afa70ebacedfaf9c > > and https://gcc.gnu.org/g:c4771b3438a8cd9afcef1762957b763f8df3fa6e (for the > > missing changelog entries). > > Hello. > > About the missing changelog entries. The will be added automatically by Daily > bump. You can check it with: > ./contrib/gcc-changelog/git_check_commit.py > 26ca6dbda23bc6dfab96ce07afa70ebacedfaf9c -p > > What's missing for you so that you pushed > c4771b3438a8cd9afcef1762957b763f8df3fa6e? > > Thanks, > Martin > > > > > - Andre > > > > On Fri, 21 May 2021 19:38:00 -0700 > > Jerry D wrote: > > > >> yes, please commit > >> > >> On 5/21/21 8:08 AM, Steve Kargl via Fortran wrote: > >>> On Fri, May 21, 2021 at 10:09:02AM +0200, Andre Vehreschild wrote: > >>>> Ping, ping! > >>>> > >>>> Please find attached a rebased version of the patch for the RANDOM_INIT > >>>> issue with coarray Fortran. Nothing changed to the previous version, just > >>>> rebased to current master. > >>>> > >>>> Regtested fine on x86_64-linux/f33. Ok for trunk? > >>>> > >>> I think you've down your due diligence with 2 pings. > >>> I would commit. > >>> > >> > > > > > > -- > > Andre Vehreschild * Email: vehre ad gmx dot de > > > -- Andre Vehreschild * Email: vehre ad gmx dot de
[Ping, Patch, Fortran] PR100337 Should be able to pass non-present optional arguments to CO_BROADCAST
Ping!
On Fri, 21 May 2021 15:33:11 +0200
Andre Vehreschild wrote:
> Hi,
>
> the attached patch fixes an issue when calling CO_BROADCAST in
> -fcoarray=single mode, where the optional but non-present (in the calling
> scope) stat variable was assigned to before checking for it being not present.
>
> Regtests fine on x86-64-linux/f33. Ok for trunk?
>
> Regards,
> Andre
--
Andre Vehreschild * Email: vehre ad gmx dot de
gcc/fortran/ChangeLog:
PR fortran/100337
* trans-intrinsic.c (conv_co_collective): Check stat for null ptr
before dereferrencing.
gcc/testsuite/ChangeLog:
PR fortran/100337
* gfortran.dg/coarray_collectives_17.f90: New test.
diff --git a/gcc/fortran/trans-intrinsic.c b/gcc/fortran/trans-intrinsic.c
index 4d7451479d3..03a38090051 100644
--- a/gcc/fortran/trans-intrinsic.c
+++ b/gcc/fortran/trans-intrinsic.c
@@ -11232,8 +11232,28 @@ conv_co_collective (gfc_code *code)
if (flag_coarray == GFC_FCOARRAY_SINGLE)
{
if (stat != NULL_TREE)
- gfc_add_modify (&block, stat,
- fold_convert (TREE_TYPE (stat), integer_zero_node));
+ {
+ /* For optional stats, check the pointer is valid before zero'ing. */
+ if (gfc_expr_attr (stat_expr).optional)
+ {
+ tree tmp;
+ stmtblock_t ass_block;
+ gfc_start_block (&ass_block);
+ gfc_add_modify (&ass_block, stat,
+ fold_convert (TREE_TYPE (stat),
+ integer_zero_node));
+ tmp = fold_build2 (NE_EXPR, logical_type_node,
+ gfc_build_addr_expr (NULL_TREE, stat),
+ null_pointer_node);
+ tmp = fold_build3 (COND_EXPR, void_type_node, tmp,
+ gfc_finish_block (&ass_block),
+ build_empty_stmt (input_location));
+ gfc_add_expr_to_block (&block, tmp);
+ }
+ else
+ gfc_add_modify (&block, stat,
+ fold_convert (TREE_TYPE (stat), integer_zero_node));
+ }
return gfc_finish_block (&block);
}
diff --git a/gcc/testsuite/gfortran.dg/coarray_collectives_17.f90 b/gcc/testsuite/gfortran.dg/coarray_collectives_17.f90
new file mode 100644
index 000..84a6645865e
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/coarray_collectives_17.f90
@@ -0,0 +1,42 @@
+! { dg-do run }
+! { dg-options "-fcoarray=single" }
+!
+! PR 100337
+! Test case inspired by code submitted by Brad Richardson
+
+program main
+implicit none
+
+integer, parameter :: MESSAGE = 42
+integer :: result
+
+call myco_broadcast(MESSAGE, result, 1)
+
+if (result /= MESSAGE) error stop 1
+contains
+subroutine myco_broadcast(m, r, source_image, stat, errmsg)
+integer, intent(in) :: m
+integer, intent(out) :: r
+integer, intent(in) :: source_image
+integer, intent(out), optional :: stat
+character(len=*), intent(inout), optional :: errmsg
+
+integer :: data_length
+
+data_length = 1
+
+call co_broadcast(data_length, source_image, stat, errmsg)
+
+if (present(stat)) then
+if (stat /= 0) return
+end if
+
+if (this_image() == source_image) then
+r = m
+end if
+
+call co_broadcast(r, source_image, stat, errmsg)
+end subroutine
+
+end program
+
[Patch, Fortran, backport 2 gcc-11] PR98301 Re: RANDOM_INIT() and coarray Fortran
Hi all,
I was asked to backport the patch for pr98301 to gcc-11. The patches have
been in mainline for two weeks without any defect reports I could fined. The
patch for mainline applied with a bit of shift cleanly.
Regstested fine on x86_64/f33. Ok for backport gcc-11?
Regards,
Andre
--
Andre Vehreschild * Email: vehre ad gmx dot de
Steve Kargl
PR fortran/98301 - random_init() is broken
Correct implementation of random_init() when -fcoarray=lib is given.
Backport from mainline.
gcc/fortran/ChangeLog:
PR fortran/98301
* trans-decl.c (gfc_build_builtin_function_decls): Move decl.
* trans-intrinsic.c (conv_intrinsic_random_init): Use bool for
lib-call of caf_random_init instead of logical (4-byte).
* trans.h: Add tree var for random_init.
libgfortran/ChangeLog:
PR fortran/98301
* caf/libcaf.h (_gfortran_caf_random_init): New function.
* caf/single.c (_gfortran_caf_random_init): New function.
* gfortran.map: Added fndecl.
* intrinsics/random_init.f90: Implement random_init.
diff --git a/gcc/fortran/trans-decl.c b/gcc/fortran/trans-decl.c
index 7cded0a3ede..4fa39f71a21 100644
--- a/gcc/fortran/trans-decl.c
+++ b/gcc/fortran/trans-decl.c
@@ -170,6 +170,7 @@ tree gfor_fndecl_co_min;
tree gfor_fndecl_co_reduce;
tree gfor_fndecl_co_sum;
tree gfor_fndecl_caf_is_present;
+tree gfor_fndecl_caf_random_init;
/* Math functions. Many other math functions are handled in
@@ -233,7 +234,7 @@ tree gfor_fndecl_cgemm;
tree gfor_fndecl_zgemm;
/* RANDOM_INIT function. */
-tree gfor_fndecl_random_init;
+tree gfor_fndecl_random_init; /* libgfortran, 1 image only. */
static void
gfc_add_decl_to_parent_function (tree decl)
@@ -3516,6 +3517,8 @@ gfc_build_intrinsic_function_decls (void)
void_type_node, 3, gfc_logical4_type_node, gfc_logical4_type_node,
gfc_int4_type_node);
+ // gfor_fndecl_caf_rand_init is defined in the lib-coarray section below.
+
gfor_fndecl_sc_kind = gfc_build_library_function_decl_with_spec (
get_identifier (PREFIX("selected_char_kind")), ". . R ",
gfc_int4_type_node, 2, gfc_charlen_type_node, pchar_type_node);
@@ -4081,6 +4084,10 @@ gfc_build_builtin_function_decls (void)
get_identifier (PREFIX("caf_is_present")), ". r . r ",
integer_type_node, 3, pvoid_type_node, integer_type_node,
pvoid_type_node);
+
+ gfor_fndecl_caf_random_init = gfc_build_library_function_decl (
+ get_identifier (PREFIX("caf_random_init")),
+ void_type_node, 2, logical_type_node, logical_type_node);
}
gfc_build_intrinsic_function_decls ();
diff --git a/gcc/fortran/trans-intrinsic.c b/gcc/fortran/trans-intrinsic.c
index 68090d4defc..2c094f326e6 100644
--- a/gcc/fortran/trans-intrinsic.c
+++ b/gcc/fortran/trans-intrinsic.c
@@ -3837,38 +3837,43 @@ conv_intrinsic_random_init (gfc_code *code)
{
stmtblock_t block;
gfc_se se;
- tree arg1, arg2, arg3, tmp;
- tree logical4_type_node = gfc_get_logical_type (4);
+ tree arg1, arg2, tmp;
+ /* On none coarray == lib compiles use LOGICAL(4) else regular LOGICAL. */
+ tree used_bool_type_node = flag_coarray == GFC_FCOARRAY_LIB
+ ? logical_type_node
+ : gfc_get_logical_type (4);
/* Make the function call. */
gfc_init_block (&block);
gfc_init_se (&se, NULL);
- /* Convert REPEATABLE to a LOGICAL(4) entity. */
+ /* Convert REPEATABLE to the desired LOGICAL entity. */
gfc_conv_expr (&se, code->ext.actual->expr);
gfc_add_block_to_block (&block, &se.pre);
- arg1 = fold_convert (logical4_type_node, gfc_evaluate_now (se.expr, &block));
+ arg1 = fold_convert (used_bool_type_node, gfc_evaluate_now (se.expr, &block));
gfc_add_block_to_block (&block, &se.post);
- /* Convert IMAGE_DISTINCT to a LOGICAL(4) entity. */
+ /* Convert IMAGE_DISTINCT to the desired LOGICAL entity. */
gfc_conv_expr (&se, code->ext.actual->next->expr);
gfc_add_block_to_block (&block, &se.pre);
- arg2 = fold_convert (logical4_type_node, gfc_evaluate_now (se.expr, &block));
+ arg2 = fold_convert (used_bool_type_node, gfc_evaluate_now (se.expr, &block));
gfc_add_block_to_block (&block, &se.post);
- /* Create the hidden argument. For non-coarray codes and -fcoarray=single,
- simply set this to 0. For -fcoarray=lib, generate a call to
- THIS_IMAGE() without arguments. */
- arg3 = build_int_cst (gfc_get_int_type (4), 0);
if (flag_coarray == GFC_FCOARRAY_LIB)
{
- arg3 = build_call_expr_loc (input_location, gfor_fndecl_caf_this_image,
- 1, arg3);
- se.expr = fold_convert (gfc_get_int_type (4), arg3);
+ tmp = build_call_expr_loc (input_location, gfor_fndecl_caf_random_init,
+ 2, arg1, arg2);
+}
+ else
+{
+ /* The ABI for libgfortran needs to be maintained, so a hidden
+ argument must be include if code is compiled with -fcoarray=single
+ or without the option. Set to 0. */
+ tree arg3 = build_int_cst (gfc_get_int_type (4), 0);
+ tmp = build_call_expr_loc (input_location, gfor_fndecl_random_init,
+ 3, arg1, arg2, arg3);
Re: [COMITTED, Patch, Fortran, backport 2 gcc-11] PR98301 Re: RANDOM_INIT() and coarray Fortran
Hi Steve, hi all, the patch for pr98301 has been backported to gcc-11 as 002745ca3668fc5e87c22acc81caaeaaadf9c47a Regards, Andre On Sat, 5 Jun 2021 09:27:16 -0700 Steve Kargl wrote: > On Sat, Jun 05, 2021 at 04:04:51PM +0200, Andre Vehreschild wrote: > > > > I was asked to backport the patch for pr98301 to gcc-11. The patches have > > been in mainline for two weeks without any defect reports I could fined. The > > patch for mainline applied with a bit of shift cleanly. > > > > Regstested fine on x86_64/f33. Ok for backport gcc-11? > > > > I think the backport is fine. > -- Andre Vehreschild * Email: vehre ad gmx dot de
Re: [Ping, Patch, Fortran, Update 2] PR98301 Re: RANDOM_INIT() and coarray Fortran
Ping! Ok for trunk? I have looked at other patches, but none was patching any location I have worked on previously. Therefore I can't return the favor of reviewing any currently open patches and have to ask for volunteers here. - Andre On Mon, 26 Apr 2021 12:36:36 +0200 Andre Vehreschild via Fortran wrote: > Hi Steve, hi all, > > I agree. The cas-things have been removed (I will put the patch for them into > the pr98301 ticket, so safe them), streamlining the patch a bit more. > > Bootstraped and regtested ok on x86_64-linux/f33. Ok for trunk? > > Regards, > Andre > > Steve Kargl > > PR fortran/98301 - random_init() is broken > > Correct implementation of random_init() when -fcoarray=lib is given. > > gcc/fortran/ChangeLog: > > PR fortran/98301 > * trans-decl.c (gfc_build_builtin_function_decls): Move decl. > * trans-intrinsic.c (conv_intrinsic_random_init): Use bool for > lib-call of caf_random_init instead of logical (4-byte). > * trans.h: Add tree var for random_init. > > libgfortran/ChangeLog: > > PR fortran/98302 > * caf/libcaf.h (_gfortran_caf_random_init): New function. > * caf/single.c (_gfortran_caf_random_init): New function. > * gfortran.map: Added fndecl. > * intrinsics/random_init.f90: Implement random_init. > > > > On Sun, 25 Apr 2021 13:03:34 -0700 > Steve Kargl wrote: > > > Andre, > > > > The patch looks fine to me. I wonder, however, if we should > > comment out all of the shared memory stuff, i.e., the _cas_ > > stuff. I don't know when Thomas/Nicolas will merge their > > work-in-progress. > > > > > -- > Andre Vehreschild * Email: vehre ad gmx dot de -- Andre Vehreschild * Email: vehre ad gmx dot de
Re: [Patch, fortran] PRs 46691 and 99819: Assumed and explicit size class arrays
Hi Paul, this and the Changelog LGTM at least for 12. Give it a consolidation time before applying to 11. Having had some issues in the vicinity of the code you addressed I am quite happy to see how easy the fix looks. Any chances you can take a look at https://gcc.gnu.org/pipermail/fortran/2021-April/055990.html ? Regards, Andre On Thu, 6 May 2021 07:57:05 +0100 Paul Richard Thomas via Fortran wrote: > Hi All, > > Although I had undertaken to concentrate on PDTs, PR99819 so intrigued me > that I became locked into it :-( After extensive, fruitless rummaging > through decl.c and trans-decl.c, I realised that the problem was far > simpler than it seemed and that it lay in class.c. After that PR was fixed, > PR46691 was a trivial follow up. > > The comments in the patch explain the fixes. I left a TODO for the extent > checking of assumed size class arrays. I will try to fix it before pushing. > > Regtested on FC33/x86_64 and checked against the 'other brand'. OK for > 12-branch and, perhaps, 11-branch? > > Regards > > Paul > > Fortran: Assumed and explicit size class arrays [PR46691/99819]. > > 2021-05-06 Paul Thomas > > gcc/fortran/ChangeLog > > PR fortran/46691 > PR fortran/99819 > * class.c (gfc_build_class_symbol): Class array types that are > not deferred shape or assumed rank are given a unique name and > placed in the procedure namespace. > * trans-array.c (gfc_trans_g77_array): Obtain the data pointer > for class arrays. > (gfc_trans_dummy_array_bias): Suppress the runtime error for > extent violations in explicit shape class arrays because it > always fails. > * trans-expr.c (gfc_conv_procedure_call): Handle assumed size > class actual arguments passed to non-descriptor formal args by > using the data pointer, stored as the symbol's backend decl. > > gcc/testsuite/ChangeLog > > PR fortran/46691 > PR fortran/99819 > * gfortran.dg/class_dummy_6.f90: New test. > * gfortran.dg/class_dummy_6.f90: New test. -- Andre Vehreschild * Email: vehre ad gmx dot de
Re: [Ping^2, Patch, Fortran] PR100337 Should be able to pass non-present optional arguments to CO_BROADCAST
PING!
On Fri, 4 Jun 2021 18:05:18 +0200
Andre Vehreschild wrote:
> Ping!
>
> On Fri, 21 May 2021 15:33:11 +0200
> Andre Vehreschild wrote:
>
> > Hi,
> >
> > the attached patch fixes an issue when calling CO_BROADCAST in
> > -fcoarray=single mode, where the optional but non-present (in the calling
> > scope) stat variable was assigned to before checking for it being not
> > present.
> >
> > Regtests fine on x86-64-linux/f33. Ok for trunk?
> >
> > Regards,
> > Andre
>
>
--
Andre Vehreschild * Email: vehre ad gmx dot de
gcc/fortran/ChangeLog:
PR fortran/100337
* trans-intrinsic.c (conv_co_collective): Check stat for null ptr
before dereferrencing.
gcc/testsuite/ChangeLog:
PR fortran/100337
* gfortran.dg/coarray_collectives_17.f90: New test.
diff --git a/gcc/fortran/trans-intrinsic.c b/gcc/fortran/trans-intrinsic.c
index 4d7451479d3..03a38090051 100644
--- a/gcc/fortran/trans-intrinsic.c
+++ b/gcc/fortran/trans-intrinsic.c
@@ -11232,8 +11232,28 @@ conv_co_collective (gfc_code *code)
if (flag_coarray == GFC_FCOARRAY_SINGLE)
{
if (stat != NULL_TREE)
- gfc_add_modify (&block, stat,
- fold_convert (TREE_TYPE (stat), integer_zero_node));
+ {
+ /* For optional stats, check the pointer is valid before zero'ing. */
+ if (gfc_expr_attr (stat_expr).optional)
+ {
+ tree tmp;
+ stmtblock_t ass_block;
+ gfc_start_block (&ass_block);
+ gfc_add_modify (&ass_block, stat,
+ fold_convert (TREE_TYPE (stat),
+ integer_zero_node));
+ tmp = fold_build2 (NE_EXPR, logical_type_node,
+ gfc_build_addr_expr (NULL_TREE, stat),
+ null_pointer_node);
+ tmp = fold_build3 (COND_EXPR, void_type_node, tmp,
+ gfc_finish_block (&ass_block),
+ build_empty_stmt (input_location));
+ gfc_add_expr_to_block (&block, tmp);
+ }
+ else
+ gfc_add_modify (&block, stat,
+ fold_convert (TREE_TYPE (stat), integer_zero_node));
+ }
return gfc_finish_block (&block);
}
diff --git a/gcc/testsuite/gfortran.dg/coarray_collectives_17.f90 b/gcc/testsuite/gfortran.dg/coarray_collectives_17.f90
new file mode 100644
index 000..84a6645865e
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/coarray_collectives_17.f90
@@ -0,0 +1,42 @@
+! { dg-do run }
+! { dg-options "-fcoarray=single" }
+!
+! PR 100337
+! Test case inspired by code submitted by Brad Richardson
+
+program main
+implicit none
+
+integer, parameter :: MESSAGE = 42
+integer :: result
+
+call myco_broadcast(MESSAGE, result, 1)
+
+if (result /= MESSAGE) error stop 1
+contains
+subroutine myco_broadcast(m, r, source_image, stat, errmsg)
+integer, intent(in) :: m
+integer, intent(out) :: r
+integer, intent(in) :: source_image
+integer, intent(out), optional :: stat
+character(len=*), intent(inout), optional :: errmsg
+
+integer :: data_length
+
+data_length = 1
+
+call co_broadcast(data_length, source_image, stat, errmsg)
+
+if (present(stat)) then
+if (stat /= 0) return
+end if
+
+if (this_image() == source_image) then
+r = m
+end if
+
+call co_broadcast(r, source_image, stat, errmsg)
+end subroutine
+
+end program
+
Re: [Ping^2, Patch, Fortran] PR100337 Should be able to pass non-present optional arguments to CO_BROADCAST
Hi Tobias, thanks for the review. To the questions: - I added a test only for -fcoarray=single because in the library case the optional stat is just propagated to the library, which is already tested a lot of times and which needs to handle the optional stat in any case. So an error there would have been detected in one of the earlier tests. I did not want to add unnecessary test overhead given that the tests already run for a long time. - I did not add tests for the other CO_* routines, i.e. CO_MIN, CO_MAX, CO_REDUCE or CO_SUM, that are also handled by this routine, because I believe that showing that the fix works for CO_BROADCAST shows that the others work, too. Because the four others do not have any special handling in their implementation in trans_intrinsic. Or do you mean other coarray-routines besides the five handled by conv_co_collective()? If it is ok for you, I would apply the patch as is, or do you see a reason to add more tests? Regards, Andre On Mon, 21 Jun 2021 14:30:21 +0200 Tobias Burnus wrote: > Any reason that you did not put it under >gfortran.dg/coarray/ > such that it is also run with -fcoarray=lib (-lcaf_single)? > I know that the issue only exists for single, but it also makes > sense to check that libcaf_single works > > In that sense, I wonder whether also the other CO_* should be > checked in the testsuite as they are handled differently in > libcaf_... (but identical with -fcoarray=single). > > Except for those two nits, it LGTM. Thanks! > > Tobias > > PS: The function is used by > case GFC_ISYM_CO_BROADCAST: > case GFC_ISYM_CO_MIN: > case GFC_ISYM_CO_MAX: > case GFC_ISYM_CO_REDUCE: > case GFC_ISYM_CO_SUM: > and, with -fcoarray=single, errmsg is not touched > as stat is (unconditionally) 0 (success).. > > > On 19.06.21 13:23, Andre Vehreschild via Fortran wrote: > > PING! > > > > On Fri, 4 Jun 2021 18:05:18 +0200 > > Andre Vehreschild wrote: > > > >> Ping! > >> > >> On Fri, 21 May 2021 15:33:11 +0200 > >> Andre Vehreschild wrote: > >> > >>> Hi, > >>> > >>> the attached patch fixes an issue when calling CO_BROADCAST in > >>> -fcoarray=single mode, where the optional but non-present (in the calling > >>> scope) stat variable was assigned to before checking for it being not > >>> present. > >>> > >>> Regtests fine on x86-64-linux/f33. Ok for trunk? > >>> > >>> Regards, > >>> Andre > >> > > > > -- > > Andre Vehreschild * Email: vehre ad gmx dot de > - > Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München > Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank > Thürauf -- Andre Vehreschild * Email: vehre ad gmx dot de
Re: [Ping^2, Patch, Fortran] PR100337 Should be able to pass non-present optional arguments to CO_BROADCAST
Hi all, the fix for PR100337 was submitted as da13e4ebebb07a47d5fb50eab8893f8fe38683df. Thanks for the review Tobias. @Tobias: You are right, caf_single does not get much testing. But this part (not providing a stat) is tested multiple times, because of the laziness. Nearly none of the tests in the testsuite provides a variable for stat. Will wait two weeks for any errors introduced by this patch before backporting to gcc-11, ok? Regards, Andre On Tue, 22 Jun 2021 10:37:27 +0200 Tobias Burnus wrote: > Hi Andre, > > On 22.06.21 09:40, Andre Vehreschild via Fortran wrote: > > To the questions: > > - I added a test only for -fcoarray=single because in the library case the > >optional stat is just propagated to the library, which is already tested > > a lot of times and which needs to handle the optional stat in any case. So > > an error there would have been detected in one of the earlier tests. I did > > not want to add unnecessary test overhead given that the tests already run > > for a long time. > Fair point. > > - I did not add tests for the other CO_* routines, i.e. CO_MIN, CO_MAX, > >CO_REDUCE or CO_SUM, that are also handled by this routine, because I > > believe that showing that the fix works for CO_BROADCAST shows that the > > others work, too. Because the four others do not have any special handling > > in their implementation in trans_intrinsic. Or do you mean other > > coarray-routines besides the five handled by conv_co_collective()? > Well, that relates more to the first point – for -fcoarray=lib, it > likely makes a difference. For -fcoarray=single not. If the former is > skipped, it is much less relevant for the second. > > If it is ok for you, I would apply the patch as is, or do you see a reason > > to add more tests? > > OK. > > Although, I am not that sure that libcaf_single gets that much testing. > On the other hand, -fcoarray=lib with -lcaf_single is also not that > relevant in the real world, either. > > Tobias > > - > Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München > Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank > Thürauf -- Andre Vehreschild * Email: vehre ad gmx dot de
