Re: [Patch, Fortran] CO_BROADCAST for derived types with allocatable components

[Alessandro Fanfarillo]

Hi Paul,
that message was a copy/paste leftover. It doesn't make any sense in
that part of the code, it's now removed.

Committed as revision 276164.

Thanks!


Il giorno dom 15 set 2019 alle ore 12:19 Paul Richard Thomas
 ha scritto:
>
> Hi Sandro,
>
> This patch looks fine to me. I have a question about the comment:
> "This code is obviously added because the finalizer is not trusted to
> free all memory."
> 'obviously'? Not to me :-( Maybe you can expand on this?
>
> As to the stat and errmsg: Leave them for the time being. However, an
> attempt will have to be made to implement F2018 "16.6 Collective
> subroutines". I don't know enough about the coarrays implementation to
> be able to help you about detecting these conditions. Maybe Tobias
> Burnus can help?
>
> OK to commit.
>
> Paul
>
> PS Sometime before very long, something will have to be done about the
> exponential code bloat that structure_alloc_comps. The more users that
> there are for it the tougher it is going to get!
>
> On Thu, 22 Aug 2019 at 18:41, Alessandro Fanfarillo
>  wrote:
> >
> > Dear all,
> > please find in attachment a preliminary patch that adds support to
> > co_broadcast for allocatable components of derived types.
> > The patch is currently ignoring the stat and errmsg arguments, mostly
> > because I am not sure how to handle them properly. I have created a
> > new data structure called used to pass those argument to the
> > preexisting structure_alloc_comps.
> > Suggestions on how to handle them are more than welcome :-)
> >
> > The patch builds correctly on x86_64 and it has been tested with
> > OpenCoarrays and the following test cases:
> >
> > https://github.com/sourceryinstitute/OpenCoarrays/blob/co_broadcast-derived-type/src/tests/unit/collectives/co_broadcast_allocatable_components.f90
> >
> > https://github.com/sourceryinstitute/OpenCoarrays/blob/co_broadcast-derived-type/src/tests/unit/collectives/co_broadcast_allocatable_components_array.f90
> >
> > Regards,
>
>
>
> --
> "If you can't explain it simply, you don't understand it well enough"
> - Albert Einstein

[Patch, Fortran] CO_BROADCAST for derived types with allocatable components

[Alessandro Fanfarillo]

Dear all,
please find in attachment a preliminary patch that adds support to
co_broadcast for allocatable components of derived types.
The patch is currently ignoring the stat and errmsg arguments, mostly
because I am not sure how to handle them properly. I have created a
new data structure called used to pass those argument to the
preexisting structure_alloc_comps.
Suggestions on how to handle them are more than welcome :-)

The patch builds correctly on x86_64 and it has been tested with
OpenCoarrays and the following test cases:

https://github.com/sourceryinstitute/OpenCoarrays/blob/co_broadcast-derived-type/src/tests/unit/collectives/co_broadcast_allocatable_components.f90

https://github.com/sourceryinstitute/OpenCoarrays/blob/co_broadcast-derived-type/src/tests/unit/collectives/co_broadcast_allocatable_components_array.f90

Regards,
commit b9458ff4414615263ed92d8965c93fd0a953f4a9
Author: Alessandro Fanfarillo 
Date:   Thu Aug 22 10:50:17 2019 -0600

Co_broadcast derived types with allocatable components

diff --git a/gcc/fortran/trans-array.c b/gcc/fortran/trans-array.c
index c8d74e588dd..005646f1359 100644
--- a/gcc/fortran/trans-array.c
+++ b/gcc/fortran/trans-array.c
@@ -8571,13 +8571,15 @@ gfc_caf_is_dealloc_only (int caf_mode)
 
 enum {DEALLOCATE_ALLOC_COMP = 1, NULLIFY_ALLOC_COMP,
   COPY_ALLOC_COMP, COPY_ONLY_ALLOC_COMP, REASSIGN_CAF_COMP,
-  ALLOCATE_PDT_COMP, DEALLOCATE_PDT_COMP, CHECK_PDT_DUMMY};
+  ALLOCATE_PDT_COMP, DEALLOCATE_PDT_COMP, CHECK_PDT_DUMMY,
+  BCAST_ALLOC_COMP};
 
 static gfc_actual_arglist *pdt_param_list;
 
 static tree
 structure_alloc_comps (gfc_symbol * der_type, tree decl,
-		   tree dest, int rank, int purpose, int caf_mode)
+		   tree dest, int rank, int purpose, int caf_mode,
+		   gfc_co_subroutines_args *args)
 {
   gfc_component *c;
   gfc_loopinfo loop;
@@ -8663,14 +8665,14 @@ structure_alloc_comps (gfc_symbol * der_type, tree decl,
 	  && !caf_enabled (caf_mode))
 	{
 	  tmp = build_fold_indirect_ref_loc (input_location,
-	 gfc_conv_array_data (dest));
+	 gfc_conv_array_data (dest));
 	  dref = gfc_build_array_ref (tmp, index, NULL);
 	  tmp = structure_alloc_comps (der_type, vref, dref, rank,
-   COPY_ALLOC_COMP, 0);
+   COPY_ALLOC_COMP, 0, args);
 	}
   else
 	tmp = structure_alloc_comps (der_type, vref, NULL_TREE, rank, purpose,
- caf_mode);
+ caf_mode, args);
 
   gfc_add_expr_to_block (, tmp);
 
@@ -8704,13 +8706,13 @@ structure_alloc_comps (gfc_symbol * der_type, tree decl,
   if (purpose == DEALLOCATE_ALLOC_COMP && der_type->attr.pdt_type)
 {
   tmp = structure_alloc_comps (der_type, decl, NULL_TREE, rank,
-   DEALLOCATE_PDT_COMP, 0);
+   DEALLOCATE_PDT_COMP, 0, args);
   gfc_add_expr_to_block (, tmp);
 }
   else if (purpose == ALLOCATE_PDT_COMP && der_type->attr.alloc_comp)
 {
   tmp = structure_alloc_comps (der_type, decl, NULL_TREE, rank,
-   NULLIFY_ALLOC_COMP, 0);
+   NULLIFY_ALLOC_COMP, 0, args);
   gfc_add_expr_to_block (, tmp);
 }
 
@@ -8732,6 +8734,128 @@ structure_alloc_comps (gfc_symbol * der_type, tree decl,
 
   switch (purpose)
 	{
+
+	case BCAST_ALLOC_COMP:
+
+	  tree ubound;
+	  tree cdesc;
+	  stmtblock_t derived_type_block;
+
+	  gfc_init_block ();
+
+	  comp = fold_build3_loc (input_location, COMPONENT_REF, ctype,
+  decl, cdecl, NULL_TREE);
+
+	  /* Shortcut to get the attributes of the component.  */
+	  if (c->ts.type == BT_CLASS)
+	{
+	  attr = _DATA (c)->attr;
+	  if (attr->class_pointer)
+		continue;
+	}
+	  else
+	{
+	  attr = >attr;
+	  if (attr->pointer)
+		continue;
+	}
+
+	  add_when_allocated = NULL_TREE;
+	  if (cmp_has_alloc_comps
+	  && !c->attr.pointer && !c->attr.proc_pointer)
+	{
+	  /* Add checked deallocation of the components.  This code is
+		 obviously added because the finalizer is not trusted to free
+		 all memory.  */
+	  if (c->ts.type == BT_CLASS)
+		{
+		  rank = CLASS_DATA (c)->as ? CLASS_DATA (c)->as->rank : 0;
+		  add_when_allocated
+		  = structure_alloc_comps (CLASS_DATA (c)->ts.u.derived,
+	   comp, NULL_TREE, rank, purpose,
+	   caf_mode, args);
+		}
+	  else
+		{
+		  rank = c->as ? c->as->rank : 0;
+		  add_when_allocated = structure_alloc_comps (c->ts.u.derived,
+			  comp, NULL_TREE,
+			  rank, purpose,
+			  caf_mode, args);
+		}
+	}
+
+	  gfc_init_block (_type_block);
+	  if (add_when_allocated)
+	gfc_add_expr_to_block (_type_block, add_when_allocated);
+	  tmp = gfc_finish_block (_type_block);
+	  gfc_add_expr_to_block (, tmp);
+
+	  /* Convert the component into a rank 1 descriptor type.  */
+	  if (attr->dimension)
+	{
+	  tmp = gfc_get_element_type (TREE_TYPE (comp));
+	  ubound = gfc_full_array_size (, comp,
+	c->

Re: [PATCH, fortran] Support Fortran 2018 teams

[Alessandro Fanfarillo]

Committed revision 257105.

Thanks.

On Wed, Jan 24, 2018 at 3:17 PM, Jakub Jelinek <ja...@redhat.com> wrote:
> On Wed, Jan 24, 2018 at 08:19:58PM +, Paul Richard Thomas wrote:
>> (Jakub, This is all hidden behind the -fcoarray option. To my mind
>> this is safe for release.)
>
> Ok from RM POV.
>
> Jakub



-- 

Alessandro Fanfarillo, Ph.D.
Postdoctoral Researcher
National Center for Atmospheric Research
Mesa Lab, Boulder, CO, USA
303-497-1229

Re: [PATCH, fortran] Support Fortran 2018 teams

[Alessandro Fanfarillo]

I can confirm that the little change suggested by Steve passes the
regtests (on x86_64-pc-linux-gnu) and the regular tests using
OpenCoarrays.

On Fri, Jan 19, 2018 at 10:33 AM, Steve Kargl
<s...@troutmask.apl.washington.edu> wrote:
> On Fri, Jan 19, 2018 at 09:18:14AM -0800, Damian Rouson wrote:
>> Thanks for catching that, Steve, and for responding, Alessandro.
>>
>> Anything else?
>>
>
> I've only just started to look at the patch.   Unfortunately,
> I know zero about teams, so need to read the patch and F2018
> standard simultaneously.
>
> --
> Steve



-- 

Alessandro Fanfarillo, Ph.D.
Postdoctoral Researcher
National Center for Atmospheric Research
Mesa Lab, Boulder, CO, USA
303-497-1229

Re: [PATCH, fortran] Support Fortran 2018 teams

[Alessandro Fanfarillo]

Yes, definitively ar->team.

On Fri, Jan 19, 2018 at 8:36 AM, Steve Kargl
<s...@troutmask.apl.washington.edu> wrote:
> index 882fe577b76..b4baf5be554 100644
> --- a/gcc/fortran/array.c
> +++ b/gcc/fortran/array.c
> @@ -158,6 +158,7 @@ gfc_match_array_ref (gfc_array_ref *ar, gfc_array_spec
> *as, int init,
>bool matched_bracket = false;
>gfc_expr *tmp;
>bool stat_just_seen = false;
> +  bool team_just_seen = false;
>
>memset (ar, '\0', sizeof (*ar));
>
> @@ -230,8 +231,21 @@ coarray:
>if (m == MATCH_ERROR)
> return MATCH_ERROR;
>
> +  team_just_seen = false;
>stat_just_seen = false;
> -  if (gfc_match(" , stat = %e",) == MATCH_YES && ar->stat == NULL)
> +  if (gfc_match (" , team = %e", ) == MATCH_YES && ar->stat == NULL)
>
>
> Is the 2nd ar->stat suppose to be ar->team?
>
> --
> Steve



-- 

Alessandro Fanfarillo, Ph.D.
Postdoctoral Researcher
National Center for Atmospheric Research
Mesa Lab, Boulder, CO, USA
303-497-1229

Re: [PATCH, Fortran, Coarray, v1] Add support for failed images

[Alessandro Fanfarillo]

Hi Andre,
thanks for your work on the patch. I agree with you about exit(0)
statement in libcaf_single.
Could you please add my name and contact (Alessandro Fanfarillo
<fanfarillo@gmail.com>) below yours in the changelog?

Thanks,
Alessandro


2017-03-04 10:58 GMT-07:00 Andre Vehreschild <ve...@gmx.de>:
> Hi all,
>
> attached patch polishes the one begun by Alessandro. It adds documentation and
> fixes the style issues. Furthermore did I try to interpret the standard
> according to the FAIL IMAGE statement. IMHO should it just quit the executable
> without any error code. The caf_single library emits "FAIL IMAGE" to stderr,
> while in coarray=single mode it just quits. What do you think?
>
> Bootstraps and regtests ok on x86_64-linux/f25. Ok for trunk? (May be later).
>
> Gruß,
> Andre
> --
> Andre Vehreschild * Email: vehre ad gmx dot de

Re: [Fortran, Patch, CAF] Failed Images patch (TS 18508)

[Alessandro Fanfarillo]

Now with the patch attached.

2017-02-13 13:35 GMT-07:00 Alessandro Fanfarillo <fanfarillo@gmail.com>:
> Thanks Jerry. That test case is supposed only to be compiled (it never
> runs). Anyway, the attached patch has been modified according to your
> suggestion.
>
> Patch built and regtested on x86_64-pc-linux-gnu.
>
> 2017-02-12 10:24 GMT-07:00 Jerry DeLisle <jvdeli...@charter.net>:
>> On 02/11/2017 03:02 PM, Alessandro Fanfarillo wrote:
>>>
>>> Dear all,
>>> please find in attachment a new patch following the discussion at
>>> https://gcc.gnu.org/ml/fortran/2017-01/msg00054.html.
>>>
>>> Suggestions on how to fix potential issues are more than welcome.
>>>
>>> Regards,
>>> Alessandro
>>>
>>
>> On the failed images test:
>>
>> program test_image_status
>> +  implicit none
>> +
>> +  write(*,*) image_status(1)
>> +
>>
>> Write to a string and test the results.
>>
>> I assume you have regression tested this again as stated in the earlier
>> discussion.
>>
>> I think this is OK to go in.
>>
>> Jerry
>>
>>
commit 06ed189ff99710d4d18fefa7a83e12192c5d10bf
Author: Alessandro Fanfarillo <elfa...@ucar.edu>
Date:   Mon Feb 13 12:54:22 2017 -0700

Resurrected patch and tests - REV1

diff --git a/gcc/fortran/check.c b/gcc/fortran/check.c
index c22bfa9..ed88a19 100644
--- a/gcc/fortran/check.c
+++ b/gcc/fortran/check.c
@@ -1136,6 +1136,116 @@ gfc_check_atomic_ref (gfc_expr *value, gfc_expr *atom, 
gfc_expr *stat)
   return gfc_check_atomic (atom, 1, value, 0, stat, 2);
 }
 
+bool
+gfc_check_image_status (gfc_expr *image, gfc_expr *team)
+{
+  if (!type_check (image, 1, BT_INTEGER))
+return false;
+
+  int i = gfc_validate_kind (BT_INTEGER, image->ts.kind, false);
+  int j = gfc_validate_kind (BT_INTEGER, gfc_default_integer_kind, false);
+
+  if (gfc_integer_kinds[i].range < gfc_integer_kinds[j].range)
+{
+  gfc_error ("IMAGE argument of the IMAGE_STATUS intrinsic function at %L "
+"shall have at least the range of the default integer",
+>where);
+  return false;
+}
+
+  j = gfc_validate_kind (BT_INTEGER, gfc_default_integer_kind*2, false);
+
+  if (gfc_integer_kinds[i].range > gfc_integer_kinds[j].range)
+{
+  gfc_error ("IMAGE argument of the IMAGE_STATUS intrinsic function at %L "
+"shall have at most the range of the double precision integer",
+>where);
+  return false;
+}
+
+  if (team)
+{
+  gfc_error ("TEAM argument of the IMAGE_STATUS intrinsic function at %L "
+"not yet supported",
+>where);
+  return false;
+}
+  return true;
+}
+
+bool
+gfc_check_failed_images (gfc_expr *team, gfc_expr *kind)
+{
+  if (team)
+{
+  gfc_error ("TEAM argument of the FAILED_IMAGES intrinsic function "
+"at %L not yet supported", >where);
+  return false;
+}
+
+  if (kind)
+{
+  int i = gfc_validate_kind (BT_INTEGER, kind->ts.kind, false);
+  int j = gfc_validate_kind (BT_INTEGER, gfc_default_integer_kind, false);
+
+  if (gfc_integer_kinds[i].range < gfc_integer_kinds[j].range)
+   {
+ gfc_error ("KIND argument of the FAILED_IMAGES intrinsic function "
+"at %L shall have at least the range "
+"of the default integer", >where);
+ return false;
+   }
+
+  j = gfc_validate_kind (BT_INTEGER, gfc_default_integer_kind*2, false);
+
+  if (gfc_integer_kinds[i].range > gfc_integer_kinds[j].range)
+   {
+ gfc_error ("KIND argument of the FAILED_IMAGES "
+"intrinsic function at %L shall have at most the "
+"range of the double precision integer",
+>where);
+ return false;
+   }
+}
+  return true;
+}
+
+bool
+gfc_check_stopped_images (gfc_expr *team, gfc_expr *kind)
+{
+  if (team)
+{
+  gfc_error ("TEAM argument of the STOPPED_IMAGES intrinsic function "
+"at %L not yet supported", >where);
+  return false;
+}
+
+  if (kind)
+{
+  int i = gfc_validate_kind (BT_INTEGER, kind->ts.kind, false);
+  int j = gfc_validate_kind (BT_INTEGER, gfc_default_integer_kind, false);
+
+  if (gfc_integer_kinds[i].range < gfc_integer_kinds[j].range)
+   {
+ gfc_error ("KIND argument of the STOPPED_IMAGES intrinsic function "
+"at %L shall have at least the range "
+"of the default integer", >where);
+ return fal

Re: [Fortran, Patch, CAF] Failed Images patch (TS 18508)

[Alessandro Fanfarillo]

Thanks Jerry. That test case is supposed only to be compiled (it never
runs). Anyway, the attached patch has been modified according to your
suggestion.

Patch built and regtested on x86_64-pc-linux-gnu.

2017-02-12 10:24 GMT-07:00 Jerry DeLisle <jvdeli...@charter.net>:
> On 02/11/2017 03:02 PM, Alessandro Fanfarillo wrote:
>>
>> Dear all,
>> please find in attachment a new patch following the discussion at
>> https://gcc.gnu.org/ml/fortran/2017-01/msg00054.html.
>>
>> Suggestions on how to fix potential issues are more than welcome.
>>
>> Regards,
>> Alessandro
>>
>
> On the failed images test:
>
> program test_image_status
> +  implicit none
> +
> +  write(*,*) image_status(1)
> +
>
> Write to a string and test the results.
>
> I assume you have regression tested this again as stated in the earlier
> discussion.
>
> I think this is OK to go in.
>
> Jerry
>
>

[Fortran, Patch, CAF] Failed Images patch (TS 18508)

[Alessandro Fanfarillo]

Dear all,
please find in attachment a new patch following the discussion at
https://gcc.gnu.org/ml/fortran/2017-01/msg00054.html.

Suggestions on how to fix potential issues are more than welcome.

Regards,
Alessandro
commit e2dad3cc56447daea85c147f08b3f58a8e70617f
Author: Alessandro Fanfarillo <elfa...@ucar.edu>
Date:   Fri Feb 10 13:45:37 2017 -0700

Resurrected patch and tests

diff --git a/gcc/fortran/check.c b/gcc/fortran/check.c
index c22bfa9..ed88a19 100644
--- a/gcc/fortran/check.c
+++ b/gcc/fortran/check.c
@@ -1136,6 +1136,116 @@ gfc_check_atomic_ref (gfc_expr *value, gfc_expr *atom, 
gfc_expr *stat)
   return gfc_check_atomic (atom, 1, value, 0, stat, 2);
 }

+bool
+gfc_check_image_status (gfc_expr *image, gfc_expr *team)
+{
+  if (!type_check (image, 1, BT_INTEGER))
+return false;
+
+  int i = gfc_validate_kind (BT_INTEGER, image->ts.kind, false);
+  int j = gfc_validate_kind (BT_INTEGER, gfc_default_integer_kind, false);
+
+  if (gfc_integer_kinds[i].range < gfc_integer_kinds[j].range)
+{
+  gfc_error ("IMAGE argument of the IMAGE_STATUS intrinsic function at %L "
+"shall have at least the range of the default integer",
+>where);
+  return false;
+}
+
+  j = gfc_validate_kind (BT_INTEGER, gfc_default_integer_kind*2, false);
+
+  if (gfc_integer_kinds[i].range > gfc_integer_kinds[j].range)
+{
+  gfc_error ("IMAGE argument of the IMAGE_STATUS intrinsic function at %L "
+"shall have at most the range of the double precision integer",
+>where);
+  return false;
+}
+
+  if (team)
+{
+  gfc_error ("TEAM argument of the IMAGE_STATUS intrinsic function at %L "
+"not yet supported",
+>where);
+  return false;
+}
+  return true;
+}
+
+bool
+gfc_check_failed_images (gfc_expr *team, gfc_expr *kind)
+{
+  if (team)
+{
+  gfc_error ("TEAM argument of the FAILED_IMAGES intrinsic function "
+"at %L not yet supported", >where);
+  return false;
+}
+
+  if (kind)
+{
+  int i = gfc_validate_kind (BT_INTEGER, kind->ts.kind, false);
+  int j = gfc_validate_kind (BT_INTEGER, gfc_default_integer_kind, false);
+
+  if (gfc_integer_kinds[i].range < gfc_integer_kinds[j].range)
+   {
+ gfc_error ("KIND argument of the FAILED_IMAGES intrinsic function "
+"at %L shall have at least the range "
+"of the default integer", >where);
+ return false;
+   }
+
+  j = gfc_validate_kind (BT_INTEGER, gfc_default_integer_kind*2, false);
+
+  if (gfc_integer_kinds[i].range > gfc_integer_kinds[j].range)
+   {
+ gfc_error ("KIND argument of the FAILED_IMAGES "
+"intrinsic function at %L shall have at most the "
+"range of the double precision integer",
+>where);
+ return false;
+   }
+}
+  return true;
+}
+
+bool
+gfc_check_stopped_images (gfc_expr *team, gfc_expr *kind)
+{
+  if (team)
+{
+  gfc_error ("TEAM argument of the STOPPED_IMAGES intrinsic function "
+"at %L not yet supported", >where);
+  return false;
+}
+
+  if (kind)
+{
+  int i = gfc_validate_kind (BT_INTEGER, kind->ts.kind, false);
+  int j = gfc_validate_kind (BT_INTEGER, gfc_default_integer_kind, false);
+
+  if (gfc_integer_kinds[i].range < gfc_integer_kinds[j].range)
+   {
+ gfc_error ("KIND argument of the STOPPED_IMAGES intrinsic function "
+"at %L shall have at least the range "
+"of the default integer", >where);
+ return false;
+   }
+
+  j = gfc_validate_kind (BT_INTEGER, gfc_default_integer_kind*2, false);
+
+  if (gfc_integer_kinds[i].range > gfc_integer_kinds[j].range)
+   {
+ gfc_error ("KIND argument of the STOPPED_IMAGES "
+"intrinsic function at %L shall have at most the "
+"range of the double precision integer",
+>where);
+ return false;
+   }
+}
+  return true;
+}
 
 bool
 gfc_check_atomic_cas (gfc_expr *atom, gfc_expr *old, gfc_expr *compare,
diff --git a/gcc/fortran/dump-parse-tree.c b/gcc/fortran/dump-parse-tree.c
index 36fc4cc..4525573 100644
--- a/gcc/fortran/dump-parse-tree.c
+++ b/gcc/fortran/dump-parse-tree.c
@@ -1818,6 +1818,9 @@ show_code_node (int level, gfc_code *c)
 
   break;
 
+case EXEC_FAIL_IMAGE:
+  fputs ("FAIL IMAGE ", dumpfile);
+
 case EXEC_SYNC_ALL:
   fputs ("SYNC ALL ", dumpfile);
   if (c->expr2 != NULL)
diff --git a/gcc

Re: [Fortran, Patch] First patch for coarray FAILED IMAGES (TS 18508)

[Alessandro Fanfarillo]

* PING *

2016-09-21 19:03 GMT+01:00 Alessandro Fanfarillo <fanfarillo@gmail.com>:
> Thanks Andre.
>
> 2016-09-19 9:55 GMT-06:00 Andre Vehreschild <ve...@gmx.de>:
>> Hi Alessandro,
>
>> The if in resolve.c at 8837: resolve_failed_image (... is intentional? It is
>> doing nothing. So do you plan to add more code, or will there never be
>> anything. If the later I recommend to just put a comment there and remove the
>> empty if.
>
> I added the if statement during the development and I forgot to remove it.
>
>>
>> There still is no test when -fcoarray=single is used. This shouldn't be so
>> hard, should it?
>
> Done.
>
> Built and regtested on x86_64-pc-linux-gnu.
>
>>
>> Regards,
>> Andre
>>
>> On Mon, 19 Sep 2016 08:30:12 -0700
>> Alessandro Fanfarillo <fanfarillo@gmail.com> wrote:
>>
>>> * PING *
>>>
>>> On Sep 7, 2016 3:01 PM, "Alessandro Fanfarillo" <fanfarillo@gmail.com>
>>> wrote:
>>>
>>> > Dear all,
>>> > the attached patch supports failed images also when -fcoarray=single is
>>> > used.
>>> >
>>> > Built and regtested on x86_64-pc-linux-gnu.
>>> >
>>> > Cheers,
>>> > Alessandro
>>> >
>>> > 2016-08-09 5:22 GMT-06:00 Paul Richard Thomas <
>>> > paul.richard.tho...@gmail.com>:
>>> > > Hi Sandro,
>>> > >
>>> > > As far as I can see, this is OK barring a couple of minor wrinkles and
>>> > > a question:
>>> > >
>>> > > For coarray_failed_images_err.f90 and coarray_image_status_err.f90 you
>>> > > have used the option -fdump-tree-original without making use of the
>>> > > tree dump.
>>> > >
>>> > > Mikael asked you to provide an executable test with -fcoarray=single.
>>> > > Is this not possible for some reason?
>>> > >
>>> > > Otherwise, this is OK for trunk.
>>> > >
>>> > > Thanks for the patch.
>>> > >
>>> > > Paul
>>> > >
>>> > > On 4 August 2016 at 05:07, Alessandro Fanfarillo
>>> > > <fanfarillo@gmail.com> wrote:
>>> > >> * PING *
>>> > >>
>>> > >> 2016-07-21 13:05 GMT-06:00 Alessandro Fanfarillo <
>>> > fanfarillo@gmail.com>:
>>> > >>> Dear Mikael and all,
>>> > >>>
>>> > >>> in attachment the new patch, built and regtested on
>>> > x86_64-pc-linux-gnu.
>>> > >>>
>>> > >>> Cheers,
>>> > >>> Alessandro
>>> > >>>
>>> > >>> 2016-07-20 13:17 GMT-06:00 Mikael Morin <morin-mik...@orange.fr>:
>>> > >>>> Le 20/07/2016 à 11:39, Andre Vehreschild a écrit :
>>> > >>>>>
>>> > >>>>> Hi Mikael,
>>> > >>>>>
>>> > >>>>>
>>> > >>>>>>> +  if(st == ST_FAIL_IMAGE)
>>> > >>>>>>> +new_st.op = EXEC_FAIL_IMAGE;
>>> > >>>>>>> +  else
>>> > >>>>>>> +gcc_unreachable();
>>> > >>>>>>
>>> > >>>>>> You can use
>>> > >>>>>> gcc_assert (st == ST_FAIL_IMAGE);
>>> > >>>>>> foo...;
>>> > >>>>>> instead of
>>> > >>>>>> if (st == ST_FAIL_IMAGE)
>>> > >>>>>> foo...;
>>> > >>>>>> else
>>> > >>>>>> gcc_unreachable ();
>>> > >>>>>
>>> > >>>>>
>>> > >>>>> Be careful, this is not 100% identical in the general case. For 
>>> > >>>>> older
>>> > >>>>> gcc version (gcc < 4008) gcc_assert() is mapped to nothing, esp. not
>>> > to
>>> > >>>>> an abort(), so the behavior can change. But in this case everything
>>> > is
>>> > >>>>> fine, because the patch is most likely not backported.
>>> > >>>>>
>>> > >>>> Didn't know about this. The difference seems

Re: [Fortran, Patch] First patch for coarray FAILED IMAGES (TS 18508)

[Alessandro Fanfarillo]

Thanks Andre.

2016-09-19 9:55 GMT-06:00 Andre Vehreschild <ve...@gmx.de>:
> Hi Alessandro,

> The if in resolve.c at 8837: resolve_failed_image (... is intentional? It is
> doing nothing. So do you plan to add more code, or will there never be
> anything. If the later I recommend to just put a comment there and remove the
> empty if.

I added the if statement during the development and I forgot to remove it.

>
> There still is no test when -fcoarray=single is used. This shouldn't be so
> hard, should it?

Done.

Built and regtested on x86_64-pc-linux-gnu.

>
> Regards,
> Andre
>
> On Mon, 19 Sep 2016 08:30:12 -0700
> Alessandro Fanfarillo <fanfarillo@gmail.com> wrote:
>
>> * PING *
>>
>> On Sep 7, 2016 3:01 PM, "Alessandro Fanfarillo" <fanfarillo@gmail.com>
>> wrote:
>>
>> > Dear all,
>> > the attached patch supports failed images also when -fcoarray=single is
>> > used.
>> >
>> > Built and regtested on x86_64-pc-linux-gnu.
>> >
>> > Cheers,
>> > Alessandro
>> >
>> > 2016-08-09 5:22 GMT-06:00 Paul Richard Thomas <
>> > paul.richard.tho...@gmail.com>:
>> > > Hi Sandro,
>> > >
>> > > As far as I can see, this is OK barring a couple of minor wrinkles and
>> > > a question:
>> > >
>> > > For coarray_failed_images_err.f90 and coarray_image_status_err.f90 you
>> > > have used the option -fdump-tree-original without making use of the
>> > > tree dump.
>> > >
>> > > Mikael asked you to provide an executable test with -fcoarray=single.
>> > > Is this not possible for some reason?
>> > >
>> > > Otherwise, this is OK for trunk.
>> > >
>> > > Thanks for the patch.
>> > >
>> > > Paul
>> > >
>> > > On 4 August 2016 at 05:07, Alessandro Fanfarillo
>> > > <fanfarillo@gmail.com> wrote:
>> > >> * PING *
>> > >>
>> > >> 2016-07-21 13:05 GMT-06:00 Alessandro Fanfarillo <
>> > fanfarillo@gmail.com>:
>> > >>> Dear Mikael and all,
>> > >>>
>> > >>> in attachment the new patch, built and regtested on
>> > x86_64-pc-linux-gnu.
>> > >>>
>> > >>> Cheers,
>> > >>> Alessandro
>> > >>>
>> > >>> 2016-07-20 13:17 GMT-06:00 Mikael Morin <morin-mik...@orange.fr>:
>> > >>>> Le 20/07/2016 à 11:39, Andre Vehreschild a écrit :
>> > >>>>>
>> > >>>>> Hi Mikael,
>> > >>>>>
>> > >>>>>
>> > >>>>>>> +  if(st == ST_FAIL_IMAGE)
>> > >>>>>>> +new_st.op = EXEC_FAIL_IMAGE;
>> > >>>>>>> +  else
>> > >>>>>>> +gcc_unreachable();
>> > >>>>>>
>> > >>>>>> You can use
>> > >>>>>> gcc_assert (st == ST_FAIL_IMAGE);
>> > >>>>>> foo...;
>> > >>>>>> instead of
>> > >>>>>> if (st == ST_FAIL_IMAGE)
>> > >>>>>> foo...;
>> > >>>>>> else
>> > >>>>>> gcc_unreachable ();
>> > >>>>>
>> > >>>>>
>> > >>>>> Be careful, this is not 100% identical in the general case. For older
>> > >>>>> gcc version (gcc < 4008) gcc_assert() is mapped to nothing, esp. not
>> > to
>> > >>>>> an abort(), so the behavior can change. But in this case everything
>> > is
>> > >>>>> fine, because the patch is most likely not backported.
>> > >>>>>
>> > >>>> Didn't know about this. The difference seems to be very subtle.
>> > >>>> I don't mind much anyway. The original version can stay if preferred,
>> > this
>> > >>>> was just a suggestion.
>> > >>>>
>> > >>>> By the way, if the function is inlined in its single caller, the
>> > assert or
>> > >>>> unreachable statement can be removed, which avoids choosing between
>> > them.
>> > >>>> That's another suggestion.
>> > >>>>
>> > >>

Re: [Fortran, Patch] First patch for coarray FAILED IMAGES (TS 18508)

[Alessandro Fanfarillo]

Dear all,
the attached patch supports failed images also when -fcoarray=single is used.

Built and regtested on x86_64-pc-linux-gnu.

Cheers,
Alessandro

2016-08-09 5:22 GMT-06:00 Paul Richard Thomas <paul.richard.tho...@gmail.com>:
> Hi Sandro,
>
> As far as I can see, this is OK barring a couple of minor wrinkles and
> a question:
>
> For coarray_failed_images_err.f90 and coarray_image_status_err.f90 you
> have used the option -fdump-tree-original without making use of the
> tree dump.
>
> Mikael asked you to provide an executable test with -fcoarray=single.
> Is this not possible for some reason?
>
> Otherwise, this is OK for trunk.
>
> Thanks for the patch.
>
> Paul
>
> On 4 August 2016 at 05:07, Alessandro Fanfarillo
> <fanfarillo@gmail.com> wrote:
>> * PING *
>>
>> 2016-07-21 13:05 GMT-06:00 Alessandro Fanfarillo <fanfarillo@gmail.com>:
>>> Dear Mikael and all,
>>>
>>> in attachment the new patch, built and regtested on x86_64-pc-linux-gnu.
>>>
>>> Cheers,
>>> Alessandro
>>>
>>> 2016-07-20 13:17 GMT-06:00 Mikael Morin <morin-mik...@orange.fr>:
>>>> Le 20/07/2016 à 11:39, Andre Vehreschild a écrit :
>>>>>
>>>>> Hi Mikael,
>>>>>
>>>>>
>>>>>>> +  if(st == ST_FAIL_IMAGE)
>>>>>>> +new_st.op = EXEC_FAIL_IMAGE;
>>>>>>> +  else
>>>>>>> +gcc_unreachable();
>>>>>>
>>>>>> You can use
>>>>>> gcc_assert (st == ST_FAIL_IMAGE);
>>>>>> foo...;
>>>>>> instead of
>>>>>> if (st == ST_FAIL_IMAGE)
>>>>>> foo...;
>>>>>> else
>>>>>> gcc_unreachable ();
>>>>>
>>>>>
>>>>> Be careful, this is not 100% identical in the general case. For older
>>>>> gcc version (gcc < 4008) gcc_assert() is mapped to nothing, esp. not to
>>>>> an abort(), so the behavior can change. But in this case everything is
>>>>> fine, because the patch is most likely not backported.
>>>>>
>>>> Didn't know about this. The difference seems to be very subtle.
>>>> I don't mind much anyway. The original version can stay if preferred, this
>>>> was just a suggestion.
>>>>
>>>> By the way, if the function is inlined in its single caller, the assert or
>>>> unreachable statement can be removed, which avoids choosing between them.
>>>> That's another suggestion.
>>>>
>>>>
>>>>>>> +
>>>>>>> +  return MATCH_YES;
>>>>>>> +
>>>>>>> + syntax:
>>>>>>> +  gfc_syntax_error (st);
>>>>>>> +
>>>>>>> +  return MATCH_ERROR;
>>>>>>> +}
>>>>>>> +
>>>>>>> +match
>>>>>>> +gfc_match_fail_image (void)
>>>>>>> +{
>>>>>>> +  /* if (!gfc_notify_std (GFC_STD_F2008_TS, "FAIL IMAGE statement
>>>>>>> at %C")) */
>>>>>>> +  /*   return MATCH_ERROR; */
>>>>>>> +
>>>>>>
>>>>>> Can this be uncommented?
>>>>>>
>>>>>>> +  return fail_image_statement (ST_FAIL_IMAGE);
>>>>>>> +}
>>>>>>>
>>>>>>>  /* Match LOCK/UNLOCK statement. Syntax:
>>>>>>>   LOCK ( lock-variable [ , lock-stat-list ] )
>>>>>>> diff --git a/gcc/fortran/trans-intrinsic.c
>>>>>>> b/gcc/fortran/trans-intrinsic.c index 1aaf4e2..b2f5596 100644
>>>>>>> --- a/gcc/fortran/trans-intrinsic.c
>>>>>>> +++ b/gcc/fortran/trans-intrinsic.c
>>>>>>> @@ -1647,6 +1647,24 @@ trans_this_image (gfc_se * se, gfc_expr
>>>>>>> *expr) m, lbound));
>>>>>>>  }
>>>>>>>
>>>>>>> +static void
>>>>>>> +gfc_conv_intrinsic_image_status (gfc_se *se, gfc_expr *expr)
>>>>>>> +{
>>>>>>> +  unsigned int num_args;
>>>>>>> +  tree *args,tmp;
>>>>>>> +
>>>>>>> +  num_args = gfc_intrinsic_argument_list_length (expr);
>>>>>>> +  args = XALLOCAVEC (tree, num_args);
>&g

Re: [Fortran, Patch] First patch for coarray FAILED IMAGES (TS 18508)

[Alessandro Fanfarillo]

Thanks Paul,

I fixed the unused -fdump-tree-original on the tests.

About -fcoarray=single, I agree with Andre about not producing code
for failed images functions when running in single-image mode. If you,
or anybody else, thing otherwise I can adjust the functions to return
a constant value (except for fail image... :)).


2016-08-09 5:22 GMT-06:00 Paul Richard Thomas <paul.richard.tho...@gmail.com>:
> Hi Sandro,
>
> As far as I can see, this is OK barring a couple of minor wrinkles and
> a question:
>
> For coarray_failed_images_err.f90 and coarray_image_status_err.f90 you
> have used the option -fdump-tree-original without making use of the
> tree dump.
>
> Mikael asked you to provide an executable test with -fcoarray=single.
> Is this not possible for some reason?
>
> Otherwise, this is OK for trunk.
>
> Thanks for the patch.
>
> Paul
>
> On 4 August 2016 at 05:07, Alessandro Fanfarillo
> <fanfarillo@gmail.com> wrote:
>> * PING *
>>
>> 2016-07-21 13:05 GMT-06:00 Alessandro Fanfarillo <fanfarillo@gmail.com>:
>>> Dear Mikael and all,
>>>
>>> in attachment the new patch, built and regtested on x86_64-pc-linux-gnu.
>>>
>>> Cheers,
>>> Alessandro
>>>
>>> 2016-07-20 13:17 GMT-06:00 Mikael Morin <morin-mik...@orange.fr>:
>>>> Le 20/07/2016 à 11:39, Andre Vehreschild a écrit :
>>>>>
>>>>> Hi Mikael,
>>>>>
>>>>>
>>>>>>> +  if(st == ST_FAIL_IMAGE)
>>>>>>> +new_st.op = EXEC_FAIL_IMAGE;
>>>>>>> +  else
>>>>>>> +gcc_unreachable();
>>>>>>
>>>>>> You can use
>>>>>> gcc_assert (st == ST_FAIL_IMAGE);
>>>>>> foo...;
>>>>>> instead of
>>>>>> if (st == ST_FAIL_IMAGE)
>>>>>> foo...;
>>>>>> else
>>>>>> gcc_unreachable ();
>>>>>
>>>>>
>>>>> Be careful, this is not 100% identical in the general case. For older
>>>>> gcc version (gcc < 4008) gcc_assert() is mapped to nothing, esp. not to
>>>>> an abort(), so the behavior can change. But in this case everything is
>>>>> fine, because the patch is most likely not backported.
>>>>>
>>>> Didn't know about this. The difference seems to be very subtle.
>>>> I don't mind much anyway. The original version can stay if preferred, this
>>>> was just a suggestion.
>>>>
>>>> By the way, if the function is inlined in its single caller, the assert or
>>>> unreachable statement can be removed, which avoids choosing between them.
>>>> That's another suggestion.
>>>>
>>>>
>>>>>>> +
>>>>>>> +  return MATCH_YES;
>>>>>>> +
>>>>>>> + syntax:
>>>>>>> +  gfc_syntax_error (st);
>>>>>>> +
>>>>>>> +  return MATCH_ERROR;
>>>>>>> +}
>>>>>>> +
>>>>>>> +match
>>>>>>> +gfc_match_fail_image (void)
>>>>>>> +{
>>>>>>> +  /* if (!gfc_notify_std (GFC_STD_F2008_TS, "FAIL IMAGE statement
>>>>>>> at %C")) */
>>>>>>> +  /*   return MATCH_ERROR; */
>>>>>>> +
>>>>>>
>>>>>> Can this be uncommented?
>>>>>>
>>>>>>> +  return fail_image_statement (ST_FAIL_IMAGE);
>>>>>>> +}
>>>>>>>
>>>>>>>  /* Match LOCK/UNLOCK statement. Syntax:
>>>>>>>   LOCK ( lock-variable [ , lock-stat-list ] )
>>>>>>> diff --git a/gcc/fortran/trans-intrinsic.c
>>>>>>> b/gcc/fortran/trans-intrinsic.c index 1aaf4e2..b2f5596 100644
>>>>>>> --- a/gcc/fortran/trans-intrinsic.c
>>>>>>> +++ b/gcc/fortran/trans-intrinsic.c
>>>>>>> @@ -1647,6 +1647,24 @@ trans_this_image (gfc_se * se, gfc_expr
>>>>>>> *expr) m, lbound));
>>>>>>>  }
>>>>>>>
>>>>>>> +static void
>>>>>>> +gfc_conv_intrinsic_image_status (gfc_se *se, gfc_expr *expr)
>>>>>>> +{
>>>>>>> +  unsigned int num_args;
>>>>>>> +  tree *args,tmp;
>>>>>>> +
>>>>>>> +  num_args = gfc_intrinsic_argument_list_length (expr);
>>>>>>> +  args = XALLOCAVEC (tree, num_args);
>>>>>>> +
>>>>>>> +  gfc_conv_intrinsic_function_args (se, expr, args, num_args);
>>>>>>> +
>>>>>>> +  if (flag_coarray == GFC_FCOARRAY_LIB)
>>>>>>> +{
>>>>>>
>>>>>> Can everything be put under the if?
>>>>>> Does it work with -fcoarray=single?
>>>>>
>>>>>
>>>>> IMO coarray=single should not generate code here, therefore putting
>>>>> everything under the if should to fine.
>>>>>
>>>> My point was more avoiding generating code for the arguments if they are 
>>>> not
>>>> used in the end.
>>>> Regarding the -fcoarray=single case, the function returns a result, which
>>>> can be used in an expression, so I don't think it will work without at 
>>>> least
>>>> hardcoding a fixed value as result in that case.
>>>> But even that wouldn't be enough, as the function wouldn't work 
>>>> consistently
>>>> with the fail image statement.
>>>>
>>>>> Sorry for the comments ...
>>>>>
>>>> Comments are welcome here, as far as I know. ;-)
>>>>
>>>> Mikael
>
>
>
> --
> The difference between genius and stupidity is; genius has its limits.
>
> Albert Einstein

Re: [Fortran, Patch] First patch for coarray FAILED IMAGES (TS 18508)

[Alessandro Fanfarillo]

* PING *

2016-07-21 13:05 GMT-06:00 Alessandro Fanfarillo <fanfarillo@gmail.com>:
> Dear Mikael and all,
>
> in attachment the new patch, built and regtested on x86_64-pc-linux-gnu.
>
> Cheers,
> Alessandro
>
> 2016-07-20 13:17 GMT-06:00 Mikael Morin <morin-mik...@orange.fr>:
>> Le 20/07/2016 à 11:39, Andre Vehreschild a écrit :
>>>
>>> Hi Mikael,
>>>
>>>
>>>>> +  if(st == ST_FAIL_IMAGE)
>>>>> +new_st.op = EXEC_FAIL_IMAGE;
>>>>> +  else
>>>>> +gcc_unreachable();
>>>>
>>>> You can use
>>>> gcc_assert (st == ST_FAIL_IMAGE);
>>>> foo...;
>>>> instead of
>>>> if (st == ST_FAIL_IMAGE)
>>>> foo...;
>>>> else
>>>> gcc_unreachable ();
>>>
>>>
>>> Be careful, this is not 100% identical in the general case. For older
>>> gcc version (gcc < 4008) gcc_assert() is mapped to nothing, esp. not to
>>> an abort(), so the behavior can change. But in this case everything is
>>> fine, because the patch is most likely not backported.
>>>
>> Didn't know about this. The difference seems to be very subtle.
>> I don't mind much anyway. The original version can stay if preferred, this
>> was just a suggestion.
>>
>> By the way, if the function is inlined in its single caller, the assert or
>> unreachable statement can be removed, which avoids choosing between them.
>> That's another suggestion.
>>
>>
>>>>> +
>>>>> +  return MATCH_YES;
>>>>> +
>>>>> + syntax:
>>>>> +  gfc_syntax_error (st);
>>>>> +
>>>>> +  return MATCH_ERROR;
>>>>> +}
>>>>> +
>>>>> +match
>>>>> +gfc_match_fail_image (void)
>>>>> +{
>>>>> +  /* if (!gfc_notify_std (GFC_STD_F2008_TS, "FAIL IMAGE statement
>>>>> at %C")) */
>>>>> +  /*   return MATCH_ERROR; */
>>>>> +
>>>>
>>>> Can this be uncommented?
>>>>
>>>>> +  return fail_image_statement (ST_FAIL_IMAGE);
>>>>> +}
>>>>>
>>>>>  /* Match LOCK/UNLOCK statement. Syntax:
>>>>>   LOCK ( lock-variable [ , lock-stat-list ] )
>>>>> diff --git a/gcc/fortran/trans-intrinsic.c
>>>>> b/gcc/fortran/trans-intrinsic.c index 1aaf4e2..b2f5596 100644
>>>>> --- a/gcc/fortran/trans-intrinsic.c
>>>>> +++ b/gcc/fortran/trans-intrinsic.c
>>>>> @@ -1647,6 +1647,24 @@ trans_this_image (gfc_se * se, gfc_expr
>>>>> *expr) m, lbound));
>>>>>  }
>>>>>
>>>>> +static void
>>>>> +gfc_conv_intrinsic_image_status (gfc_se *se, gfc_expr *expr)
>>>>> +{
>>>>> +  unsigned int num_args;
>>>>> +  tree *args,tmp;
>>>>> +
>>>>> +  num_args = gfc_intrinsic_argument_list_length (expr);
>>>>> +  args = XALLOCAVEC (tree, num_args);
>>>>> +
>>>>> +  gfc_conv_intrinsic_function_args (se, expr, args, num_args);
>>>>> +
>>>>> +  if (flag_coarray == GFC_FCOARRAY_LIB)
>>>>> +{
>>>>
>>>> Can everything be put under the if?
>>>> Does it work with -fcoarray=single?
>>>
>>>
>>> IMO coarray=single should not generate code here, therefore putting
>>> everything under the if should to fine.
>>>
>> My point was more avoiding generating code for the arguments if they are not
>> used in the end.
>> Regarding the -fcoarray=single case, the function returns a result, which
>> can be used in an expression, so I don't think it will work without at least
>> hardcoding a fixed value as result in that case.
>> But even that wouldn't be enough, as the function wouldn't work consistently
>> with the fail image statement.
>>
>>> Sorry for the comments ...
>>>
>> Comments are welcome here, as far as I know. ;-)
>>
>> Mikael

Re: [Fortran, Patch] First patch for coarray FAILED IMAGES (TS 18508)

[Alessandro Fanfarillo]

Dear Mikael and all,

in attachment the new patch, built and regtested on x86_64-pc-linux-gnu.

Cheers,
Alessandro

2016-07-20 13:17 GMT-06:00 Mikael Morin <morin-mik...@orange.fr>:
> Le 20/07/2016 à 11:39, Andre Vehreschild a écrit :
>>
>> Hi Mikael,
>>
>>
>>>> +  if(st == ST_FAIL_IMAGE)
>>>> +new_st.op = EXEC_FAIL_IMAGE;
>>>> +  else
>>>> +gcc_unreachable();
>>>
>>> You can use
>>> gcc_assert (st == ST_FAIL_IMAGE);
>>> foo...;
>>> instead of
>>> if (st == ST_FAIL_IMAGE)
>>> foo...;
>>> else
>>> gcc_unreachable ();
>>
>>
>> Be careful, this is not 100% identical in the general case. For older
>> gcc version (gcc < 4008) gcc_assert() is mapped to nothing, esp. not to
>> an abort(), so the behavior can change. But in this case everything is
>> fine, because the patch is most likely not backported.
>>
> Didn't know about this. The difference seems to be very subtle.
> I don't mind much anyway. The original version can stay if preferred, this
> was just a suggestion.
>
> By the way, if the function is inlined in its single caller, the assert or
> unreachable statement can be removed, which avoids choosing between them.
> That's another suggestion.
>
>
>>>> +
>>>> +  return MATCH_YES;
>>>> +
>>>> + syntax:
>>>> +  gfc_syntax_error (st);
>>>> +
>>>> +  return MATCH_ERROR;
>>>> +}
>>>> +
>>>> +match
>>>> +gfc_match_fail_image (void)
>>>> +{
>>>> +  /* if (!gfc_notify_std (GFC_STD_F2008_TS, "FAIL IMAGE statement
>>>> at %C")) */
>>>> +  /*   return MATCH_ERROR; */
>>>> +
>>>
>>> Can this be uncommented?
>>>
>>>> +  return fail_image_statement (ST_FAIL_IMAGE);
>>>> +}
>>>>
>>>>  /* Match LOCK/UNLOCK statement. Syntax:
>>>>   LOCK ( lock-variable [ , lock-stat-list ] )
>>>> diff --git a/gcc/fortran/trans-intrinsic.c
>>>> b/gcc/fortran/trans-intrinsic.c index 1aaf4e2..b2f5596 100644
>>>> --- a/gcc/fortran/trans-intrinsic.c
>>>> +++ b/gcc/fortran/trans-intrinsic.c
>>>> @@ -1647,6 +1647,24 @@ trans_this_image (gfc_se * se, gfc_expr
>>>> *expr) m, lbound));
>>>>  }
>>>>
>>>> +static void
>>>> +gfc_conv_intrinsic_image_status (gfc_se *se, gfc_expr *expr)
>>>> +{
>>>> +  unsigned int num_args;
>>>> +  tree *args,tmp;
>>>> +
>>>> +  num_args = gfc_intrinsic_argument_list_length (expr);
>>>> +  args = XALLOCAVEC (tree, num_args);
>>>> +
>>>> +  gfc_conv_intrinsic_function_args (se, expr, args, num_args);
>>>> +
>>>> +  if (flag_coarray == GFC_FCOARRAY_LIB)
>>>> +{
>>>
>>> Can everything be put under the if?
>>> Does it work with -fcoarray=single?
>>
>>
>> IMO coarray=single should not generate code here, therefore putting
>> everything under the if should to fine.
>>
> My point was more avoiding generating code for the arguments if they are not
> used in the end.
> Regarding the -fcoarray=single case, the function returns a result, which
> can be used in an expression, so I don't think it will work without at least
> hardcoding a fixed value as result in that case.
> But even that wouldn't be enough, as the function wouldn't work consistently
> with the fail image statement.
>
>> Sorry for the comments ...
>>
> Comments are welcome here, as far as I know. ;-)
>
> Mikael
commit d6c91b2c14a12d1d012738f13f4920e207113982
Author: Alessandro Fanfarillo <elfa...@ucar.edu>
Date:   Thu Jul 21 10:01:33 2016 -0600

First review of failed images patch

diff --git a/gcc/fortran/check.c b/gcc/fortran/check.c
index d26e45e..121551c 100644
--- a/gcc/fortran/check.c
+++ b/gcc/fortran/check.c
@@ -1210,6 +1210,97 @@ gfc_check_event_query (gfc_expr *event, gfc_expr *count, 
gfc_expr *stat)
   return true;
 }
 
+bool
+gfc_check_image_status (gfc_expr *image, gfc_expr *team)
+{
+
+  if (flag_coarray != GFC_FCOARRAY_LIB)
+{
+  gfc_fatal_error ("Failed images features "
+  "usable only with %<-fcoarray=lib%>");
+  return false;
+}
+
+  if (!type_check (image, 1, BT_INTEGER))
+return false;
+
+  int i = gfc_validate_kind (BT_INTEGER, image->ts.kind, false);
+  int j = gfc_validate_kind (BT_INTEGER, gfc_default_integer_kind, false

Re: [Fortran, Patch] First patch for coarray FAILED IMAGES (TS 18508)

[Alessandro Fanfarillo]

Third *PING*

2016-07-04 16:46 GMT-06:00 Alessandro Fanfarillo <fanfarillo@gmail.com>:
> * PING *
>
> 2016-06-21 10:59 GMT-06:00 Alessandro Fanfarillo <fanfarillo@gmail.com>:
>> * PING *
>>
>> 2016-06-06 15:05 GMT-06:00 Alessandro Fanfarillo <fanfarillo@gmail.com>:
>>> Dear all,
>>>
>>> please find in attachment the first patch (of n) for the FAILED IMAGES
>>> capability defined in the coarray TS 18508.
>>> The patch adds support for three new intrinsic functions defined in
>>> the TS for simulating a failure (fail image), checking an image status
>>> (image_status) and getting the list of failed images (failed_images).
>>> The patch has been built and regtested on x86_64-pc-linux-gnu.
>>>
>>> Ok for trunk?
>>>
>>> Alessandro

Re: [Fortran] Help with STAT= attribute in coarray reference

[Alessandro Fanfarillo]

Thanks, committed as rev. 238007.

2016-07-04 14:41 GMT-06:00 Mikael Morin <morin-mik...@orange.fr>:
> Le 30/06/2016 06:05, Alessandro Fanfarillo a écrit :
>>
>> Dear Mikael,
>>
>> thanks for your review and for the test. The attached patch, built and
>> regtested for x86_64-pc-linux-gnu, addresses all the suggestions.
>>
>> The next patch will change the documentation related to the caf_get
>> and caf_send functions and will add support for STAT= to the sendget
>> function.
>>
>> In the meantime, is this patch OK for trunk?
>>
> Yes, thanks.
>
> Mikael
>
>

Re: [Fortran, Patch] First patch for coarray FAILED IMAGES (TS 18508)

[Alessandro Fanfarillo]

* PING *

2016-06-21 10:59 GMT-06:00 Alessandro Fanfarillo <fanfarillo@gmail.com>:
> * PING *
>
> 2016-06-06 15:05 GMT-06:00 Alessandro Fanfarillo <fanfarillo@gmail.com>:
>> Dear all,
>>
>> please find in attachment the first patch (of n) for the FAILED IMAGES
>> capability defined in the coarray TS 18508.
>> The patch adds support for three new intrinsic functions defined in
>> the TS for simulating a failure (fail image), checking an image status
>> (image_status) and getting the list of failed images (failed_images).
>> The patch has been built and regtested on x86_64-pc-linux-gnu.
>>
>> Ok for trunk?
>>
>> Alessandro

Re: [Fortran] Help with STAT= attribute in coarray reference

[Alessandro Fanfarillo]

Dear Mikael,

thanks for your review and for the test. The attached patch, built and
regtested for x86_64-pc-linux-gnu, addresses all the suggestions.

The next patch will change the documentation related to the caf_get
and caf_send functions and will add support for STAT= to the sendget
function.

In the meantime, is this patch OK for trunk?


2016-06-23 14:45 GMT-06:00 Mikael <morin-mik...@orange.fr>:
> Le 20/06/2016 22:01, Alessandro Fanfarillo a écrit :
>>
>> Hi Mikael and all,
>>
>> in attachment the new version of the patch.
>> I've addressed all the suggestions except for the stat_se's pre block
>> to se's pre block (commented in the patch for caf_get).
>> Could you please provide a simple example of a complex case? I've
>> already made several test cases and I should be able to produce a
>> complete patch in a couple of days.
>> Thanks,
>>
> Hello,
>
> Second version of comments below.
>
>> diff --git a/gcc/fortran/array.c b/gcc/fortran/array.c
>> index 1430e80..723cc4a 100644
>> --- a/gcc/fortran/array.c
>> +++ b/gcc/fortran/array.c
>> @@ -156,6 +156,7 @@ gfc_match_array_ref (gfc_array_ref *ar, gfc_array_spec
>> *as, int init,
>>  {
>>match m;
>>bool matched_bracket = false;
>> +  gfc_expr *tmp;
>>
>>memset (ar, '\0', sizeof (*ar));
>>
>> @@ -226,6 +227,11 @@ coarray:
>>if (m == MATCH_ERROR)
>> return MATCH_ERROR;
>>
>> +  if(gfc_match(",stat = %e",) == MATCH_YES)
>
> Still some mishandled cases, for example:
>
> tmp = me[i ,  stat=stat]
>
>
>> +   ar->stat = tmp;
>> +  else
>> +   ar->stat = NULL;
>> +
>>if (gfc_match_char (']') == MATCH_YES)
>> {
>>   ar->codimen++;
>> @@ -237,6 +243,14 @@ coarray:
>> }
>>   if (ar->codimen > corank)
>> {
>> + /* Entering in this branch means that something bad
>> happened, except
>> +  * when stat has been detected. If this is the case, we need
>> to
>> +  * decrement the codimension by one. */
>
> OK, I said I didn't understand the code, but that was meaning I didn't
> understand why it is not a problem when stat is there, and why we need to
> decrement by one. I could figure out the rest myself.
> One example I have in mind is this (currently accepted):
>
>   integer :: ca[*]
>   tmp = ca[1,2,stat=foo]
>
> There is also this case (accepted, is it correct?):
>
>   integer :: ca[5, *]
>   tmp = ca[1,stat=foo,2]
>
>> + if(ar->stat)
>> +   {
>> + ar->codimen--;
>> + return MATCH_YES;
>> +   }
>>   gfc_error ("Too many codimensions at %C, expected %d not
>> %d",
>>  corank, ar->codimen);
>>   return MATCH_ERROR;
>
>
>
>> diff --git a/gcc/fortran/trans-decl.c b/gcc/fortran/trans-decl.c
>> index 04339a6..bfffba6 100644
>> --- a/gcc/fortran/trans-decl.c
>> +++ b/gcc/fortran/trans-decl.c
>> @@ -3529,16 +3529,16 @@ gfc_build_builtin_function_decls (void)
>>  ppvoid_type_node, pint_type, pchar_type_node, integer_type_node);
>>
>>gfor_fndecl_caf_get = gfc_build_library_function_decl_with_spec (
>> -   get_identifier (PREFIX("caf_get")), ".R.RRRW", void_type_node, 9,
>> +   get_identifier (PREFIX("caf_get")), ".R.RRRW.", void_type_node,
>> 10,
>
> Unless you plan to do strange things in the implementation of get, you can
> probably use W as spec character for stat.
>
>>  pvoid_type_node, size_type_node, integer_type_node,
>> pvoid_type_node,
>> pvoid_type_node, pvoid_type_node, integer_type_node,
>> integer_type_node,
>> -   boolean_type_node);
>> +   boolean_type_node, pint_type);
>>
>>gfor_fndecl_caf_send = gfc_build_library_function_decl_with_spec (
>> -   get_identifier (PREFIX("caf_send")), ".R.", void_type_node, 9,
>> +   get_identifier (PREFIX("caf_send")), ".R..", void_type_node,
>> 10,
>
> same here.
>
>>  pvoid_type_node, size_type_node, integer_type_node,
>> pvoid_type_node,
>> pvoid_type_node, pvoid_type_node, integer_type_node,
>> integer_type_node,
>> -   boolean_type_node);
>> +   boolean_type_node, pint_type);
>>
>>gfor_fndecl_caf_sendget = gfc_build_library_function

Re: [Fortran, Patch] First patch for coarray FAILED IMAGES (TS 18508)

[Alessandro Fanfarillo]

* PING *

2016-06-06 15:05 GMT-06:00 Alessandro Fanfarillo <fanfarillo@gmail.com>:
> Dear all,
>
> please find in attachment the first patch (of n) for the FAILED IMAGES
> capability defined in the coarray TS 18508.
> The patch adds support for three new intrinsic functions defined in
> the TS for simulating a failure (fail image), checking an image status
> (image_status) and getting the list of failed images (failed_images).
> The patch has been built and regtested on x86_64-pc-linux-gnu.
>
> Ok for trunk?
>
> Alessandro

[Fortran, Patch] First patch for coarray FAILED IMAGES (TS 18508)

[Alessandro Fanfarillo]

Dear all,

please find in attachment the first patch (of n) for the FAILED IMAGES
capability defined in the coarray TS 18508.
The patch adds support for three new intrinsic functions defined in
the TS for simulating a failure (fail image), checking an image status
(image_status) and getting the list of failed images (failed_images).
The patch has been built and regtested on x86_64-pc-linux-gnu.

Ok for trunk?

Alessandro
commit b3bca5b09f4cbcf18f2409dae2485a16a7c06498
Author: Alessandro Fanfarillo <fanfari...@ing.uniroma2.it>
Date:   Mon Jun 6 14:27:37 2016 -0600

First patch Failed Images CAF TS-18508

diff --git a/gcc/fortran/check.c b/gcc/fortran/check.c
index d26e45e..71931cb 100644
--- a/gcc/fortran/check.c
+++ b/gcc/fortran/check.c
@@ -1210,6 +1210,62 @@ gfc_check_event_query (gfc_expr *event, gfc_expr *count, 
gfc_expr *stat)
   return true;
 }
 
+bool
+gfc_check_image_status (gfc_expr *image, gfc_expr *team)
+{
+  if (!type_check (image, 1, BT_INTEGER))
+return false;
+
+  if(team)
+{
+  gfc_error ("TEAM argument of the IMAGE_STATUS intrinsic function at %L "
+"not yet supported",
+>where);
+  return false;
+}
+
+  int i = gfc_validate_kind (BT_INTEGER, image->ts.kind, false);
+  int j = gfc_validate_kind (BT_INTEGER, gfc_default_integer_kind, false);
+
+  if (gfc_integer_kinds[i].range < gfc_integer_kinds[j].range)
+{
+  gfc_error ("IMAGE argument of the IMAGE_STATUS intrinsic function at %L "
+"shall have at least the range of the default integer",
+>where);
+  return false;
+}
+
+  return true;
+}
+
+bool
+gfc_check_failed_images (gfc_expr *team, gfc_expr *kind)
+{
+  if (team)
+{
+  gfc_error ("TEAM argument of the FAILED_IMAGES intrinsic function at %L "
+"not yet supported",
+>where);
+  return false;
+}
+
+  if (kind)
+{
+  int i = gfc_validate_kind (BT_INTEGER, kind->ts.kind, false);
+  int j = gfc_validate_kind (BT_INTEGER, gfc_default_integer_kind, false);
+
+  if (gfc_integer_kinds[i].range < gfc_integer_kinds[j].range)
+   {
+ gfc_error ("KIND argument of the FAILED_IMAGES intrinsic function at 
%L "
+"shall have at least the range of the default integer",
+>where);
+ return false;
+   }
+}
+
+  return true;
+}
+
 
 bool
 gfc_check_atomic_fetch_op (gfc_expr *atom, gfc_expr *value, gfc_expr *old,
diff --git a/gcc/fortran/dump-parse-tree.c b/gcc/fortran/dump-parse-tree.c
index f507434..41ed664 100644
--- a/gcc/fortran/dump-parse-tree.c
+++ b/gcc/fortran/dump-parse-tree.c
@@ -1628,6 +1628,9 @@ show_code_node (int level, gfc_code *c)
 
   break;
 
+case EXEC_FAIL_IMAGE:
+  fputs ("FAIL IMAGE ", dumpfile);
+
 case EXEC_SYNC_ALL:
   fputs ("SYNC ALL ", dumpfile);
   if (c->expr2 != NULL)
diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
index 0bb71cb..6d87632 100644
--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
@@ -253,7 +253,7 @@ enum gfc_statement
   ST_OMP_END_TARGET_TEAMS_DISTRIBUTE_PARALLEL_DO_SIMD,
   ST_PROCEDURE, ST_GENERIC, ST_CRITICAL, ST_END_CRITICAL,
   ST_GET_FCN_CHARACTERISTICS, ST_LOCK, ST_UNLOCK, ST_EVENT_POST,
-  ST_EVENT_WAIT,ST_NONE
+  ST_EVENT_WAIT,ST_FAIL_IMAGE,ST_NONE
 };
 
 /* Types of interfaces that we can have.  Assignment interfaces are
@@ -411,6 +411,7 @@ enum gfc_isym_id
   GFC_ISYM_EXP,
   GFC_ISYM_EXPONENT,
   GFC_ISYM_EXTENDS_TYPE_OF,
+  GFC_ISYM_FAILED_IMAGES,
   GFC_ISYM_FDATE,
   GFC_ISYM_FE_RUNTIME_ERROR,
   GFC_ISYM_FGET,
@@ -454,6 +455,7 @@ enum gfc_isym_id
   GFC_ISYM_IEOR,
   GFC_ISYM_IERRNO,
   GFC_ISYM_IMAGE_INDEX,
+  GFC_ISYM_IMAGE_STATUS,
   GFC_ISYM_INDEX,
   GFC_ISYM_INT,
   GFC_ISYM_INT2,
@@ -2382,7 +2384,7 @@ enum gfc_exec_op
   EXEC_OPEN, EXEC_CLOSE, EXEC_WAIT,
   EXEC_READ, EXEC_WRITE, EXEC_IOLENGTH, EXEC_TRANSFER, EXEC_DT_END,
   EXEC_BACKSPACE, EXEC_ENDFILE, EXEC_INQUIRE, EXEC_REWIND, EXEC_FLUSH,
-  EXEC_LOCK, EXEC_UNLOCK, EXEC_EVENT_POST, EXEC_EVENT_WAIT,
+  EXEC_LOCK, EXEC_UNLOCK, EXEC_EVENT_POST, EXEC_EVENT_WAIT, EXEC_FAIL_IMAGE,
   EXEC_OACC_KERNELS_LOOP, EXEC_OACC_PARALLEL_LOOP, EXEC_OACC_ROUTINE,
   EXEC_OACC_PARALLEL, EXEC_OACC_KERNELS, EXEC_OACC_DATA, EXEC_OACC_HOST_DATA,
   EXEC_OACC_LOOP, EXEC_OACC_UPDATE, EXEC_OACC_WAIT, EXEC_OACC_CACHE,
diff --git a/gcc/fortran/intrinsic.c b/gcc/fortran/intrinsic.c
index 1d7503d..8dfb568 100644
--- a/gcc/fortran/intrinsic.c
+++ b/gcc/fortran/intrinsic.c
@@ -1823,6 +1823,10 @@ add_functions (void)
 a, BT_UNKNOWN, 0, REQUIRED,
 mo, BT_UNKNOWN, 0, REQUIRED);
 
+  add_sym_2 ("failed_images", GFC_ISYM_FAILED_IMAGES, CLASS_TRANSFORMATIONAL, 
ACTUAL_NO, BT_INTEGER,
+dd, GFC_STD_F2008_TS, gfc_check_failed_images, NULL,
+gfc_reso

Re: [Patch, Fortran] STOP issue with coarrays

[Alessandro Fanfarillo]

Thanks for the quick feedback.

The patch has been committed on trunk as rev. 234502 and on
gcc-5-branch as rev. 234503 .

In attachment three simple test cases; coarray_stop_str.f90 fails
because of a syntax error in the check string.
I guess the problem is due to the presence of the "STOPPED" string
inside the check string; how can I manage this situation?

Thanks,

Alessandro

PS: Paul, I see this as a good exercise before working on Failed Images :-).


2016-03-27 19:14 GMT+02:00 Paul Richard Thomas <paul.richard.tho...@gmail.com>:
> Hi Alessandro,
>
> The patch is fine for trunk and 5-branch - going on trivial, I would
> say! Are you going to add the testcase?
>
> Thanks a lot! I am impressed that you are doing these between
> celebrating your doctorate and preparing for your move :-)
>
> Paul
>
> On 27 March 2016 at 17:10, Alessandro Fanfarillo
> <fanfarillo@gmail.com> wrote:
>> Dear all,
>>
>> the attached patch fixes the issue reported by Anton Shterenlikht
>> (https://gcc.gnu.org/ml/fortran/2016-03/msg00037.html). The compiler
>> delegates the external library to manage the STOP statement in case
>> -fcoarray=lib is used.
>>
>> Built and regtested on x86_64-pc-linux-gnu.
>>
>> Ok for trunk and gcc-5-branch?
>
>
>
> --
> The difference between genius and stupidity is; genius has its limits.
>
> Albert Einstein
! { dg-do compile }
! { dg-options "-fdump-tree-original -fcoarray=lib" }
!
! STOP managed by the external library
!
implicit none
  STOP
end

! { dg-final { scan-tree-dump-times "_gfortran_caf_stop_str \\(0B, 0\\);" 1 "original" } }
! { dg-do compile }
! { dg-options "-fdump-tree-original -fcoarray=lib" }
!
! STOP managed by the external library
!
implicit none
  STOP 100
end

! { dg-final { scan-tree-dump-times "_gfortran_caf_stop_numeric \\(100\\);" 1 "original" } }
! { dg-do compile }
! { dg-options "-fdump-tree-original -fcoarray=lib" }
!
! STOP managed by the external library
!
implicit none
  STOP "STOPPED"
end

! { dg-final { scan-tree-dump-times "_gfortran_caf_stop_str \\(&\\"STOPPED\\"[1]\\{lb: 1 sz: 1\\}, 7\\);" 1 "original" } }

[Patch, Fortran] STOP issue with coarrays

[Alessandro Fanfarillo]

Dear all,

the attached patch fixes the issue reported by Anton Shterenlikht
(https://gcc.gnu.org/ml/fortran/2016-03/msg00037.html). The compiler
delegates the external library to manage the STOP statement in case
-fcoarray=lib is used.

Built and regtested on x86_64-pc-linux-gnu.

Ok for trunk and gcc-5-branch?
gcc/fortran/ChangeLog
2016-03-27  Alessandro Fanfarillo  <fanfarillo@gmail.com>

* trans-decl.c (gfc_build_builtin_function_decls):
caf_stop_numeric and caf_stop_str definition.
* trans-stmt.c (gfc_trans_stop): invoke external functions
for stop and stop_str when coarrays are used.
* trans.h: extern for new functions.

libgfortran/ChangeLog
2016-03-27  Alessandro Fanfarillo  <fanfarillo@gmail.com>

* caf/libcaf.h: caf_stop_numeric and caf_stop_str prototype.
* caf/single.c: _gfortran_caf_stop_numeric and
_gfortran_caf_stop_str implementation.

commit bb407679e918dfb9cbc769594cf39a6bd09dd9d9
Author: Alessandro Fanfarillo <fanfari...@ing.uniroma2.it>
Date:   Sun Mar 27 16:42:59 2016 +0200

Adding caf_stop_str and caf_stop_numeric

diff --git a/gcc/fortran/trans-decl.c b/gcc/fortran/trans-decl.c
index 4bd7dc4..309baf1 100644
--- a/gcc/fortran/trans-decl.c
+++ b/gcc/fortran/trans-decl.c
@@ -137,6 +137,8 @@ tree gfor_fndecl_caf_sendget;
 tree gfor_fndecl_caf_sync_all;
 tree gfor_fndecl_caf_sync_memory;
 tree gfor_fndecl_caf_sync_images;
+tree gfor_fndecl_caf_stop_str;
+tree gfor_fndecl_caf_stop_numeric;
 tree gfor_fndecl_caf_error_stop;
 tree gfor_fndecl_caf_error_stop_str;
 tree gfor_fndecl_caf_atomic_def;
@@ -3550,6 +3552,18 @@ gfc_build_builtin_function_decls (void)
   /* CAF's ERROR STOP doesn't return.  */
   TREE_THIS_VOLATILE (gfor_fndecl_caf_error_stop_str) = 1;
 
+  gfor_fndecl_caf_stop_numeric = gfc_build_library_function_decl_with_spec 
(
+get_identifier (PREFIX("caf_stop_numeric")), ".R.",
+void_type_node, 1, gfc_int4_type_node);
+  /* CAF's STOP doesn't return.  */
+  TREE_THIS_VOLATILE (gfor_fndecl_caf_stop_numeric) = 1;
+
+  gfor_fndecl_caf_stop_str = gfc_build_library_function_decl_with_spec (
+get_identifier (PREFIX("caf_stop_str")), ".R.",
+void_type_node, 2, pchar_type_node, gfc_int4_type_node);
+  /* CAF's STOP doesn't return.  */
+  TREE_THIS_VOLATILE (gfor_fndecl_caf_stop_str) = 1;
+
   gfor_fndecl_caf_atomic_def = gfc_build_library_function_decl_with_spec (
get_identifier (PREFIX("caf_atomic_define")), "R..RW",
void_type_node, 7, pvoid_type_node, size_type_node, integer_type_node,
diff --git a/gcc/fortran/trans-stmt.c b/gcc/fortran/trans-stmt.c
index cb54499..2fc43ed 100644
--- a/gcc/fortran/trans-stmt.c
+++ b/gcc/fortran/trans-stmt.c
@@ -635,7 +635,9 @@ gfc_trans_stop (gfc_code *code, bool error_stop)
 ? (flag_coarray == GFC_FCOARRAY_LIB
? gfor_fndecl_caf_error_stop_str
: gfor_fndecl_error_stop_string)
-: gfor_fndecl_stop_string,
+: (flag_coarray == GFC_FCOARRAY_LIB
+   ? gfor_fndecl_caf_stop_str
+   : gfor_fndecl_stop_string),
 2, build_int_cst (pchar_type_node, 0), tmp);
 }
   else if (code->expr1->ts.type == BT_INTEGER)
@@ -646,7 +648,9 @@ gfc_trans_stop (gfc_code *code, bool error_stop)
 ? (flag_coarray == GFC_FCOARRAY_LIB
? gfor_fndecl_caf_error_stop
: gfor_fndecl_error_stop_numeric)
-: gfor_fndecl_stop_numeric_f08, 1,
+: (flag_coarray == GFC_FCOARRAY_LIB
+   ? gfor_fndecl_caf_stop_numeric
+   : gfor_fndecl_stop_numeric_f08), 1,
 fold_convert (gfc_int4_type_node, se.expr));
 }
   else
@@ -657,7 +661,9 @@ gfc_trans_stop (gfc_code *code, bool error_stop)
 ? (flag_coarray == GFC_FCOARRAY_LIB
? gfor_fndecl_caf_error_stop_str
: gfor_fndecl_error_stop_string)
-: gfor_fndecl_stop_string,
+: (flag_coarray == GFC_FCOARRAY_LIB
+   ? gfor_fndecl_caf_stop_str
+   : gfor_fndecl_stop_string),
 2, se.expr, se.string_length);
 }
 
diff --git a/gcc/fortran/trans.h b/gcc/fortran/trans.h
index 316ee9b..add0cea 100644
--- a/gcc/fortran/trans.h
+++ b/gcc/fortran/trans.h
@@ -762,6 +762,8 @@ extern GTY(()) tree gfor_fndecl_caf_sendget;
 extern GTY(()) tree gfor_fndecl_

Re: [Fortran, Patch] (Coarrays) Wrong events size

[Alessandro Fanfarillo]

Thanks!

Patch committed as revision 233779 on trunk and as revision 233780 on
gcc-5-branch.

2016-02-26 19:14 GMT+01:00 Paul Richard Thomas :
> Dear Alessandro,
>
> Seconded! I saw your ping on my phone and was going to respond.
> well, now :-)
>
> Thanks for the patch
>
> Paul
>
> On 26 February 2016 at 18:29, Thomas Koenig  wrote:
>> Hi Allessandro,
>>
>>> * PING *
>>
>>
>> Looks obvious and simple enough for me.
>>
>> OK.
>>
>> Thanks for the patch!
>>
>> Thomas
>>
>
>
>
> --
> The difference between genius and stupidity is; genius has its limits.
>
> Albert Einstein

Re: [Fortran, Patch] (Coarrays) Wrong events size

[Alessandro Fanfarillo]

* PING *

2016-02-20 18:25 GMT+01:00 Alessandro Fanfarillo <fanfarillo@gmail.com>:
> Dear all,
>
> currently, the compiler doesn't pass the right size to the
> registration routine of OpenCoarrays for event variables:
>
> size.15 = 0;
>
> ev.data = (void * restrict) _gfortran_caf_register (MAX_EXPR <size.15,
> 1>, 6, , 0B, 0B, 0);
>
> The attached patch solves the problem.
>
> I don't understand the following block in trans-types.c:
>
>   if (flag_coarray != GFC_FCOARRAY_LIB
>   && derived->from_intmod == INTMOD_ISO_FORTRAN_ENV
>   && derived->intmod_sym_id == ISOFORTRAN_EVENT_TYPE)
> return gfc_get_int_type (gfc_default_integer_kind);
>
> Why should an event variable be different from a lock variable when
> LIBCAF_SINGLE is used?
>
> The patch has been built and regtested on x86_64-pc-linux-gnu.
>
> Ok for trunk and gcc-5-branch?

[Fortran, Patch] (Coarrays) Wrong events size

[Alessandro Fanfarillo]

Dear all,

currently, the compiler doesn't pass the right size to the
registration routine of OpenCoarrays for event variables:

size.15 = 0;

ev.data = (void * restrict) _gfortran_caf_register (MAX_EXPR <size.15,
1>, 6, , 0B, 0B, 0);

The attached patch solves the problem.

I don't understand the following block in trans-types.c:

  if (flag_coarray != GFC_FCOARRAY_LIB
  && derived->from_intmod == INTMOD_ISO_FORTRAN_ENV
  && derived->intmod_sym_id == ISOFORTRAN_EVENT_TYPE)
return gfc_get_int_type (gfc_default_integer_kind);

Why should an event variable be different from a lock variable when
LIBCAF_SINGLE is used?

The patch has been built and regtested on x86_64-pc-linux-gnu.

Ok for trunk and gcc-5-branch?
gcc/fortran/Changelog

2016-02-20  Alessandro Fanfarillo  <fanfarillo@gmail.com>

* trans.c (gfc_allocate_allocatable): size conversion
from byte to number of elements for event variables.
* trans-types.c (gfc_get_derived_type): event variables
represented as a pointer (like lock variable).

commit 095c091a8a3ff10fc703e75585b9b57340723648
Author: Alessandro <fanfari...@ing.uniroma2.it>
Date:   Sat Feb 20 18:11:22 2016 +0100

Right size for events

diff --git a/gcc/fortran/trans-types.c b/gcc/fortran/trans-types.c
index f3d0841..a71cf0b 100644
--- a/gcc/fortran/trans-types.c
+++ b/gcc/fortran/trans-types.c
@@ -2370,7 +2370,8 @@ gfc_get_derived_type (gfc_symbol * derived)
   if (derived->attr.unlimited_polymorphic
   || (flag_coarray == GFC_FCOARRAY_LIB
  && derived->from_intmod == INTMOD_ISO_FORTRAN_ENV
- && derived->intmod_sym_id == ISOFORTRAN_LOCK_TYPE))
+ && (derived->intmod_sym_id == ISOFORTRAN_LOCK_TYPE
+ || derived->intmod_sym_id == ISOFORTRAN_EVENT_TYPE)))
 return ptr_type_node;
 
   if (flag_coarray != GFC_FCOARRAY_LIB
diff --git a/gcc/fortran/trans.c b/gcc/fortran/trans.c
index e71430b..c6688d3 100644
--- a/gcc/fortran/trans.c
+++ b/gcc/fortran/trans.c
@@ -820,7 +820,7 @@ gfc_allocate_allocatable (stmtblock_t * block, tree mem, 
tree size, tree token,
 the FE only passes the pointer around and leaves the actual
 representation to the library. Hence, we have to convert back to the
 number of elements.  */
-  if (lock_var)
+  if (lock_var || event_var)
size = fold_build2_loc (input_location, TRUNC_DIV_EXPR, size_type_node,
size, TYPE_SIZE_UNIT (ptr_type_node));
 

Re: [Fortran, Patch] (RFC, Coarray) Implement TS18508's EVENTS

[Alessandro Fanfarillo]

Committed on gcc-5-branch as rev. 233379.

2016-02-12 0:04 GMT+01:00 Alessandro Fanfarillo <fanfarillo@gmail.com>:
> Dear all,
>
> in attachment the EVENT patch for gcc-5-branch directly back-ported
> from the trunk.
>
> Built and regtested on x86_64-pc-linux-gnu. I plan to commit this
> patch this evening (Feb 12th, CET).
>
> Cheers,
>
> Alessandro
>
> 2015-12-17 17:19 GMT+01:00 Alessandro Fanfarillo <fanfarillo@gmail.com>:
>> Great! Thanks.
>>
>> 2015-12-17 15:57 GMT+01:00 Steve Kargl <s...@troutmask.apl.washington.edu>:
>>> On Thu, Dec 17, 2015 at 01:22:06PM +0100, Alessandro Fanfarillo wrote:
>>>>
>>>> I've noticed that this patch has been applied only on trunk and not on
>>>> the gcc-5-branch. Is it a problem to include EVENTS in gcc-5?
>>>>
>>>
>>> No problem.  When I applied the EVENTS patch to trunk,
>>> the 5.3 release was being prepared.  I was going to
>>> wait for a week or two after 5.3 came out, then apply
>>> the patch.  Now that you have commit access, feel
>>> free to back port the patch.  Rememer to post the
>>> patch that you commit to both the fortran and gcc-patches
>>> list.
>>>
>>> --
>>> Steve

Re: [Fortran, Patch] (RFC, Coarray) Implement TS18508's EVENTS

[Alessandro Fanfarillo]

Dear all,

in attachment the EVENT patch for gcc-5-branch directly back-ported
from the trunk.

Built and regtested on x86_64-pc-linux-gnu. I plan to commit this
patch this evening (Feb 12th, CET).

Cheers,

Alessandro

2015-12-17 17:19 GMT+01:00 Alessandro Fanfarillo <fanfarillo@gmail.com>:
> Great! Thanks.
>
> 2015-12-17 15:57 GMT+01:00 Steve Kargl <s...@troutmask.apl.washington.edu>:
>> On Thu, Dec 17, 2015 at 01:22:06PM +0100, Alessandro Fanfarillo wrote:
>>>
>>> I've noticed that this patch has been applied only on trunk and not on
>>> the gcc-5-branch. Is it a problem to include EVENTS in gcc-5?
>>>
>>
>> No problem.  When I applied the EVENTS patch to trunk,
>> the 5.3 release was being prepared.  I was going to
>> wait for a week or two after 5.3 came out, then apply
>> the patch.  Now that you have commit access, feel
>> free to back port the patch.  Rememer to post the
>> patch that you commit to both the fortran and gcc-patches
>> list.
>>
>> --
>> Steve
commit 9504c4c79accddeb6e2386c9bf60d651c3d8f627
Author: Alessandro Fanfarillo <fanfarillo@gmail.com>
Date:   Thu Feb 11 11:24:53 2016 +

Events patch backported from gcc-trunk

diff --git ./gcc/fortran/check.c ./gcc/fortran/check.c
index 3196420..049a6fb 100644
--- ./gcc/fortran/check.c
+++ ./gcc/fortran/check.c
@@ -1157,6 +1157,59 @@ gfc_check_atomic_cas (gfc_expr *atom, gfc_expr *old, 
gfc_expr *compare,
   return true;
 }
 
+bool
+gfc_check_event_query (gfc_expr *event, gfc_expr *count, gfc_expr *stat)
+{
+  if (event->ts.type != BT_DERIVED
+  || event->ts.u.derived->from_intmod != INTMOD_ISO_FORTRAN_ENV
+  || event->ts.u.derived->intmod_sym_id != ISOFORTRAN_EVENT_TYPE)
+{
+  gfc_error ("EVENT argument at %L to the intrinsic EVENT_QUERY "
+"shall be of type EVENT_TYPE", >where);
+  return false;
+}
+
+  if (!scalar_check (event, 0))
+return false;
+
+  if (!gfc_check_vardef_context (count, false, false, false, NULL))
+{
+  gfc_error ("COUNT argument of the EVENT_QUERY intrinsic function at %L "
+"shall be definable", >where);
+  return false;
+}
+
+  if (!type_check (count, 1, BT_INTEGER))
+return false;
+
+  int i = gfc_validate_kind (BT_INTEGER, count->ts.kind, false);
+  int j = gfc_validate_kind (BT_INTEGER, gfc_default_integer_kind, false);
+
+  if (gfc_integer_kinds[i].range < gfc_integer_kinds[j].range)
+{
+  gfc_error ("COUNT argument of the EVENT_QUERY intrinsic function at %L "
+"shall have at least the range of the default integer",
+>where);
+  return false;
+}
+
+  if (stat != NULL)
+{
+  if (!type_check (stat, 2, BT_INTEGER))
+   return false;
+  if (!scalar_check (stat, 2))
+   return false;
+  if (!variable_check (stat, 2, false))
+   return false;
+
+  if (!gfc_notify_std (GFC_STD_F2008_TS, "STAT= argument to %s at %L",
+  gfc_current_intrinsic, >where))
+   return false;
+}
+
+  return true;
+}
+
 
 bool
 gfc_check_atomic_fetch_op (gfc_expr *atom, gfc_expr *value, gfc_expr *old,
diff --git ./gcc/fortran/dump-parse-tree.c ./gcc/fortran/dump-parse-tree.c
index 83ecbaa..c886010 100644
--- ./gcc/fortran/dump-parse-tree.c
+++ ./gcc/fortran/dump-parse-tree.c
@@ -1659,6 +1659,33 @@ show_code_node (int level, gfc_code *c)
}
   break;
 
+case EXEC_EVENT_POST:
+case EXEC_EVENT_WAIT:
+  if (c->op == EXEC_EVENT_POST)
+   fputs ("EVENT POST ", dumpfile);
+  else
+   fputs ("EVENT WAIT ", dumpfile);
+
+  fputs ("event-variable=", dumpfile);
+  if (c->expr1 != NULL)
+   show_expr (c->expr1);
+  if (c->expr4 != NULL)
+   {
+ fputs (" until_count=", dumpfile);
+ show_expr (c->expr4);
+   }
+  if (c->expr2 != NULL)
+   {
+ fputs (" stat=", dumpfile);
+ show_expr (c->expr2);
+   }
+  if (c->expr3 != NULL)
+   {
+ fputs (" errmsg=", dumpfile);
+ show_expr (c->expr3);
+   }
+  break;
+
 case EXEC_LOCK:
 case EXEC_UNLOCK:
   if (c->op == EXEC_LOCK)
diff --git ./gcc/fortran/expr.c ./gcc/fortran/expr.c
index c90e823..2e74375 100644
--- ./gcc/fortran/expr.c
+++ ./gcc/fortran/expr.c
@@ -4864,6 +4864,19 @@ gfc_check_vardef_context (gfc_expr* e, bool pointer, 
bool alloc_obj,
   return false;
 }
 
+  /* TS18508, C702/C203.  */
+  if (!alloc_obj
+  && (attr.lock_comp
+ || (e->ts.type == BT_DERIVED
+ && e->ts.u.derived->from_intmod == INTMOD_ISO_FORTRAN_ENV
+ && e->ts.u.derived->intmod_sym_id == ISOF

Re: [Fortran, Patch] (RFC, Coarray) Implement TS18508's EVENTS

[Alessandro Fanfarillo]

Great! Thanks.

2015-12-17 15:57 GMT+01:00 Steve Kargl <s...@troutmask.apl.washington.edu>:
> On Thu, Dec 17, 2015 at 01:22:06PM +0100, Alessandro Fanfarillo wrote:
>>
>> I've noticed that this patch has been applied only on trunk and not on
>> the gcc-5-branch. Is it a problem to include EVENTS in gcc-5?
>>
>
> No problem.  When I applied the EVENTS patch to trunk,
> the 5.3 release was being prepared.  I was going to
> wait for a week or two after 5.3 came out, then apply
> the patch.  Now that you have commit access, feel
> free to back port the patch.  Rememer to post the
> patch that you commit to both the fortran and gcc-patches
> list.
>
> --
> Steve

Re: [Fortran, Patch] (RFC, Coarray) Implement TS18508's EVENTS

[Alessandro Fanfarillo]

Hi,

I've noticed that this patch has been applied only on trunk and not on
the gcc-5-branch. Is it a problem to include EVENTS in gcc-5?

2015-12-02 23:00 GMT+01:00 Steve Kargl <s...@troutmask.apl.washington.edu>:
> Committed as revision 231208.
>
> Alessandro, Tobias, is this a candidate for a commit to
> the 5-branch when it is re-opened?
>
> --
> steve
>
> On Wed, Dec 02, 2015 at 03:16:05PM +0100, Alessandro Fanfarillo wrote:
>> *PING*
>>
>> 2015-11-26 17:51 GMT+01:00 Steve Kargl <s...@troutmask.apl.washington.edu>:
>> > On Wed, Nov 25, 2015 at 06:24:49PM +0100, Alessandro Fanfarillo wrote:
>> >> Dear all,
>> >>
>> >> in attachment the previous patch compatible with the current trunk.
>> >> The patch also includes the changes introduced in the latest TS 18508.
>> >>
>> >> Built and regtested on x86_64-pc-linux-gnu.
>> >>
>> >> PS: I will add the test cases in a different patch.
>> >>
>> >
>> > I have now built and regression tested the patch on
>> > x86_64-*-freebsd and i386-*-freebsd.  There were no
>> > regressions.  In reading through the patch, nothing
>> > jumped out at me as suspicious/wrong.  Tobias, this
>> > is OK to commit.  If you don't committed by Sunday,
>> > I'll do it for you.
>> >
>> > --
>> > steve
>
> --
> Steve

Re: [COMMITTED] Add myself to MAINTAINERS (Write After Approval)

[Alessandro Fanfarillo]

Sorry about that.

Committed revision 231657

Index: ChangeLog
===
--- ChangeLog(revision 231656)
+++ ChangeLog(working copy)
@@ -1,4 +1,4 @@
-2015-12-15  Alessandro Fanfarillo <fanfarillo@gmail.com>
+2015-12-15  Alessandro Fanfarillo  <fanfarillo@gmail.com>

 * MAINTAINERS (Write After Approval): Add myself.

2015-12-15 14:09 GMT+01:00 Jakub Jelinek <ja...@redhat.com>:
> On Tue, Dec 15, 2015 at 02:07:40PM +0100, Alessandro Fanfarillo wrote:
>> I've added myself to Write After Approval maintainers.
>
>> --- ChangeLog(revision 231646)
>> +++ ChangeLog(working copy)
>> @@ -1,3 +1,7 @@
>> +2015-12-15  Alessandro Fanfarillo <fanfarillo@gmail.com>
>
> Two spaces before < instead of one.
>
> Jakub

[COMMITTED] Add myself to MAINTAINERS (Write After Approval)

[Alessandro Fanfarillo]

Dear all,

I've added myself to Write After Approval maintainers.

Committed revision 231647.

Index: MAINTAINERS
===
--- MAINTAINERS(revision 231646)
+++ MAINTAINERS(working copy)
@@ -388,6 +388,7 @@
 Ansgar Esztermann<ans...@thphy.uni-duesseldorf.de>
 Doug Evans<d...@google.com>
 Chris Fairles<cfair...@gcc.gnu.org>
+Alessandro Fanfarillo<fanfarillo@gmail.com>
 Changpeng Fang<changpeng.f...@amd.com>
 Li Feng<nemoking...@gmail.com>
 Max Filippov<jcmvb...@gmail.com>
Index: ChangeLog
===
--- ChangeLog(revision 231646)
+++ ChangeLog(working copy)
@@ -1,3 +1,7 @@
+2015-12-15  Alessandro Fanfarillo <fanfarillo@gmail.com>
+
+* MAINTAINERS (Write After Approval): Add myself.
+
 2015-12-02  Ian Lance Taylor  <i...@google.com>

 PR go/66147

Re: [Fortran, Patch} Fix ICE for coarray Critical inside module procedure

[Alessandro Fanfarillo]

Committed as revision 231649 on trunk and as revision 231650 on gcc-5-branch.

Thanks.

2015-12-14 20:02 GMT+01:00 Tobias Burnus <bur...@net-b.de>:
> Dear Alessandro,
>
> Alessandro Fanfarillo wrote:
>>
>> the compiler returns an ICE when a coarray critical section is used
>> inside a module procedure.
>> The symbols related with the lock variables were left uncommitted
>> inside resolve_critical(). A gfc_commit_symbol after each symbol or a
>> gfc_commit_symbols at the end of resolve_critical() fixed the issue.
>>
>> The latter solution is proposed in the attached patch.
>> Built and regtested on x86_64-pc-linux-gnu
>
>
> Looks good to me.
>
>> PS: This patch should be also included in GCC 5.
>
>
> Yes, that's fine with me.
>
> Tobias
>
> PS: I saw that you now have a GCC account, which you can use to commit to
> both the trunk and gcc-5-branch. See https://gcc.gnu.org/svnwrite.html.
> Additionally, you should update MAINTAINERS (trunk only) by adding yourself
> under "Write After Approval"; you can simply commit this patch yourself, but
> you should write an email to gcc-patches with the patch - like Alan did at
> https://gcc.gnu.org/ml/gcc-patches/2015-10/msg02390.html

Re: [Fortran, Patch] Memory sync after coarray image control statements and assignment

[Alessandro Fanfarillo]

2015-12-09 23:16 GMT+01:00 Tobias Burnus :
> Thanks. Committed as r231476.

Thanks.

>
> Do we need to do anything about GCC 5 or is this only a GCC 6 issue?
>

Yes, the patch should be applied to GCC 5 too.

> That can be changed: Simply fill out the form and list me (burnus (at]
> gcc.gnu.org) as sponsor: https://sourceware.org/cgi-bin/pdw/ps_form.cgi –
> and see https://gcc.gnu.org/svnwrite.html

Done. Thanks.

>
> Tobias

[Fortran, Patch} Fix ICE for coarray Critical inside module procedure

[Alessandro Fanfarillo]

Dear all,

the compiler returns an ICE when a coarray critical section is used
inside a module procedure.
The symbols related with the lock variables were left uncommitted
inside resolve_critical(). A gfc_commit_symbol after each symbol or a
gfc_commit_symbols at the end of resolve_critical() fixed the issue.

The latter solution is proposed in the attached patch.

Built and regtested on x86_64-pc-linux-gnu

Cheers

Alessandro


PS: This patch should be also included in GCC 5.
2015-12-10  Alessandro Fanfarillo <fanfarillo@gmail.com>

* resolve.c (resolve_critical): Committing symbols of
lock variables.

2015-12-10  Alessandro Fanfarillo <fanfarillo@gmail.com>

* gfortran.dg/coarray_critical_1.f90: New.

commit ccc06accb4891ab95d33135b62c479d895b2270f
Author: Alessandro Fanfarillo <fanfari...@ing.uniroma2.it>
Date:   Thu Dec 10 15:28:33 2015 +

Committing symbols of lock variables inside resolve_critical()

diff --git a/gcc/fortran/resolve.c b/gcc/fortran/resolve.c
index 65a2b7f..3988b3c 100644
--- a/gcc/fortran/resolve.c
+++ b/gcc/fortran/resolve.c
@@ -8852,6 +8852,7 @@ resolve_critical (gfc_code *code)
   symtree->n.sym->as->cotype = AS_EXPLICIT;
   symtree->n.sym->as->lower[0] = gfc_get_int_expr (gfc_default_integer_kind,
   NULL, 1);
+  gfc_commit_symbols();
 }
 
 
diff --git a/gcc/testsuite/gfortran.dg/coarray_critical_1.f90 
b/gcc/testsuite/gfortran.dg/coarray_critical_1.f90
new file mode 100644
index 000..4d93bf6
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/coarray_critical_1.f90
@@ -0,0 +1,12 @@
+! { dg-do compile }
+! { dg-options "-fcoarray=lib" }
+!
+
+module m
+ contains
+   subroutine f()
+ critical
+ end critical
+   end subroutine f
+ end module m
+end program

Re: [Fortran, Patch] Memory sync after coarray image control statements and assignment

[Alessandro Fanfarillo]

Done.

I have permission for contributing but I don't have write permission
on the repository.

2015-12-09 8:23 GMT+01:00 Tobias Burnus <bur...@net-b.de>:
> Alessandro Fanfarillo wrote:
>>
>> in attachment the new patch. I also checked the behavior with
>> move_alloc: it synchronizes right after the deregistration of the
>> destination.
>> I also noticed that __asm__ __volatile__ ("":::"memory") is called
>> before sync all and not after. It shouldn't be a problem, right?
>
>
> The patch looks mostly good to me, except:
>
>> @@ -1222,6 +1230,15 @@ gfc_conv_intrinsic_caf_get (gfc_se *se, gfc_expr
>> *expr, tree lhs, tree lhs_kind,
>> se->expr = res_var;
>> if (array_expr->ts.type == BT_CHARACTER)
>>   se->string_length = argse.string_length;
>> +
>> +  /* It guarantees memory consistency within the same segment */
>> +  /* tmp = gfc_build_string_const (strlen ("memory")+1, "memory"), */
>> +  /* tmp = build5_loc (input_location, ASM_EXPR, void_type_node, */
>> +  /*   gfc_build_string_const (1, ""), NULL_TREE, NULL_TREE,
>> */
>> +  /*   tree_cons (NULL_TREE, tmp, NULL_TREE), NULL_TREE); */
>> +  /* ASM_VOLATILE_P (tmp) = 1; */
>> +  /* gfc_add_expr_to_block (>pre, tmp); */
>> +
>>   }
>
>
> Do not add out-commented code. Please remove.
>
>
>>  gfc_trans_critical (gfc_code *code)
>>  {
>>stmtblock_t block;
>>tree tmp, token = NULL_TREE;
>>
>>gfc_start_block ();
>>
>>if (flag_coarray == GFC_FCOARRAY_LIB)
>>  {
>>token = gfc_get_symbol_decl (code->resolved_sym);
>>token = GFC_TYPE_ARRAY_CAF_TOKEN (TREE_TYPE (token));
>>tmp = build_call_expr_loc (input_location, gfor_fndecl_caf_lock, 7,
>>  token, integer_zero_node,
>> integer_one_node,
>>  null_pointer_node, null_pointer_node,
>>  null_pointer_node, integer_zero_node);
>>gfc_add_expr_to_block (, tmp);
>>  }
>>
>> +  /* It guarantees memory consistency within the same segment */
>> +  tmp = gfc_build_string_const (strlen ("memory")+1, "memory"),
>> +tmp = build5_loc (input_location, ASM_EXPR, void_type_node,
>> + gfc_build_string_const (1, ""), NULL_TREE,
>> NULL_TREE,
>> + tree_cons (NULL_TREE, tmp, NULL_TREE), NULL_TREE);
>> +  ASM_VOLATILE_P (tmp) = 1;
>> +
>> +  gfc_add_expr_to_block (, tmp);
>> +
>>tmp = gfc_trans_code (code->block->next);
>>gfc_add_expr_to_block (, tmp);
>>
>>if (flag_coarray == GFC_FCOARRAY_LIB)
>>  {
>>tmp = build_call_expr_loc (input_location, gfor_fndecl_caf_unlock,
>> 6,
>>  token, integer_zero_node,
>> integer_one_node,
>>  null_pointer_node, null_pointer_node,
>>  integer_zero_node);
>>gfc_add_expr_to_block (, tmp);
>>  }
>>
>> +  /* It guarantees memory consistency within the same segment */
>> +  tmp = gfc_build_string_const (strlen ("memory")+1, "memory"),
>> +  tmp = build5_loc (input_location, ASM_EXPR, void_type_node,
>> +   gfc_build_string_const (1, ""), NULL_TREE,
>> NULL_TREE,
>> +   tree_cons (NULL_TREE, tmp, NULL_TREE), NULL_TREE);
>> +      ASM_VOLATILE_P (tmp) = 1;
>> +
>> +  gfc_add_expr_to_block (, tmp);
>>
>>return gfc_finish_block ();
>>  }
>
>
> Please move the two new code additions into the associated "if (flag_coarray
> == GFC_FCOARRAY_LIB)" blocks - and keep an eye on the indentation: for the
> current code location, the second block is wrongly idented.
>
>
> With the two issues fixed: LGTM.
>
> Cheers,
>
> Tobias
>
> PS: I assume you can still commit yourself. I am asking because Steve did
> commit the recent EVENT patch for you.
2015-12-09  Tobias Burnus  <bur...@net-b.de>
Alessandro Fanfarillo <fanfarillo@gmail.com>

* trans.c (gfc_allocate_using_lib,gfc_deallocate_with_status):
Introducing __asm__ __volatile__ ("":::"memory")
after image control statements.
* trans-stmt.c  (gfc_trans_sync, gfc_trans_event_post_wait,
gfc_trans_lock_unlock, gfc_trans_critical): Ditto.
* trans-intrinsic.c (gfc_conv_intrinsic_caf_get,
   

Re: [Fortran, Patch] Memory sync after coarray image control statements and assignment

[Alessandro Fanfarillo]

Hi,

in attachment the new patch. I also checked the behavior with
move_alloc: it synchronizes right after the deregistration of the
destination.
I also noticed that __asm__ __volatile__ ("":::"memory") is called
before sync all and not after. It shouldn't be a problem, right?


2015-12-08 11:01 GMT+01:00 Tobias Burnus <tobias.bur...@physik.fu-berlin.de>:
> Dear Alessandro, dear all,
>
> On Mon, Dec 07, 2015 at 03:48:17PM +0100, Alessandro Fanfarillo wrote:
>> Your patch fixes the issues. In attachment patch, test case and changelog.
>
> Regarding the ChangeLog: Please include the added lines, only, and not the
> change as patch. gcc/testsuite/ChangeLog changes too often such that a patch
> won't apply.
>
>
> Regarding the patch, I wonder where the memory synchronization makes sense
> and where it is not required. (cf. also my email to Matthew in this thread,
> https://gcc.gnu.org/ml/gcc-patches/2015-12/msg00828.html)
>
>
> I think it should be after all image control statements (8.5.1 in
> http://j3-fortran.org/doc/year/15/15-007r2.pdf):
> SYNC ALL, SYNC IMAGES, SYNC MEMORY, ALLOCATE, DEALLOCATE,
> CRITICAL ... END CRITICAL, EVENT POST, EVENT WAIT, LOCK, UNLOCK,
> MOVE_ALLOC.
>
> Thus:
> - SYNC ..., ALLOCATE/DEALLOCATE: I think those are all handled by the
>   current patch
> - MOVE_ALLOC: This one should be handled via the internal (de)allocate
>   handling (please check!)
> - EVENT WAIT, CRITICAL and LOCK: Obtaining a lock or receiving an event
>   implies that quite likely some other process has changed something
>   before. For those, the assembler statement really has to be added.
> - EVENT POST, UNLOCK and END CRITICAL: While those are image control
>   statements, I do not see how a remote image could modify a value in
>   a well defined way, which is guaranteed to be available after that
>   statement - but might not yet be available already at the previous
>   segment (i.e. the previous image control statement).
>
> Hence: I think you should update the patch to also handle
> EVENT WAIT, CRITICAL and LOCK - and to check MOVE_ALLOC.
>
>
>
> Additionally, we need to handle the alias issue of:
>   var = 5
>   var[this_image()] = 42
>   tmp = var
>
> Both _gfortran_caf_send and _gfortran_caf_sendget can modify the
> value of a variable; thus, calling the assembler after the function
> makes sense.
>
>
> However, _gfortran_caf_get does not modify the associated variable;
> adding the assembler statement *after* _gfortran_caf_get. The
> question is, however, whether one needs to take care of 'flushing'
> the variable before the _gfortran_caf_get:
>var = 7
>...
>var = 5
>tmp = var[this_image()]
>result = var + tmp
> Here, one needs to prevent the compiler of keeping "5" only in a
> register or moving "var = 5" after the _gfortran_caf_get call.
>
>
> Thus, you have to move the assembler statement before the library
> call in _gfortran_caf_get - and add another one at the beginning
> of _gfortran_caf_sendget.
>
> (For send/get, might might come up with something better than
> ""::"memory", but for now, it should do.)
>
> Cheers,
>
> Tobias
2015-12-08  Tobias Burnus  <bur...@net-b.de>
Alessandro Fanfarillo <fanfarillo@gmail.com>

* trans.c (gfc_allocate_using_lib,gfc_deallocate_with_status):
Introducing __asm__ __volatile__ ("":::"memory")
after image control statements.
* trans-stmt.c  (gfc_trans_sync, gfc_trans_event_post_wait,
gfc_trans_lock_unlock, gfc_trans_critical): Ditto.
* trans-intrinsic.c (gfc_conv_intrinsic_caf_get,
conv_caf_send): Introducing __asm__ __volatile__ ("":::"memory")
after send, before get and around sendget.

2015-12-08  Tobias Burnus  <bur...@net-b.de>
Alessandro Fanfarillo <fanfarillo@gmail.com>

* gfortran.dg/coarray_40.f90: New.

commit 6cdffc285931eb604d4c900d77fe60b96d604556
Author: Alessandro Fanfarillo <fanfari...@ing.uniroma2.it>
Date:   Tue Dec 8 14:58:20 2015 +0100

Introducing __asm__ __volatile__ (:::memory) after image control 
statements, send, get and sendget

diff --git a/gcc/fortran/trans-intrinsic.c b/gcc/fortran/trans-intrinsic.c
index 21efe44..0e4fcc5 100644
--- a/gcc/fortran/trans-intrinsic.c
+++ b/gcc/fortran/trans-intrinsic.c
@@ -1211,6 +1211,14 @@ gfc_conv_intrinsic_caf_get (gfc_se *se, gfc_expr *expr, 
tree lhs, tree lhs_kind,
   if (lhs == NULL_TREE)
 may_require_tmp = boolean_false_node;
 
+  /* It guarantees memory consistency within the same segment */
+  tmp = gfc_build_string_const (strlen ("memory")+1, "memory"),
+  tmp = build5_loc (inp

Re: [Fortran, Patch] Memory sync after coarray image control statements and assignment

[Alessandro Fanfarillo]

Your patch fixes the issues. In attachment patch, test case and changelog.

Thanks!

2015-12-07 11:06 GMT+01:00 Tobias Burnus <tobias.bur...@physik.fu-berlin.de>:
> I wrote:
>> I wonder whether using
>>
>> __asm__ __volatile__ ("":::"memory");
>>
>> would be sufficient as it has a way lower overhead than
>> __sync_synchronize().
>
> Namely, something like the attached patch.
>
> Regarding the original patch submission: Is there a reason that you didn't
> include the test case of Deepak from 
> https://gcc.gnu.org/ml/fortran/2015-04/msg00062.html
> It should work as -fcoarray=lib -lcaf_single "dg-do run" test.
>
> Tobias
commit 69e650945454491bbaf81513a1eed10b5b6b0f45
Author: Alessandro Fanfarillo <fanfari...@ing.uniroma2.it>
Date:   Mon Dec 7 15:46:10 2015 +0100

Introducing __asm__ __volatile__ (:::memory) after image control 
statements, send and get

diff --git a/gcc/fortran/trans-intrinsic.c b/gcc/fortran/trans-intrinsic.c
index 21efe44..25ff311 100644
--- a/gcc/fortran/trans-intrinsic.c
+++ b/gcc/fortran/trans-intrinsic.c
@@ -1222,6 +1222,15 @@ gfc_conv_intrinsic_caf_get (gfc_se *se, gfc_expr *expr, 
tree lhs, tree lhs_kind,
   se->expr = res_var;
   if (array_expr->ts.type == BT_CHARACTER)
 se->string_length = argse.string_length;
+
+  /* It guarantees memory consistency within the same segment */
+  tmp = gfc_build_string_const (strlen ("memory")+1, "memory"),
+  tmp = build5_loc (input_location, ASM_EXPR, void_type_node,
+   gfc_build_string_const (1, ""), NULL_TREE, NULL_TREE,
+   tree_cons (NULL_TREE, tmp, NULL_TREE), NULL_TREE);
+  ASM_VOLATILE_P (tmp) = 1;
+  gfc_add_expr_to_block (>pre, tmp);
+
 }
 
 
@@ -1390,6 +1399,15 @@ conv_caf_send (gfc_code *code) {
   gfc_add_expr_to_block (, tmp);
   gfc_add_block_to_block (, _se.post);
   gfc_add_block_to_block (, _se.post);
+
+  /* It guarantees memory consistency within the same segment */
+  tmp = gfc_build_string_const (strlen ("memory")+1, "memory"),
+  tmp = build5_loc (input_location, ASM_EXPR, void_type_node,
+   gfc_build_string_const (1, ""), NULL_TREE, NULL_TREE,
+   tree_cons (NULL_TREE, tmp, NULL_TREE), NULL_TREE);
+  ASM_VOLATILE_P (tmp) = 1;
+  gfc_add_expr_to_block (, tmp);
+
   return gfc_finish_block ();
 }
 
diff --git a/gcc/fortran/trans-stmt.c b/gcc/fortran/trans-stmt.c
index 3df483a..b7e1faa 100644
--- a/gcc/fortran/trans-stmt.c
+++ b/gcc/fortran/trans-stmt.c
@@ -818,6 +818,15 @@ gfc_trans_lock_unlock (gfc_code *code, gfc_exec_op op)
   errmsg, errmsg_len);
   gfc_add_expr_to_block (, tmp);
 
+  /* It guarantees memory consistency within the same segment */
+  tmp = gfc_build_string_const (strlen ("memory")+1, "memory"),
+  tmp = build5_loc (input_location, ASM_EXPR, void_type_node,
+   gfc_build_string_const (1, ""), NULL_TREE, NULL_TREE,
+   tree_cons (NULL_TREE, tmp, NULL_TREE), NULL_TREE);
+  ASM_VOLATILE_P (tmp) = 1;
+
+  gfc_add_expr_to_block (, tmp);
+
   if (stat2 != NULL_TREE)
gfc_add_modify (, stat2,
fold_convert (TREE_TYPE (stat2), stat));
@@ -995,6 +1004,14 @@ gfc_trans_event_post_wait (gfc_code *code, gfc_exec_op op)
   errmsg, errmsg_len);
   gfc_add_expr_to_block (, tmp);
 
+  /* It guarantees memory consistency within the same segment */
+  tmp = gfc_build_string_const (strlen ("memory")+1, "memory"),
+  tmp = build5_loc (input_location, ASM_EXPR, void_type_node,
+   gfc_build_string_const (1, ""), NULL_TREE, NULL_TREE,
+   tree_cons (NULL_TREE, tmp, NULL_TREE), NULL_TREE);
+  ASM_VOLATILE_P (tmp) = 1;
+  gfc_add_expr_to_block (, tmp);
+
   if (stat2 != NULL_TREE)
 gfc_add_modify (, stat2, fold_convert (TREE_TYPE (stat2), stat));
 
@@ -1080,6 +1097,18 @@ gfc_trans_sync (gfc_code *code, gfc_exec_op type)
   fold_convert (integer_type_node, images));
 }
 
+  /* Per F2008, 8.5.1, a SYNC MEMORY is implied by calling the
+ image control statements SYNC IMAGES and SYNC ALL.  */
+  if (flag_coarray == GFC_FCOARRAY_LIB)
+{
+  tmp = gfc_build_string_const (strlen ("memory")+1, "memory"),
+  tmp = build5_loc (input_location, ASM_EXPR, void_type_node,
+   gfc_build_string_const (1, ""), NULL_TREE, NULL_TREE,
+   tree_cons (NULL_TREE, tmp, NULL_TREE), NULL_TREE);
+  ASM_VOLATILE_P (tmp) = 1;
+  gfc_add_expr_to_block (, tmp);
+}
+
   if (flag_coarray != GFC_FCOARRAY_LIB)
 {
   /* Set STAT to zero.  */
diff --git a/gcc/fortran/trans.c b/gcc/fortran/trans.c
index 001db41..1993743 

Re: [Fortran, Patch] Memory sync after coarray image control statements and assignment

[Alessandro Fanfarillo]

Hi,

2015-12-07 8:20 GMT+01:00 Tobias Burnus :
> Always - or only with optimization?
>

Only with optimization.

> I wonder whether using
>
> __asm__ __volatile__ ("":::"memory");
>
> would be sufficient as it has a way lower overhead than
> __sync_synchronize().
>
>
> That would be something like:
>
>   r = build_stmt (input_location, ASM_EXPR, string,
>   output_operands, input_operands,
>   clobbers, labels);
>   ASM_VOLATILE_P (r) = 1;
>
> with string = "", output_operands = NULL_TREE, input_operands = NULL_TREE,
> clobbers = "memory" and labels = NULL_TREE.  (Except that string+clobbers
> are trees and not char[].)
>

I'm going to try it. Thanks.

[Fortran, Patch] Memory sync after coarray image control statements and assignment

[Alessandro Fanfarillo]

Dear all,

currently, a coarray assignment in a program composed by a single
segment (without any sync statements) produces wrong results.
Furthermore, a coarray code
compiled with an optimization flag higher that -O0 may produce wrong
results. The patch (re)introduces a __sync_synchronize() after coarray
image control statements (sync all, sync images, critical, locks and
events) and get/put.

The attached patch fixes the problems reported by Deepak in the
following discussion:
https://gcc.gnu.org/ml/fortran/2015-04/msg00062.html.

Built and regtested on x86_64-pc-linux-gnu.


PS: Adding the __sync_synchronize() after get/put and sync statements
fix all the problems reported by Deepak. I do the same also for locks,
critical and events. Suggestions are warmly welcome.
commit 75c93d3085116748115c8f69ad5ad58f4ad9369c
Author: Alessandro Fanfarillo <fanfari...@ing.uniroma2.it>
Date:   Sun Dec 6 18:50:51 2015 +0100

Introducing __sync_synchronize() after image control statements, send and 
get

diff --git a/gcc/fortran/ChangeLog b/gcc/fortran/ChangeLog
index ba176a1..b450fae 100644
--- a/gcc/fortran/ChangeLog
+++ b/gcc/fortran/ChangeLog
@@ -1,3 +1,13 @@
+2015-12-06  Alessandro Fanfarillo <fanfarillo@gmail.com>
+
+   * trans.c (gfc_allocate_using_lib,gfc_deallocate_with_status):
+   Introducing __sync_synchronize() after image control statements.
+   * trans-stmt.c (gfc_trans_sync,gfc_trans_event_post_wait,
+   gfc_trans_lock_unlock): Ditto.
+   * trans-intrinsic.c (gfc_conv_intrinsic_caf_get,
+   conv_caf_send): Introducing __sync_synchronize() after send,
+   get and sendget.
+
 2015-12-05  Paul Thomas  <pa...@gcc.gnu.org>
 
PR fortran/68676
diff --git a/gcc/fortran/trans-intrinsic.c b/gcc/fortran/trans-intrinsic.c
index 21efe44..07795ca 100644
--- a/gcc/fortran/trans-intrinsic.c
+++ b/gcc/fortran/trans-intrinsic.c
@@ -1222,6 +1222,12 @@ gfc_conv_intrinsic_caf_get (gfc_se *se, gfc_expr *expr, 
tree lhs, tree lhs_kind,
   se->expr = res_var;
   if (array_expr->ts.type == BT_CHARACTER)
 se->string_length = argse.string_length;
+
+  /* It guarantees memory consistency within the same segment */
+  tmp = builtin_decl_explicit (BUILT_IN_SYNC_SYNCHRONIZE);
+  tmp = build_call_expr_loc (input_location, tmp, 0);
+  gfc_add_expr_to_block (>pre, tmp);
+
 }
 
 
@@ -1390,6 +1396,12 @@ conv_caf_send (gfc_code *code) {
   gfc_add_expr_to_block (, tmp);
   gfc_add_block_to_block (, _se.post);
   gfc_add_block_to_block (, _se.post);
+
+  /* It guarantees memory consistency within the same segment */
+  tmp = builtin_decl_explicit (BUILT_IN_SYNC_SYNCHRONIZE);
+  tmp = build_call_expr_loc (input_location, tmp, 0);
+  gfc_add_expr_to_block (, tmp);
+
   return gfc_finish_block ();
 }
 
diff --git a/gcc/fortran/trans-stmt.c b/gcc/fortran/trans-stmt.c
index 3df483a..b1de0f5 100644
--- a/gcc/fortran/trans-stmt.c
+++ b/gcc/fortran/trans-stmt.c
@@ -818,6 +818,11 @@ gfc_trans_lock_unlock (gfc_code *code, gfc_exec_op op)
   errmsg, errmsg_len);
   gfc_add_expr_to_block (, tmp);
 
+  /* It guarantees memory consistency within the same segment */
+  tmp = builtin_decl_explicit (BUILT_IN_SYNC_SYNCHRONIZE);
+  tmp = build_call_expr_loc (input_location, tmp, 0);
+  gfc_add_expr_to_block (, tmp);
+
   if (stat2 != NULL_TREE)
gfc_add_modify (, stat2,
fold_convert (TREE_TYPE (stat2), stat));
@@ -995,6 +1000,11 @@ gfc_trans_event_post_wait (gfc_code *code, gfc_exec_op op)
   errmsg, errmsg_len);
   gfc_add_expr_to_block (, tmp);
 
+  /* It guarantees memory consistency within the same segment */
+  tmp = builtin_decl_explicit (BUILT_IN_SYNC_SYNCHRONIZE);
+  tmp = build_call_expr_loc (input_location, tmp, 0);
+  gfc_add_expr_to_block (, tmp);
+
   if (stat2 != NULL_TREE)
 gfc_add_modify (, stat2, fold_convert (TREE_TYPE (stat2), stat));
 
@@ -1080,6 +1090,15 @@ gfc_trans_sync (gfc_code *code, gfc_exec_op type)
   fold_convert (integer_type_node, images));
 }
 
+  /* Per F2008, 8.5.1, a SYNC MEMORY is implied by calling the
+ image control statements SYNC IMAGES and SYNC ALL.  */
+  if (flag_coarray == GFC_FCOARRAY_LIB)
+{
+  tmp = builtin_decl_explicit (BUILT_IN_SYNC_SYNCHRONIZE);
+  tmp = build_call_expr_loc (input_location, tmp, 0);
+  gfc_add_expr_to_block (, tmp);
+}
+  
   if (flag_coarray != GFC_FCOARRAY_LIB)
 {
   /* Set STAT to zero.  */
diff --git a/gcc/fortran/trans.c b/gcc/fortran/trans.c
index 001db41..e7803bd 100644
--- a/gcc/fortran/trans.c
+++ b/gcc/fortran/trans.c
@@ -746,6 +746,11 @@ gfc_allocate_using_lib (stmtblock_t * block, tree pointer, 
tree size,
 TREE_TYPE (pointer), pointer,
 fold_convert ( TREE_TYPE (pointer), tmp));
   gfc_add_expr_to_block (block, tmp);
+
+  /* It guarantees memory consistency wit

Re: [Fortran, Patch] (RFC, Coarray) Implement TS18508's EVENTS

[Alessandro Fanfarillo]

Yes please.

Thanks.

2015-12-02 23:00 GMT+01:00 Steve Kargl <s...@troutmask.apl.washington.edu>:
> Committed as revision 231208.
>
> Alessandro, Tobias, is this a candidate for a commit to
> the 5-branch when it is re-opened?
>
> --
> steve
>
> On Wed, Dec 02, 2015 at 03:16:05PM +0100, Alessandro Fanfarillo wrote:
>> *PING*
>>
>> 2015-11-26 17:51 GMT+01:00 Steve Kargl <s...@troutmask.apl.washington.edu>:
>> > On Wed, Nov 25, 2015 at 06:24:49PM +0100, Alessandro Fanfarillo wrote:
>> >> Dear all,
>> >>
>> >> in attachment the previous patch compatible with the current trunk.
>> >> The patch also includes the changes introduced in the latest TS 18508.
>> >>
>> >> Built and regtested on x86_64-pc-linux-gnu.
>> >>
>> >> PS: I will add the test cases in a different patch.
>> >>
>> >
>> > I have now built and regression tested the patch on
>> > x86_64-*-freebsd and i386-*-freebsd.  There were no
>> > regressions.  In reading through the patch, nothing
>> > jumped out at me as suspicious/wrong.  Tobias, this
>> > is OK to commit.  If you don't committed by Sunday,
>> > I'll do it for you.
>> >
>> > --
>> > steve
>
> --
> Steve

Re: [Fortran, Patch] (RFC, Coarray) Implement TS18508's EVENTS

[Alessandro Fanfarillo]

*PING*

2015-11-26 17:51 GMT+01:00 Steve Kargl <s...@troutmask.apl.washington.edu>:
> On Wed, Nov 25, 2015 at 06:24:49PM +0100, Alessandro Fanfarillo wrote:
>> Dear all,
>>
>> in attachment the previous patch compatible with the current trunk.
>> The patch also includes the changes introduced in the latest TS 18508.
>>
>> Built and regtested on x86_64-pc-linux-gnu.
>>
>> PS: I will add the test cases in a different patch.
>>
>
> I have now built and regression tested the patch on
> x86_64-*-freebsd and i386-*-freebsd.  There were no
> regressions.  In reading through the patch, nothing
> jumped out at me as suspicious/wrong.  Tobias, this
> is OK to commit.  If you don't committed by Sunday,
> I'll do it for you.
>
> --
> steve

Re: [Fortran, Patch] (RFC, Coarray) Implement TS18508's EVENTS

[Alessandro Fanfarillo]

Hi all,

in attachment the patch for tests and missing functions in
libcaf_single (needed by the test suite).

Built and regtested on x86_64-pc-linux-gnu.

2015-11-26 17:51 GMT+01:00 Steve Kargl <s...@troutmask.apl.washington.edu>:
> On Wed, Nov 25, 2015 at 06:24:49PM +0100, Alessandro Fanfarillo wrote:
>> Dear all,
>>
>> in attachment the previous patch compatible with the current trunk.
>> The patch also includes the changes introduced in the latest TS 18508.
>>
>> Built and regtested on x86_64-pc-linux-gnu.
>>
>> PS: I will add the test cases in a different patch.
>>
>
> I have now built and regression tested the patch on
> x86_64-*-freebsd and i386-*-freebsd.  There were no
> regressions.  In reading through the patch, nothing
> jumped out at me as suspicious/wrong.  Tobias, this
> is OK to commit.  If you don't committed by Sunday,
> I'll do it for you.
>
> --
> steve
commit d68b49bae714a7b5881587a61d30d643fa0cdb90
Author: Alessandro Fanfarillo <fanfari...@ing.uniroma2.it>
Date:   Thu Nov 26 18:51:47 2015 +0100

New tests for coarray events and new functions in libcaf_single

diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index e0b16f5..bcc99ea 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,9 @@
+2015-11-26  Tobias Burnus  <bur...@net-b.de>
+Alessandro Fanfarillo <fanfarillo@gmail.com>
+
+   * gfortran.dg/coarray/event_1.f90: New.
+   * gfortran.dg/coarray/event_2.f90: New.
+
 2015-11-25  Markus Trippelsdorf  <mar...@trippelsdorf.de>
Paolo Carlini  <paolo.carl...@oracle.com>
 
diff --git a/gcc/testsuite/gfortran.dg/coarray/event_1.f90 
b/gcc/testsuite/gfortran.dg/coarray/event_1.f90
new file mode 100644
index 000..b4385f3
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/coarray/event_1.f90
@@ -0,0 +1,51 @@
+! { dg-do run }
+!
+! Run-time test for EVENT_TYPE
+!
+use iso_fortran_env, only: event_type
+implicit none
+
+type(event_type), save :: var[*]
+integer :: count, stat
+
+count = -42
+call event_query (var, count)
+if (count /= 0) call abort()
+
+stat = 99
+event post (var, stat=stat)
+if (stat /= 0) call abort()
+call event_query(var, count, stat=stat)
+if (count /= 1 .or. stat /= 0) call abort()
+
+stat = 99
+event post (var[this_image()])
+call event_query(var, count)
+if (count /= 2) call abort()
+
+stat = 99
+event wait (var)
+call event_query(var, count)
+if (count /= 1) call abort()
+
+stat = 99
+event post (var)
+call event_query(var, count)
+if (count /= 2) call abort()
+
+stat = 99
+event post (var)
+call event_query(var, count)
+if (count /= 3) call abort()
+
+stat = 99
+event wait (var, until_count=2)
+call event_query(var, count)
+if (count /= 1) call abort()
+
+stat = 99
+event wait (var, stat=stat, until_count=1)
+if (stat /= 0) call abort()
+call event_query(event=var, stat=stat, count=count)
+if (count /= 0 .or. stat /= 0) call abort()
+end
diff --git a/gcc/testsuite/gfortran.dg/coarray/event_2.f90 
b/gcc/testsuite/gfortran.dg/coarray/event_2.f90
new file mode 100644
index 000..2d451a5
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/coarray/event_2.f90
@@ -0,0 +1,89 @@
+! { dg-do run }
+!
+! Run-time test for EVENT_TYPE
+!
+use iso_fortran_env, only: event_type
+implicit none
+
+type(event_type), save, allocatable :: var(:)[:]
+integer :: count, stat
+
+allocate(var(3)[*])
+
+count = -42
+call event_query (var(1), count)
+if (count /= 0) call abort()
+call event_query (var(1), count)
+if (count /= 0) call abort()
+call event_query (var(2), count)
+if (count /= 0) call abort()
+call event_query (var(3), count)
+if (count /= 0) call abort()
+
+stat = 99
+event post (var(2), stat=stat)
+if (stat /= 0) call abort()
+call event_query (var(1), count)
+if (count /= 0) call abort()
+call event_query(var(2), count, stat=stat)
+if (count /= 1 .or. stat /= 0) call abort()
+call event_query (var(3), count)
+if (count /= 0) call abort()
+
+stat = 99
+event post (var(2)[this_image()])
+call event_query(var(1), count)
+if (count /= 0) call abort()
+call event_query(var(2), count)
+if (count /= 2) call abort()
+call event_query(var(2), count)
+if (count /= 2) call abort()
+call event_query(var(3), count)
+if (count /= 0) call abort()
+
+stat = 99
+event wait (var(2))
+call event_query(var(1), count)
+if (count /= 0) call abort()
+call event_query(var(2), count)
+if (count /= 1) call abort()
+call event_query(var(3), count)
+if (count /= 0) call abort()
+
+stat = 99
+event post (var(2))
+call event_query(var(1), count)
+if (count /= 0) call abort()
+call event_query(var(2), count)
+if (count /= 2) call abort()
+call event_query(var(3), count)
+if (count /= 0) call abort()
+
+stat = 99
+event post (var(2))
+call event_query(var(1), count)
+if (count /= 0) call abort()
+call event_query(var(2), count)
+if (count /= 3) call abort()
+call event_query(var(3), count)
+if (count /= 0) call ab

Re: [Fortran, Patch] (RFC, Coarray) Implement TS18508's EVENTS

[Alessandro Fanfarillo]

Dear all,

in attachment the previous patch compatible with the current trunk.
The patch also includes the changes introduced in the latest TS 18508.

Built and regtested on x86_64-pc-linux-gnu.

PS: I will add the test cases in a different patch.

2015-04-29 9:55 GMT+02:00 Tobias Burnus <tobias.bur...@physik.fu-berlin.de>:
> Dear all,
>
> attached patch fixes a bug and implements EVENTS. I think the patch is
> finished, but I still have to extend the test cases, to re-read the
> patch and to write a changelog. As I am not sure how soon I will able
> to do so, I follow the paradigm: release soon, release often and post
> it here. Comments and reviews are welcome.
>
> The patch fixes two bug in the "errmsg" handling, found by Alessandro:
> I don't pass on the address of the actual argument and in libcaf_single,
> I copy only 8 characters (sizeof pointer) instead of all of the the
> characters of the error message.
>
> Regarding events: Events is a way to permit barrier/locking-free
> programming: One sends the finished data to an other image and then
> tells that image that the data is there ("event post(msg_var[idx])").
> That image can then either wait for the event by querying the status
> on the local variable ("event wait(msg_var)") or only check the status
> and if it is not ready do something else (e.g. another iteration);
> that's done via "call event_query(msg_var, count)".
>
> Technically, event_post works like atomic_add(msg_var[idx], 1) and
> event_query like "atomic_ref(msg_var, count)". event_wait is the same
> as event_query plus a spin/sleep loop waiting for the status change,
> followed by an atomic_add(msg_var, -until_count). Except that
> event_post/event_wait are image control statements. (Otherwise it
> could happen that the event is there before the data for which the
> event has been posted.)
>
> Regarding the implementation in this patch, the limitations are the
> same as for locking: Currently, neither lock_type nor event_type
> variables are permitted as (nonallocatable) components
> of a derived type - and type extension of them also not yet supported.*
>
> The spec can be found at http://bitly.com/sc22wg5 -> 2015 -> TS draft
> or directly at
> http://isotc.iso.org/livelink/livelink?func=ll=17064344=Open
>
> Tobias
>
>
> * Doing so is not really difficult but I need to handle cases like
> the following. For "allocatable" with SOURCE= I also need to handle
> it with polymorphic types.
>
> type t1
>   type(event_type) :: EV
>   type(lock_type) :: LK
> end type1
> type t2
>   type(t1) :: a(5)
> end type t2
> type t3
>   type(t2) :: b(8)
> end type t3
>
> type(t3), save :: caf(3)[*]
>
> For those, I need to call _gfortran_caf_register for
>   caf(:)%b(:)%a(:)%ev and caf(:)%b(:)%a(:)%lk
> Looping though all array references.
>
> Similar for
>   type(t3), allocatable :: caf2(:)[:]
>   allocate(caf2(n)[*])
> for the allocate call.
commit 4ebad7d27c29884eb7eed8ddc0628c9b6dc64622
Author: Alessandro Fanfarillo <fanfari...@ing.uniroma2.it>
Date:   Wed Nov 25 14:46:07 2015 +0100

ChangeLog update

diff --git a/gcc/fortran/ChangeLog b/gcc/fortran/ChangeLog
index 0c81201..3d2c4cf 100644
--- a/gcc/fortran/ChangeLog
+++ b/gcc/fortran/ChangeLog
@@ -1,3 +1,62 @@
+2015-11-25  Tobias Burnus  <bur...@net-b.de>
+   Alessandro Fanfarillo <fanfarillo@gmail.com>
+
+   * check.c (gfc_check_event_query): New function.
+   * dump-parse-tree.c (show_code_node): Handle EXEC_EVENT_POST,
+   EXEC_EVENT_WAIT.
+   * expr.c (gfc_check_vardef_context): New check for event variables
+   definition.
+   * gfortran.h (gfc_statement): Add ST_EVENT_POST, ST_EVENT_WAIT.
+   (gfc_isym_id): GFC_ISYM_EVENT_QUERY.
+   (struct symbol_attribute): New field.
+   (gfc_exec_op): Add EXEC_EVENT_POST and EXEC_EVENT_WAIT.
+   * gfortran.texi: Document about new events functions and minor
+   changes.
+   * interface.c (compare_parameter): New check.
+   (gfc_procedure_use): New check for explicit procedure interface.
+   (add_subroutines): Add event_query.
+   * intrinsic.h (gfc_check_event_query,gfc_resolve_event_query):
+   New prototypes.
+   * iresolve.c (gfc_resolve_event_query): New function.
+   * iso-fortran-env.def (event_type): New type.
+   * match.c (event_statement,gfc_match_event_post,gfc_match_event_wait):
+   New functions.
+   (gfc_match_name): New event post and event wait.
+   * match.h (gfc_match_event_post,gfc_match_event_wait):
+   New prototypes.
+   * module.c (ab_attribute): Add AB_EVENT_COMP.
+   (attr_bits): Likewise.
+   (mio_symbol_attribute): Handle event_comp attribute.
+   * parse.c (decode_s

Re: [Fortran, Patch] Passing function pointer to co_reduce

[Alessandro Fanfarillo]

Thanks a lot!

2015-07-17 15:50 GMT+02:00 Mikael Morin mikael.mo...@sfr.fr:
 Le 17/07/2015 11:02, Mikael Morin a écrit :
 Le 16/07/2015 16:34, Damian Rouson a écrit :
 Alternatively, if it’s easy, please feel free to add the directives and 
 commit.


 Never mind, I'll take care of it all.

 This is what I have committed:
 https://gcc.gnu.org/r225930 (trunk)
 https://gcc.gnu.org/r225932 (5 branch)

 Mikael

[Fortran, Patch] Passing function pointer to co_reduce

[Alessandro Fanfarillo]

Dear all,

during the implementation of co_reduce in OpenCoarrays I noticed that
GFortran passes a pointer to function instead of the function name to
co_reduce.

Currently the compiler produces the following call:

_gfortran_caf_co_reduce (desc.0, simple_reduction, 0, 0, 0B, 0B, 0, 0);

where simple_reduction is the pure function that has to be used by co_reduce.

The attached patch seems to fix the issue, any comments?


Regards

Alessandro

PS: I also attach the test case
commit a12b6ce6993df109c81a32d5684b4b9f41f69ea4
Author: Alessandro Fanfarillo fanfari...@ing.uniroma2.it
Date:   Mon Jul 13 15:46:19 2015 +0200

Fix function pointer argument for co_reduce

diff --git a/gcc/fortran/trans-intrinsic.c b/gcc/fortran/trans-intrinsic.c
index 66bc72a..967a741 100644
--- a/gcc/fortran/trans-intrinsic.c
+++ b/gcc/fortran/trans-intrinsic.c
@@ -8804,7 +8804,7 @@ conv_co_collective (gfc_code *code)
}
   opr_flags = build_int_cst (integer_type_node, opr_flag_int);
   gfc_conv_expr (argse, opr_expr);
-  opr = gfc_build_addr_expr (NULL_TREE, argse.expr);
+  opr = argse.expr;
   fndecl = build_call_expr_loc (input_location, fndecl, 8, array, opr, 
opr_flags,
image_index, stat, errmsg, strlen, 
errmsg_len);
 }
program simple_reduce
  implicit none

  integer :: me

  me = this_image()

  sync all

  call co_reduce(me,simple_reduction)

  write(*,*) this_image(),me

contains
  
  pure function simple_reduction(a,b)
integer,intent(in) :: a,b
integer :: simple_reduction

simple_reduction = a * b
  end function simple_reduction

end program simple_reduce

Re: [PATCH, Fortran] Sync memory action delegated to OpenCoarrays

[Alessandro Fanfarillo]

Totally fine with me. Thanks.

2015-03-10 0:20 GMT-07:00 Tobias Burnus bur...@net-b.de:
 Hi Alessandro,

 Alessandro Fanfarillo wrote:

 Thanks for the quick response. Patch built and regtested on
 x86_64-unknown-linux-gnu.


 Actually, I also was about to send a reworked patch myself - see attachment.
 Additional changes compared to your last patch:
 * Add the assembler statement to libcaf_single also for the other syncs.
 * Tested also the passing of the other arguments in the test case.
 * Removed the call to sync_synchronize in the compiler - leaving it
 completely to the library. (Existed for end of program, STOP and SYNC ...;
 was only issued with -fcoarray=lib)

 Are the changes okay from your side? Then I'd prefer my patch - and I'll
 commit it this evening.

  * * *

 Currently the stub for sync memory in single.c invokes asm volatile
 ( : : : memory) but _gfortran_caf_sync_memory is not called when
 the code is compiled with -fcoarray=single.


 I misremembered that we do call __sync_synchronize for -fcoarray=single. As
 it turned out, we only call it for -fcoarray=lib - and it makes sense (in my
 opinion) to leave such calls to the library.

 As you, I also added the asm to the library - which probably has no effect
 - except when the library and the program are both compiled with LTO. Still,
 it somehow looks more complete.


 Regarding: Looks good to me - except that I prefer the additional items of
 my patch - and that the ChangeLog doesn't conform to the GCC style:
 - The file names shall be relative to the change log, i.e. caf/single.c
 (as the ChangeLog file is in libgfortran/ChangeLog), similarly a
 gfortran.dg/ is missing for the test case.
 - There should be a single tab*  before the file names, not two tabs and
 no * .
 - After the file name, the changed symbols/function names shall be put in
 parentheses.
 - The item describing the change should end with a full stop.
 - The email address should be enclosed in .
 (Often missed, but not by you: date-name-email separation is two spaces.)

 Tobias


 Please let me know if I have to change the patch for -fcoarray=single.



 2015-03-06 11:20 GMT-08:00 Tobias Burnus bur...@net-b.de:

 Dear Alessandro,

 Alessandro Fanfarillo wrote:

 so far a sync memory statement is translated into a local
 __sync_synchronize ().
 The attached draft patch delegates the action for sync memory (when
 -fcoarray=lib is used) to the external function
 _gfortran_caf_sync_memory() implemented in the OpenCoarrays library.


 Looks good to me. However, you should add a test case with 'dg-options
 -fdump-tree-original -fcoarray=lib' to check that this works; cf.
 gcc/testsuite/gfortran.dg/coarray*.f90 for examples.

 And you have to provide a stub implementation in
 libgfortran/caf/{libcaf.h,single.c}.

 Tobias

 PS: I wonder whether it makes sense to remove the __sync_synchronize for
 -fcoarray=single and replace it by the equivalent to asm volatile ( :
 : :
 memory). It almost certainly does.

Re: [PATCH, Fortran] Sync memory action delegated to OpenCoarrays

[Alessandro Fanfarillo]

Thanks for the quick response.

Patch built and regtested on x86_64-unknown-linux-gnu.

Currently the stub for sync memory in single.c invokes asm volatile
( : : : memory) but _gfortran_caf_sync_memory is not called when
the code is compiled with -fcoarray=single.

Please let me know if I have to change the patch for -fcoarray=single.



2015-03-06 11:20 GMT-08:00 Tobias Burnus bur...@net-b.de:
 Dear Alessandro,

 Alessandro Fanfarillo wrote:

 so far a sync memory statement is translated into a local
 __sync_synchronize ().
 The attached draft patch delegates the action for sync memory (when
 -fcoarray=lib is used) to the external function
 _gfortran_caf_sync_memory() implemented in the OpenCoarrays library.


 Looks good to me. However, you should add a test case with 'dg-options
 -fdump-tree-original -fcoarray=lib' to check that this works; cf.
 gcc/testsuite/gfortran.dg/coarray*.f90 for examples.

 And you have to provide a stub implementation in
 libgfortran/caf/{libcaf.h,single.c}.

 Tobias

 PS: I wonder whether it makes sense to remove the __sync_synchronize for
 -fcoarray=single and replace it by the equivalent to asm volatile ( : : :
 memory). It almost certainly does.
2015-03-09  Alessandro Fanfarillo  fanfarillo@gmail.com

trans-decl.c: Add function declaration caf_sync_memory
trans.h: Ditto
trans-stmt.c: Add caf_sync_memory invocation
coarray_38.f90: Test case
single.c: caf_sync_memory implementation
libcaf.h: caf_sync_memory prototype

diff --git a/gcc/fortran/trans-decl.c b/gcc/fortran/trans-decl.c
index 3664824..a66c25c 100644
--- a/gcc/fortran/trans-decl.c
+++ b/gcc/fortran/trans-decl.c
@@ -153,6 +153,7 @@ tree gfor_fndecl_caf_get;
 tree gfor_fndecl_caf_send;
 tree gfor_fndecl_caf_sendget;
 tree gfor_fndecl_caf_sync_all;
+tree gfor_fndecl_caf_sync_memory;
 tree gfor_fndecl_caf_sync_images;
 tree gfor_fndecl_caf_error_stop;
 tree gfor_fndecl_caf_error_stop_str;
@@ -3451,6 +3452,10 @@ gfc_build_builtin_function_decls (void)
get_identifier (PREFIX(caf_sync_all)), .WW, void_type_node,
3, pint_type, pchar_type_node, integer_type_node);
 
+  gfor_fndecl_caf_sync_memory = gfc_build_library_function_decl_with_spec (
+   get_identifier (PREFIX(caf_sync_memory)), .WW, void_type_node,
+   3, pint_type, pchar_type_node, integer_type_node);
+
   gfor_fndecl_caf_sync_images = gfc_build_library_function_decl_with_spec (
get_identifier (PREFIX(caf_sync_images)), .RRWW, void_type_node,
5, integer_type_node, pint_type, pint_type,
diff --git a/gcc/fortran/trans-stmt.c b/gcc/fortran/trans-stmt.c
index 505f905..dce0e87 100644
--- a/gcc/fortran/trans-stmt.c
+++ b/gcc/fortran/trans-stmt.c
@@ -768,8 +768,7 @@ gfc_trans_sync (gfc_code *code, gfc_exec_op type)
   else
 stat = null_pointer_node;
 
-  if (code-expr3  flag_coarray == GFC_FCOARRAY_LIB
-   type != EXEC_SYNC_MEMORY)
+  if (code-expr3  flag_coarray == GFC_FCOARRAY_LIB)
 {
   gcc_assert (code-expr3-expr_type == EXPR_VARIABLE);
   gfc_init_se (argse, NULL);
@@ -778,7 +777,7 @@ gfc_trans_sync (gfc_code *code, gfc_exec_op type)
   errmsg = gfc_build_addr_expr (NULL, argse.expr);
   errmsglen = argse.string_length;
 }
-  else if (flag_coarray == GFC_FCOARRAY_LIB  type != EXEC_SYNC_MEMORY)
+  else if (flag_coarray == GFC_FCOARRAY_LIB)
 {
   errmsg = null_pointer_node;
   errmsglen = build_int_cst (integer_type_node, 0);
@@ -822,13 +821,13 @@ gfc_trans_sync (gfc_code *code, gfc_exec_op type)
gfc_add_expr_to_block (se.pre, tmp);
  }
 
-  if (flag_coarray != GFC_FCOARRAY_LIB || type == EXEC_SYNC_MEMORY)
+  if (flag_coarray != GFC_FCOARRAY_LIB)
 {
   /* Set STAT to zero.  */
   if (code-expr2)
gfc_add_modify (se.pre, stat, build_int_cst (TREE_TYPE (stat), 0));
 }
-  else if (type == EXEC_SYNC_ALL)
+  else if (type == EXEC_SYNC_ALL  || type == EXEC_SYNC_MEMORY)
 {
   /* SYNC ALL   =   stat == null_pointer_node
 SYNC ALL(stat=s)   =   stat has an integer type
@@ -840,8 +839,13 @@ gfc_trans_sync (gfc_code *code, gfc_exec_op type)
  if (TREE_TYPE (stat) == integer_type_node)
stat = gfc_build_addr_expr (NULL, stat);
 
- tmp = build_call_expr_loc (input_location, gfor_fndecl_caf_sync_all,
-3, stat, errmsg, errmsglen);
+ if(type == EXEC_SYNC_MEMORY)
+   tmp = build_call_expr_loc (input_location, 
gfor_fndecl_caf_sync_memory,
+  3, stat, errmsg, errmsglen);
+ else
+   tmp = build_call_expr_loc (input_location, gfor_fndecl_caf_sync_all,
+  3, stat, errmsg, errmsglen);
+
  gfc_add_expr_to_block (se.pre, tmp);
}
   else
diff --git a/gcc/fortran/trans.h b/gcc/fortran/trans.h
index bd1520a..3ba2f88 100644
--- a/gcc/fortran/trans.h
+++ b/gcc/fortran/trans.h

[PATCH, Fortran] Sync memory action delegated to OpenCoarrays

[Alessandro Fanfarillo]

Dear all,

so far a sync memory statement is translated into a local
__sync_synchronize ().
The attached draft patch delegates the action for sync memory (when
-fcoarray=lib is used) to the external function
_gfortran_caf_sync_memory() implemented in the OpenCoarrays library.

Any suggestions?

Best regards

Alessandro
commit fdb560e3f548fe60edf26717b8de6bb92f3fd555
Author: Alessandro Fanfarillo fanfarillo@gmail.com
Date:   Fri Mar 6 10:25:56 2015 -0800

Sync memory is translated into an invokation to _gfortran_caf_sync_memory()

diff --git a/gcc/fortran/trans-decl.c b/gcc/fortran/trans-decl.c
index 3664824..a66c25c 100644
--- a/gcc/fortran/trans-decl.c
+++ b/gcc/fortran/trans-decl.c
@@ -153,6 +153,7 @@ tree gfor_fndecl_caf_get;
 tree gfor_fndecl_caf_send;
 tree gfor_fndecl_caf_sendget;
 tree gfor_fndecl_caf_sync_all;
+tree gfor_fndecl_caf_sync_memory;
 tree gfor_fndecl_caf_sync_images;
 tree gfor_fndecl_caf_error_stop;
 tree gfor_fndecl_caf_error_stop_str;
@@ -3451,6 +3452,10 @@ gfc_build_builtin_function_decls (void)
get_identifier (PREFIX(caf_sync_all)), .WW, void_type_node,
3, pint_type, pchar_type_node, integer_type_node);
 
+  gfor_fndecl_caf_sync_memory = gfc_build_library_function_decl_with_spec (
+   get_identifier (PREFIX(caf_sync_memory)), .WW, void_type_node,
+   3, pint_type, pchar_type_node, integer_type_node);
+
   gfor_fndecl_caf_sync_images = gfc_build_library_function_decl_with_spec (
get_identifier (PREFIX(caf_sync_images)), .RRWW, void_type_node,
5, integer_type_node, pint_type, pint_type,
diff --git a/gcc/fortran/trans-stmt.c b/gcc/fortran/trans-stmt.c
index 505f905..dce0e87 100644
--- a/gcc/fortran/trans-stmt.c
+++ b/gcc/fortran/trans-stmt.c
@@ -768,8 +768,7 @@ gfc_trans_sync (gfc_code *code, gfc_exec_op type)
   else
 stat = null_pointer_node;
 
-  if (code-expr3  flag_coarray == GFC_FCOARRAY_LIB
-   type != EXEC_SYNC_MEMORY)
+  if (code-expr3  flag_coarray == GFC_FCOARRAY_LIB)
 {
   gcc_assert (code-expr3-expr_type == EXPR_VARIABLE);
   gfc_init_se (argse, NULL);
@@ -778,7 +777,7 @@ gfc_trans_sync (gfc_code *code, gfc_exec_op type)
   errmsg = gfc_build_addr_expr (NULL, argse.expr);
   errmsglen = argse.string_length;
 }
-  else if (flag_coarray == GFC_FCOARRAY_LIB  type != EXEC_SYNC_MEMORY)
+  else if (flag_coarray == GFC_FCOARRAY_LIB)
 {
   errmsg = null_pointer_node;
   errmsglen = build_int_cst (integer_type_node, 0);
@@ -822,13 +821,13 @@ gfc_trans_sync (gfc_code *code, gfc_exec_op type)
gfc_add_expr_to_block (se.pre, tmp);
  }
 
-  if (flag_coarray != GFC_FCOARRAY_LIB || type == EXEC_SYNC_MEMORY)
+  if (flag_coarray != GFC_FCOARRAY_LIB)
 {
   /* Set STAT to zero.  */
   if (code-expr2)
gfc_add_modify (se.pre, stat, build_int_cst (TREE_TYPE (stat), 0));
 }
-  else if (type == EXEC_SYNC_ALL)
+  else if (type == EXEC_SYNC_ALL  || type == EXEC_SYNC_MEMORY)
 {
   /* SYNC ALL   =   stat == null_pointer_node
 SYNC ALL(stat=s)   =   stat has an integer type
@@ -840,8 +839,13 @@ gfc_trans_sync (gfc_code *code, gfc_exec_op type)
  if (TREE_TYPE (stat) == integer_type_node)
stat = gfc_build_addr_expr (NULL, stat);
 
- tmp = build_call_expr_loc (input_location, gfor_fndecl_caf_sync_all,
-3, stat, errmsg, errmsglen);
+ if(type == EXEC_SYNC_MEMORY)
+   tmp = build_call_expr_loc (input_location, 
gfor_fndecl_caf_sync_memory,
+  3, stat, errmsg, errmsglen);
+ else
+   tmp = build_call_expr_loc (input_location, gfor_fndecl_caf_sync_all,
+  3, stat, errmsg, errmsglen);
+
  gfc_add_expr_to_block (se.pre, tmp);
}
   else
diff --git a/gcc/fortran/trans.h b/gcc/fortran/trans.h
index bd1520a..3ba2f88 100644
--- a/gcc/fortran/trans.h
+++ b/gcc/fortran/trans.h
@@ -731,6 +731,7 @@ extern GTY(()) tree gfor_fndecl_caf_get;
 extern GTY(()) tree gfor_fndecl_caf_send;
 extern GTY(()) tree gfor_fndecl_caf_sendget;
 extern GTY(()) tree gfor_fndecl_caf_sync_all;
+extern GTY(()) tree gfor_fndecl_caf_sync_memory;
 extern GTY(()) tree gfor_fndecl_caf_sync_images;
 extern GTY(()) tree gfor_fndecl_caf_error_stop;
 extern GTY(()) tree gfor_fndecl_caf_error_stop_str;

Re: [Patch, Fortran] Add CO_BROADCAST

[Alessandro Fanfarillo]

Hi, I'm implementing the co_broadcast on libcafmpi right now.

2014-09-25 8:08 GMT+02:00 Tobias Burnus bur...@net-b.de:
 Hi Paul,

 Paul Richard Thomas wrote:

 In the check.c error messages, you use 'A argument'.  Should you not
 use 'SOURCE argument', following CO BROADCAST (SOURCE, SOURCE IMAGE [,
 STAT, ERRMSG]) ? I am looking at WG5/N1983 - is there some more recent
 proposal?


 Looking at N2027, I see A, SOURCE_IMAGE [, STAT, ERRMSG]. It might be that
 J3/WG5 decided that SOURCE= is a bad name on all but one image as it would
 for all other images a DESTINATION.

 Regarding the references: I try to keep
 https://gcc.gnu.org/wiki/GFortranStandards up to date; I think the latest
 draft is N2027:
 http://isotc.iso.org/livelink/livelink?func=llobjId=16769292objAction=Open

 Thanks for cross checking!

 When do you intend to implement a _gfortran_caf_co_broadcast that does
 something?


 Well, the current libgfortran/caf/single.c is fully compliant - for a single
 image. (Ignoring allocatable components and the lacking finalization.)

 I intend to leave the MPI and GASNet implementation to Alessandro, unless I
 feel really tempted to do it.

 Anway, the patch is OK for trunk.


 Thanks for the review! I committed the unmodified patch as Rev. 215579.

 Tobias


 On 20 September 2014 16:09, Tobias Burnus bur...@net-b.de wrote:

 This patch adds a CO_BROADCAST and prepares a bit for CO_REDUCE.

 Both functions permit arguments with allocatable components
 (nonpolymophic
 or polymorphic), CO_BROADCAST also permits polymorphic arguments. This
 patch
 doesn't support allocatable/polymorphic arguments but otherwise
 CO_BROADCAST
 should work. For CO_REDUCE only some parsing/argument checking is done
 but
 no actual implementation.

 The allocatables make life harder for general coarray communication,
 broadcast and reduction and have to be implemented at some point in a
 clever
 way. I am thinking of some call-back-able function - which could also be
 used for OpenMP 4.x/5.0 to handle copying to threadprivate variables and
 for
 copyin/out to accelerators; the current spec handles allocatable
 components
 by creating the copying code in the middle end, but that won't work for
 polymorphic allocatables.

 For CO_REDUCE, it becomes even harder as currently any pure function
 works
 (elemental or not, passing arguments with array descriptor, as pointer or
 as
 value, having a hidden string length argument or [with C binding] not
 etc.
 Requiring packed array arguments or not, whether gfortran returns the
 result
 as value or as argument - and possibly more). There is some J3 discussion
 if
 one could narrow down the possibilities a bit. In any case, implementing
 co_reduce requires some thinking.

 The attached patch was build and regtested on x86-64-gnu-linux.
 OK for the trunk?

 Tobias

Re: [PATCH, Fortran] Wrong invocation of caf_atomic_op

[Alessandro Fanfarillo]

Thanks a lot. I'm moving and I didn't have time to make a test case.
Regards

2014-09-20 8:14 GMT-06:00 Tobias Burnus bur...@net-b.de:
 Hi Alessandro et al.,

 I have now committed (Rev. 215421) the attached patch, which includes a test
 case for it.

 Thanks for the report and the patch Alessandro!

 Tobias


 On 15.09.2014 23:29, Tobias Burnus wrote:

 On 15.09.2014 22:23, Alessandro Fanfarillo wrote:

 In attachment a test case which fails with the current gcc-trunk
 version but works when the patch is applied. coarray_35.f90 is my
 attempt to write a gcc test case.
 The problem is related with atomic_add.


 Well, if it is a dg-do compile test, it won't exercise the issue: Even
 without the patch, it was compiling.

 It should either be a run test (dg-do run) – and then under
 gfortran.dg/coarray/ - which will automatically link libcaf_single to it.
 (And do another run with -fcoarray=single). – Or you have to additionally
 use -fdump-tree-original and scan for the strings, using dg-final {
 scan-tree-dump-times ... or scan-tree-dump-not. See other test cases there.

 I think ATOMIC_ADD should also fail without the patch, i.e. the run test
 really would test whether it works. Thus, that might be the simpler option.
 Or you do both – but the dump one shouldn't be under gfortran.dg/coarray/ as
 that is also run with -fcoarray=single, but under gfortran.dg/ directly.

 Tobias

Re: [PATCH, Fortran] Wrong invocation of caf_atomic_op

[Alessandro Fanfarillo]

New patch after the update.

Cheers

2014-09-09 0:30 GMT-06:00 Tobias Burnus bur...@net-b.de:
 Alessandro Fanfarillo wrote:

 This email follows the previous without subject (sorry about that).


 I think I'd prefer the following patch, which avoids a temporary if none is
 required. value is a pointer if the kind is the same (see kind check
 before) and if it is not a literal. Otherwise, it isn't a pointer and one
 needs to generate a temporary.

 I do not quite understand why the current check doesn't work as both are
 integer(kind=4) but for some reasons one has a variant.

 Additionally, I wonder whether one should add a test case – one probably
 should do – and of which kind (run test + fdump-tree-original?).

 Tobias

 --- a/gcc/fortran/trans-intrinsic.c
 +++ b/gcc/fortran/trans-intrinsic.c
 @@ -8398,3 +8398,3 @@ conv_intrinsic_atomic_op (gfc_code *code)

 - if (TREE_TYPE (TREE_TYPE (atom)) != TREE_TYPE (TREE_TYPE (value)))
 + if (!POINTER_TYPE_P (TREE_TYPE (value)))

 {




 The attached patch solves the problem raised by the following code:

 program atomic
 use iso_fortran_env
 implicit none

 integer :: me
 integer(atomic_int_kind) :: atom[*]
 me = this_image()
 call atomic_define(atom[1],0)
 sync all
 call ATOMIC_ADD (atom[1], me)
 if(me == 1) call atomic_ref(me,atom[1])
 sync all
 write(*,*) me

 end program


 Ok for trunk?


2014-09-15  Alessandro Fanfarillo  fanfarillo@gmail.com
Tobias Burnus  bur...@net-b.de

* trans-intrinsic.c (conv_intrinsic_atomic_op):
Check for indirect reference for caf_atomic_op value.

diff --git a/gcc/fortran/trans-intrinsic.c b/gcc/fortran/trans-intrinsic.c
index a13b113..2d7241a 100644
--- a/gcc/fortran/trans-intrinsic.c
+++ b/gcc/fortran/trans-intrinsic.c
@@ -8396,9 +8396,11 @@ conv_intrinsic_atomic_op (gfc_code *code)
   else
image_index = integer_zero_node;
 
-  if (TREE_TYPE (TREE_TYPE (atom)) != TREE_TYPE (TREE_TYPE (value)))
+   if (!POINTER_TYPE_P (TREE_TYPE (value)))
{
  tmp = gfc_create_var (TREE_TYPE (TREE_TYPE (atom)), value);
+ if (POINTER_TYPE_P (TREE_TYPE (value)))
+   value = build_fold_indirect_ref_loc (input_location, value);
  gfc_add_modify (block, tmp, fold_convert (TREE_TYPE (tmp), value));
   value = gfc_build_addr_expr (NULL_TREE, tmp);
}

Re: [PATCH, Fortran] Wrong invocation of caf_atomic_op

[Alessandro Fanfarillo]

In attachment a test case which fails with the current gcc-trunk
version but works when the patch is applied. coarray_35.f90 is my
attempt to write a gcc test case.
The problem is related with atomic_add.

2014-09-15 12:55 GMT-06:00 Tobias Burnus bur...@net-b.de:
 On 15.09.2014 19:18, Alessandro Fanfarillo wrote:

 New patch after the update.


 2014-09-09 0:30 GMT-06:00 Tobias Burnus bur...@net-b.de:

 I think I'd prefer the following patch, which avoids a temporary if none
 is
 required. value is a pointer if the kind is the same (see kind check
 before) and if it is not a literal. Otherwise, it isn't a pointer and one
 needs to generate a temporary.

 I do not quite understand why the current check doesn't work as both are
 integer(kind=4) but for some reasons one has a variant.

 Additionally, I wonder whether one should add a test case – one probably
 should do – and of which kind (run test + fdump-tree-original?).

 @@ -8398,3 +8398,3 @@ conv_intrinsic_atomic_op (gfc_code *code)
 - if (TREE_TYPE (TREE_TYPE (atom)) != TREE_TYPE (TREE_TYPE (value)))
 + if (!POINTER_TYPE_P (TREE_TYPE (value)))

 2014-09-15  Alessandro Fanfarillo  fanfarillo@gmail.com
   Tobias Burnus  bur...@net-b.de

   * trans-intrinsic.c (conv_intrinsic_atomic_op):
   Check for indirect reference for caf_atomic_op value.

 diff --git a/gcc/fortran/trans-intrinsic.c b/gcc/fortran/trans-intrinsic.c
 index a13b113..2d7241a 100644
 --- a/gcc/fortran/trans-intrinsic.c
 +++ b/gcc/fortran/trans-intrinsic.c
 @@ -8396,9 +8396,11 @@ conv_intrinsic_atomic_op (gfc_code *code)
else
   image_index = integer_zero_node;

 -  if (TREE_TYPE (TREE_TYPE (atom)) != TREE_TYPE (TREE_TYPE (value)))
 +   if (!POINTER_TYPE_P (TREE_TYPE (value)))
   {
 tmp = gfc_create_var (TREE_TYPE (TREE_TYPE (atom)), value);
 +   if (POINTER_TYPE_P (TREE_TYPE (value)))
 + value = build_fold_indirect_ref_loc (input_location, value);


 The second part makes no sense: If value is not a pointer (which is the
 first condition), it can never be a pointer (second condition).

 Otherwise, the patch is okay. The reason I hadn't committed it myself was
 that I wanted to include a test case; I was wondering whether it should be a
 run test – or a -fdump-tree-original + scan-tree test – or both.

 Can you create a test case?

 Tobias
! { dg-do compile }
! { dg-options -fcoarray=lib }
!
use iso_fortran_env
implicit none

integer :: me,np,res
integer(atomic_int_kind) :: atom[*]

me = this_image()
np = num_images()

call atomic_define(atom[1],0)

sync all

call ATOMIC_ADD (atom[1], me)

sync all

if(me == 1) then
  call atomic_ref(res,atom[1])
  if(res /= (np*(np+1))/2) then
{ dg-error Passing pointer address }
  endif
endif

end
program atomic
use iso_fortran_env
implicit none

integer :: me,np,res
integer(atomic_int_kind) :: atom[*]

me = this_image()
np = num_images()

call atomic_define(atom[1],0)

sync all

call ATOMIC_ADD (atom[1], me)

sync all

if(me == 1) then
  call atomic_ref(res,atom[1])
  if(res /= (np*(np+1))/2) then
write(*,*) 'res',res
call abort()
  endif
  write(*,*) 'OK'  
endif

end program

[PATCH, Fortran]

[Alessandro Fanfarillo]

Dear all,

the following code produces a wrong invocation to libcaf for
caf_atomic_op (atomic_add):

program atomic
use iso_fortran_env
implicit none

integer :: me
integer(atomic_int_kind) :: atom[*]
me = this_image()
call atomic_define(atom[1],0)
sync all
call ATOMIC_ADD (atom[1], me)
if(me == 1) call atomic_ref(me,atom[1])
sync all
write(*,*) me

end program

The compiler translates the caf_atomic_op call (related with atomic_add) as:

integer(kind=4) value.3;

value.3 = (integer(kind=4)) me;
_gfortran_caf_atomic_op (1, caf_token.0, 0, 1, value.3, 0B, 0B, 1, 4);

The attached patch seems to fix the problem.


Suggestions?
Index: gcc/fortran/trans-intrinsic.c
===
*** gcc/fortran/trans-intrinsic.c   (revision 215016)
--- gcc/fortran/trans-intrinsic.c   (working copy)
*** conv_intrinsic_atomic_op (gfc_code *code
*** 8396,8408 
else
image_index = integer_zero_node;
  
-   if (TREE_TYPE (TREE_TYPE (atom)) != TREE_TYPE (TREE_TYPE (value)))
-   {
- tmp = gfc_create_var (TREE_TYPE (TREE_TYPE (atom)), value);
- gfc_add_modify (block, tmp, fold_convert (TREE_TYPE (tmp), value));
-   value = gfc_build_addr_expr (NULL_TREE, tmp);
-   }
- 
gfc_get_caf_token_offset (token, offset, caf_decl, atom, atom_expr);
  
if (code-resolved_isym-id == GFC_ISYM_ATOMIC_DEF)
--- 8396,8401 

Re: [PATCH, Fortran]

[Alessandro Fanfarillo]

Thanks, your suggestion fixes the problem.

I just noticed that I missed the subject description; I'll send the
new patch in a different email.

2014-09-08 15:50 GMT-06:00 Tobias Burnus bur...@net-b.de:
 Alessandro Fanfarillo wrote:

 the following code produces a wrong invocation to libcaf for
 caf_atomic_op (atomic_add):

 program atomic
 use iso_fortran_env
 implicit none

 integer :: me
 integer(atomic_int_kind) :: atom[*]
 me = this_image()
 call atomic_define(atom[1],0)
 sync all
 call ATOMIC_ADD (atom[1], me)
 if(me == 1) call atomic_ref(me,atom[1])
 sync all
 write(*,*) me

 end program

 The compiler translates the caf_atomic_op call (related with atomic_add) as:

 integer(kind=4) value.3;

 value.3 = (integer(kind=4)) me;
 _gfortran_caf_atomic_op (1, caf_token.0, 0, 1, value.3, 0B, 0B, 1, 4);

 The attached patch seems to fix the problem.


 But I think it doesn't do the right thing if the kind is different.

 Suggestions?


 I think you want to do something like inserting

   if (POINTER_TYPE_P (TREE_TYPE (value))
 value = build_fold_indirect_ref_loc (input_location, value);

 before the assignment:

 gfc_add_modify (block, tmp, fold_convert (TREE_TYPE (tmp), value));

 otherwise you risk that you access invalid memory, e.g. for passing a
 integer(1) variable to an integer(4) atomic. On the other hand, there is
 something wrong with the check – as it shouldn't trigger when both atom and
 value have the same kind. Thus, the modified patch might work, is probably
 not completely clean either.

 Tobias

 Index: gcc/fortran/trans-intrinsic.c
 ===
 *** gcc/fortran/trans-intrinsic.c (revision 215016)
 --- gcc/fortran/trans-intrinsic.c (working copy)
 *** conv_intrinsic_atomic_op (gfc_code *code
 *** 8396,8408 
 else
   image_index = integer_zero_node;

 -   if (TREE_TYPE (TREE_TYPE (atom)) != TREE_TYPE (TREE_TYPE (value)))
 - {
 -   tmp = gfc_create_var (TREE_TYPE (TREE_TYPE (atom)), value);
 -   gfc_add_modify (block, tmp, fold_convert (TREE_TYPE (tmp), value));
 -   value = gfc_build_addr_expr (NULL_TREE, tmp);
 - }
 -
 gfc_get_caf_token_offset (token, offset, caf_decl, atom,
 atom_expr);

 if (code-resolved_isym-id == GFC_ISYM_ATOMIC_DEF)
 --- 8396,8401 

[PATCH, Fortran] Wrong invocation of caf_atomic_op

[Alessandro Fanfarillo]

This email follows the previous without subject (sorry about that).

The attached patch solves the problem raised by the following code:

program atomic
use iso_fortran_env
implicit none

integer :: me
integer(atomic_int_kind) :: atom[*]
me = this_image()
call atomic_define(atom[1],0)
sync all
call ATOMIC_ADD (atom[1], me)
if(me == 1) call atomic_ref(me,atom[1])
sync all
write(*,*) me

end program


Ok for trunk?
2014-09-08  Alessandro Fanfarillo  fanfarillo@gmail.com
Tobias Burnus  bur...@net-b.de

* trans-intrinsic.c (conv_intrinsic_atomic_op):
Check for indirect reference for caf_atomic_op value.

Index: gcc/fortran/trans-intrinsic.c
===
*** gcc/fortran/trans-intrinsic.c   (revision 215016)
--- gcc/fortran/trans-intrinsic.c   (working copy)
*** conv_intrinsic_atomic_op (gfc_code *code
*** 8397,8407 
image_index = integer_zero_node;
  
if (TREE_TYPE (TREE_TYPE (atom)) != TREE_TYPE (TREE_TYPE (value)))
!   {
! tmp = gfc_create_var (TREE_TYPE (TREE_TYPE (atom)), value);
! gfc_add_modify (block, tmp, fold_convert (TREE_TYPE (tmp), value));
!   value = gfc_build_addr_expr (NULL_TREE, tmp);
!   }
  
gfc_get_caf_token_offset (token, offset, caf_decl, atom, atom_expr);
  
--- 8397,8409 
image_index = integer_zero_node;
  
if (TREE_TYPE (TREE_TYPE (atom)) != TREE_TYPE (TREE_TYPE (value)))
!   {
! tmp = gfc_create_var (TREE_TYPE (TREE_TYPE (atom)), value);
! if (POINTER_TYPE_P (TREE_TYPE (value)))
!   value = build_fold_indirect_ref_loc (input_location, value);
! gfc_add_modify (block, tmp, fold_convert (TREE_TYPE (tmp), value));
! value = gfc_build_addr_expr (NULL_TREE, tmp);
!   }
  
gfc_get_caf_token_offset (token, offset, caf_decl, atom, atom_expr);
  

Re: [Patch, Fortran] -fcoarray=lib - support CRITICAL, prepare for locking support

[Alessandro Fanfarillo]

Hello,

I was implementing lock/unlock on the library side when I found a
possible problem in the patch:

if (is_lock_type == GFC_CAF_CRITICAL)
+reg_type = sym-attr.artificial ? GFC_CAF_CRITICAL : GFC_CAF_LOCK_STATIC;
+  else
+reg_type = GFC_CAF_COARRAY_STATIC;

the if statement cannot be true since is_lock_type is a boolean and
GFC_CAF_CRITICAL is 4.

Using if (is_lock_type) it produces the right result for the lock registration.


Regards

Alessandro

2014-07-28 14:37 GMT-06:00 Tobias Burnus bur...@net-b.de:
 This patch implements -fcoarray=lib support for CRITICAL blocks and includes
 some preparatory work for locking. In particular:

 * Updated the documentation for locking/critical, minor cleanup. The patch
 goes on top of the unreviewed patch
 https://gcc.gnu.org/ml/fortran/2014-07/msg00155.html
 * Add libcaf_single implementation for lock/unlock
 * Add lock/unlock calls for CRITICAL
 * Register static/SAVEd locking variables and locking variables for critical
 sections.

 Build and currently regtesting on x86-64-gnu-linux.
 OK when it regtested successfully?

  * * *

 Still to be done as follow up:
 * Handling the registering of lock-type components in statically allocated
 derived types
 * Handling the registering of lock-type variables and components with
 allocate and with implicit/explicit deallocate
 * Calling lock/unlock function for those
 * Test case for locking and critical blocks

 Other coarray to-do items:
 * Type-conversion test case missing
 * Vector subscript library implementation + test cases
 * Extending the documentation
 * Issues with striding for coarray components of derived types
 * Nonallocatable polymophic coarrays and select type/associated
 * Allocatable/pointer components of coarrays; co_reduce and co_broadcast

 Tobias

Re: [Fortran] PR37336 - FIINAL patch [1/n]: Implement the finalization wrapper subroutine

[Alessandro Fanfarillo]

Dear Tobias,

there are some problems with the final-wrapper-v2.diff patch; I get
the following error

final2.f90:71.15:

end module test
   1
Internal Error at (1):
gfc_code2string(): Bad code

for every test case that I use; in attachment final2.f90.

Regards

Alessandro

2012/8/19 Tobias Burnus bur...@net-b.de:
 Dear all,

 attached is a slightly updated patch:

 * Call finalizers of nonallocatable, nonpointer components
 * Generate FINAL wrapper for abstract types which have a finalizer. (The
 allocatable components are deallocated in the first type (abstract or not)
 which has a finalizer, i.e. abstract + finalizer or first nonabstract type.)

 I had to disable some resolve warning; I did so by introducing an
 attr.artificial. I used it to also fix PR 51632, where we errored out for
 __def_init and __copy where there were coarray components.

 Build and regtested on x86-64-linux.
 OK for the trunk?

 Tobias



--


final2.f90
Description: Binary data

Re: [Patch, fortran] PR46897 - [OOP] type-bound defined ASSIGNMENT(=) not used for derived type component in intrinsic assign

[Alessandro Fanfarillo]

Dear Paul,
Dear all,

I tried to compile the check_compiler_for_memory_leaks.F90 file
provided by Damian and it produces a segfault error. May be the
problem is related with add_comp_ref.

Regards

Alessandro (from Malta)

2012/8/14 Paul Richard Thomas paul.richard.tho...@gmail.com

 Dear Mikael,

  I think there are a couple of bugs not triggered by the single component
  types in the test. See below.

 Yes, you are right.  We should have tested multiple components... my
 fault!

  This could be moved to the only next caller (`previous' doesn't need to
  be updated if `this_code' is removed) to fix one usage of `this_code'
  :-).

 That's right... I'll make it so.

  ... but I have the feeling that this makes (*code) unreachable and that
  that's wrong. Shouldn't it be root-next = *code; ?

 No.  That caused the regression that I mentioned.  (*code) is
 resolved, at entry.  resolve_code steps on to (*code)-next.

  if we do it after the typebound calls, we overwrite their job so we have
  to do it before.

 This is what is done.

  However, if we do it before, we also overwrite components to be assigned
  with a typebound call, and this can have some side effects as the LHS's
  argument can be INTENT(INOUT).

 This might be so but it is what the standard dictates should
 happen isn't it?

 Thanks for the review.  I believe, in summary, that I should handle
 'this_code' consistently so that multiple component defined
 assignments work correctly.  I should also verify that pointers do
 what they are supposed to do, although it is rather trivial.

 Cheers

 Paul




--

Dott. Alessandro Fanfarillo
Verificatore Ellisse
Cell: 339/2428012
Email: alessandro.fanfari...@gmail.com

Re: [Fortran, DRAFT patch] PR 46321 - [OOP] Polymorphic deallocation

[Alessandro Fanfarillo]

Dear all,

in attachment the new draft which also supports the polymorphic
deallocation via INTENT(OUT). Tomorrow I'll try to realize a draft for
the deallocation at the end of the scope.

Regards

2012/6/12 Alessandro Fanfarillo fanfarillo@gmail.com:
 I don't know if there's already a PR but I get an ICE compiling this
 with a non-patched version. If x is not an array everything goes ok.

 2012/6/11 Tobias Burnus bur...@net-b.de:
 On 06/11/2012 11:24 AM, Alessandro Fanfarillo wrote:

 gfortran.dg/coarray/poly_run_3.f90


 That one fails because I for forgot that se.expr in gfc_trans_deallocate
 contains the descriptor and not the pointer to the data. That's fixed by:

          tmp = se.expr;
          if (GFC_DESCRIPTOR_TYPE_P (TREE_TYPE (tmp)))
            {
              tmp = gfc_conv_descriptor_data_get (tmp);
              STRIP_NOPS (tmp);

            }
          tmp =  fold_build2_loc (input_location, NE_EXPR, boolean_type_node,
                                  tmp, build_int_cst (TREE_TYPE (tmp), 0));

 However, it still fails for the

 type t
  integer, allocatable :: comp
 end type t
 contains
  subroutine foo(x)
    class(t), allocatable, intent(out) :: x(:)
  end subroutine
 end

 (The intent(out) causes automatic deallocation.) The backtrace does not
 really point to some code which the patch touched; it shouldn't be affected
 by the class.c changes and gfc_trans_deallocate does not seem to be entered.

 While I do not immediately see why it fails, I wonder whether it is due to
 the removed else if ... BT_CLASS) case in
 gfc_deallocate_scalar_with_status. In any case, the change to
 gfc_trans_deallocate might be also needed for
 gfc_deallocate_scalar_with_status. At least, automatic deallocation (with
 intent(out) or when leaving the scope) does not seem to go through
 gfc_trans_deallocate but only through gfc_deallocate_scalar_with_status.

 Tobias
Index: gcc/fortran/trans-decl.c
===
--- gcc/fortran/trans-decl.c(revisione 188511)
+++ gcc/fortran/trans-decl.c(copia locale)
@@ -3423,6 +3423,63 @@ init_intent_out_dt (gfc_symbol * proc_sym, gfc_wra
   gfc_init_block (init);
   for (f = proc_sym-formal; f; f = f-next)
 if (f-sym  f-sym-attr.intent == INTENT_OUT
+f-sym-ts.type == BT_CLASS
+!CLASS_DATA (f-sym)-attr.class_pointer
+CLASS_DATA (f-sym)-ts.u.derived-attr.alloc_comp)
+  {
+   gfc_expr *expr, *ppc;
+   gfc_se se, free_se;
+   gfc_code *ppc_code;
+   gfc_actual_arglist *actual;
+   tree cond;
+   f-sym-attr.referenced = 1;
+   expr = gfc_lval_expr_from_sym(f-sym);
+   gcc_assert (expr-expr_type == EXPR_VARIABLE);
+
+   if (expr-ts.type == BT_CLASS)
+ gfc_add_data_component (expr);
+
+   gfc_init_se (se, NULL);
+   gfc_start_block (se.pre);
+   se.want_pointer = 1;
+   se.descriptor_only = 1;
+   gfc_conv_expr (se, expr);
+   ppc = gfc_lval_expr_from_sym(f-sym);;
+   gfc_add_vptr_component (ppc);
+   gfc_add_component_ref (ppc, _free);
+   gfc_init_se (free_se, NULL);
+   free_se.want_pointer = 1;
+   gfc_conv_expr (free_se, ppc);
+   tmp = se.expr;
+   if (GFC_DESCRIPTOR_TYPE_P (TREE_TYPE (tmp)))
+ {
+   tmp = gfc_conv_descriptor_data_get (tmp);
+   STRIP_NOPS (tmp);
+ }
+   cond = fold_build2_loc (input_location, NE_EXPR, boolean_type_node,
+   free_se.expr,
+   build_int_cst (TREE_TYPE (free_se.expr), 0));
+   tmp =  fold_build2_loc (input_location, NE_EXPR, boolean_type_node,
+   tmp, build_int_cst (TREE_TYPE (tmp), 0));
+   cond = fold_build2_loc (input_location, TRUTH_AND_EXPR,
+   boolean_type_node, cond, tmp);
+
+   actual = gfc_get_actual_arglist ();
+   actual-expr = gfc_copy_expr (expr);
+
+   ppc_code = gfc_get_code ();
+   ppc_code-resolved_sym = ppc-symtree-n.sym;
+   ppc_code-resolved_sym-attr.elemental = 1;
+   ppc_code-ext.actual = actual;
+   ppc_code-expr1 = ppc;
+   ppc_code-op = EXEC_CALL;
+   tmp = gfc_trans_call (ppc_code, true, NULL, NULL, false);
+   tmp = fold_build3_loc (input_location, COND_EXPR, void_type_node,
+   cond, tmp, build_empty_stmt (input_location));
+gfc_add_expr_to_block (init, tmp);
+gfc_free_statements (ppc_code);
+  }
+else if (f-sym  f-sym-attr.intent == INTENT_OUT
 !f-sym-attr.pointer
 f-sym-ts.type == BT_DERIVED)
   {
@@ -3446,7 +3503,7 @@ init_intent_out_dt (gfc_symbol * proc_sym, gfc_wra
else if (f-sym-value)
  gfc_init_default_dt (f-sym, init, true);
   }
-else if (f-sym  f-sym-attr.intent == INTENT_OUT
+/*else if (f-sym  f-sym-attr.intent == INTENT_OUT
  f-sym-ts.type == BT_CLASS
  !CLASS_DATA (f-sym)-attr.class_pointer

Re: [Fortran, DRAFT patch] PR 46321 - [OOP] Polymorphic deallocation

[Alessandro Fanfarillo]

I don't know if there's already a PR but I get an ICE compiling this
with a non-patched version. If x is not an array everything goes ok.

2012/6/11 Tobias Burnus bur...@net-b.de:
 On 06/11/2012 11:24 AM, Alessandro Fanfarillo wrote:

 gfortran.dg/coarray/poly_run_3.f90


 That one fails because I for forgot that se.expr in gfc_trans_deallocate
 contains the descriptor and not the pointer to the data. That's fixed by:

          tmp = se.expr;
          if (GFC_DESCRIPTOR_TYPE_P (TREE_TYPE (tmp)))
            {
              tmp = gfc_conv_descriptor_data_get (tmp);
              STRIP_NOPS (tmp);

            }
          tmp =  fold_build2_loc (input_location, NE_EXPR, boolean_type_node,
                                  tmp, build_int_cst (TREE_TYPE (tmp), 0));

 However, it still fails for the

 type t
  integer, allocatable :: comp
 end type t
 contains
  subroutine foo(x)
    class(t), allocatable, intent(out) :: x(:)
  end subroutine
 end

 (The intent(out) causes automatic deallocation.) The backtrace does not
 really point to some code which the patch touched; it shouldn't be affected
 by the class.c changes and gfc_trans_deallocate does not seem to be entered.

 While I do not immediately see why it fails, I wonder whether it is due to
 the removed else if ... BT_CLASS) case in
 gfc_deallocate_scalar_with_status. In any case, the change to
 gfc_trans_deallocate might be also needed for
 gfc_deallocate_scalar_with_status. At least, automatic deallocation (with
 intent(out) or when leaving the scope) does not seem to go through
 gfc_trans_deallocate but only through gfc_deallocate_scalar_with_status.

 Tobias

Re: [Fortran, DRAFT patch] PR 46321 - [OOP] Polymorphic deallocation

[Alessandro Fanfarillo]

Thank you for the review, with this patch I get some ICEs during the
regstest with:

gfortran.dg/coarray/poly_run_3.f90
gfortran.dg/elemental_optional_args_5.f03
gfortran.dg/select_type_26.f03
gfortran.dg/select_type_27.f03
gfortran.dg/class_48.f90
gfortran.dg/class_allocate_10.f03
gfortran.dg/class_allocate_8.f03
gfortran.dg/class_array_1.f03
gfortran.dg/class_array_2.f03
gfortran.dg/assumed_type_2.f90
gfortran.dg/class_array_9.f03
gfortran.dg/coarray_lib_alloc_2.f90

I've debugged only the first 2 and the problem seems to be related
with tmp =  fold_build2_loc (input_location, NE_EXPR,
boolean_type_node, se.expr, build_int_cst (TREE_TYPE (se.expr), 0)); 
in trans-stmt.c at line 5376. The ICE message is the following:

$ gcc/bin/gfortran -c elemental_optional_args_5.f03
elemental_optional_args_5.f03: In function ‘MAIN__’:
elemental_optional_args_5.f03:220:0: internal compiler error: in
build_int_cst_wide, at tree.c:1219
 deallocate (taa, tpa, caa, cpa)
 ^
Please submit a full bug report,
with preprocessed source if appropriate.
See http://gcc.gnu.org/bugs.html for instructions.



2012/6/10 Tobias Burnus bur...@net-b.de:
 Alessandro Fanfarillo wrote:

 with the priceless support of Tobias I've almost realized the patch
 for this PR. In attachment there's the second draft. During the
 regression test I have only one error with select_type_4.f90. The
 problem is in the destroy_list subroutine when it checks
 associated(node) after the first deallocate(node).


 --- gcc/fortran/trans-stmt.c    (revisione 188002)
 +++ gcc/fortran/trans-stmt.c    (copia locale)
 @@ -5341,7 +5341,12 @@ gfc_trans_deallocate (gfc_code *code)
    for (al = code-ext.alloc.list; al != NULL; al = al-next)
     {
 -      gfc_expr *expr = gfc_copy_expr (al-expr);
 +      gfc_expr *expr;
 +      gfc_expr *ppc;
 +      gfc_code *ppc_code;
 +      gfc_actual_arglist *actual;
 +      expr = gfc_copy_expr (al-expr);
 +      ppc = gfc_copy_expr (expr);
 ...
 +      if (expr-symtree-n.sym-ts.type == BT_CLASS)


 I'd prefer:

 gfc_expr *ppc = NULL;
 ...
 if (expr-ts.type == BT_CLASS)
  ppc = gfc_copy_expr (expr);
 ...
 if (ppc)
  ...

 Namely: Only copy the expression if needed.

 Additionally, the check if (expr-symtree-n.sym-ts.type == BT_CLASS) is
 wrong. For instance, for
  type(t) :: x
  deallocate(x%class)
 it won't trigger, but it should.

 Actually, I think a cleaner version would be:

 if (al-expr-ts.type == BT_CLASS)
  {
    gfc_expr *ppc;
    ppc = gfc_copy_expr (al-expr);

  * * *

 Furthermore, I think you call _free + free for the same component for:

 type t
   integer, allocatable :: x
 end type t
 class(t), allocatable :: y
 ...
 deallocate (y)

 * * *

 Regarding your code: You assume that al-expr points to an allocated
 variable, that's not the always the case - hence, select_type_4.f90 fails.

 * * *

 You always create a _free function; I wonder whether it makes sense to use
 _vtab-free with NULL in case that no _free is needed.

  * * *

 Attached an updated version, which does that all. No guarantee that it works
 correctly, but it should at least fix select_type_4.f90.

 Tobias

Re: [Fortran, DRAFT patch] PR 46321 - [OOP] Polymorphic deallocation

[Alessandro Fanfarillo]

Hi all,
with the priceless support of Tobias I've almost realized the patch
for this PR. In attachment there's the second draft. During the
regression test I have only one error with select_type_4.f90. The
problem is in the destroy_list subroutine when it checks
associated(node) after the first deallocate(node).

2012/6/5 Paul Richard Thomas paul.richard.tho...@gmail.com:
 Hi Alessandro,

 I am glad to see that Janus is giving you a helping hand, in addition
 to Tobias.  I am so tied up with every aspect of life that gfortran is
 not figuring much at all.

 When you clean up the patch, you might consider making this into a
 separate function:

 +         if (free_proc)
 +           {
 +             ppc = gfc_copy_expr(free_proc-initializer);
 +             ppc_code = gfc_get_code ();
 +             ppc_code-resolved_sym = ppc-symtree-n.sym;
 +             ppc_code-resolved_sym-attr.elemental = 1;
 +             ppc_code-ext.actual = actual;
 +             ppc_code-expr1 = ppc;
 +             ppc_code-op = EXEC_CALL;
 +             tmp = gfc_trans_call (ppc_code, true, NULL, NULL, false);
 +             gfc_free_statements (ppc_code);
 +             gfc_add_expr_to_block (block, tmp);
 +           }

 ... and using the function call to replace the corresponding call to
 _copy in trans_allocate.

 I suspect that we are going to do this some more :-)

 Once we have the separate function, we could at later stage replace it
 by a TREE_SSA version.

 Cheers

 Paul

 On 3 June 2012 12:15, Alessandro Fanfarillo fanfarillo@gmail.com wrote:
 Right, the problem is that the _free component is missing. Just as the
 _copy component, _free should be present for *every* vtype, no matter
 if there are allocatable components or not. If the _free component is
 not needed, it should be initialized to EXPR_NULL.

 With an empty _free function for every type which does not have
 allocatable components the problem with dynamic_dispatch_4.f03
 disappears :), thank you very much. In the afternoon I'll reorganize
 the code.

 Bye.

 Alessandro



 --
 The knack of flying is learning how to throw yourself at the ground and miss.
        --Hitchhikers Guide to the Galaxy
Index: gcc/testsuite/gfortran.dg/class_19.f03
===
--- gcc/testsuite/gfortran.dg/class_19.f03  (revisione 188002)
+++ gcc/testsuite/gfortran.dg/class_19.f03  (copia locale)
@@ -39,5 +39,5 @@ program main
 
 end program main
 
-! { dg-final { scan-tree-dump-times __builtin_free 11 original } }
+! { dg-final { scan-tree-dump-times __builtin_free 14 original } }
 ! { dg-final { cleanup-tree-dump original } }
Index: gcc/testsuite/gfortran.dg/auto_dealloc_2.f90
===
--- gcc/testsuite/gfortran.dg/auto_dealloc_2.f90(revisione 188002)
+++ gcc/testsuite/gfortran.dg/auto_dealloc_2.f90(copia locale)
@@ -25,5 +25,5 @@ contains
 
 end program 
 
-! { dg-final { scan-tree-dump-times __builtin_free 3 original } }
+! { dg-final { scan-tree-dump-times __builtin_free 4 original } }
 ! { dg-final { cleanup-tree-dump original } }
Index: gcc/fortran/trans-stmt.c
===
--- gcc/fortran/trans-stmt.c(revisione 188002)
+++ gcc/fortran/trans-stmt.c(copia locale)
@@ -5341,7 +5341,12 @@ gfc_trans_deallocate (gfc_code *code)
 
   for (al = code-ext.alloc.list; al != NULL; al = al-next)
 {
-  gfc_expr *expr = gfc_copy_expr (al-expr);
+  gfc_expr *expr;
+  gfc_expr *ppc;
+  gfc_code *ppc_code;
+  gfc_actual_arglist *actual;
+  expr = gfc_copy_expr (al-expr);
+  ppc = gfc_copy_expr (expr);
   gcc_assert (expr-expr_type == EXPR_VARIABLE);
 
   if (expr-ts.type == BT_CLASS)
@@ -5354,6 +5359,24 @@ gfc_trans_deallocate (gfc_code *code)
   se.descriptor_only = 1;
   gfc_conv_expr (se, expr);
 
+  actual = gfc_get_actual_arglist ();
+  actual-expr = gfc_copy_expr (expr);
+
+  if (expr-symtree-n.sym-ts.type == BT_CLASS)
+   {
+ gfc_add_vptr_component (ppc);
+ gfc_add_component_ref (ppc, _free);
+ ppc_code = gfc_get_code ();
+ ppc_code-resolved_sym = ppc-symtree-n.sym;
+ ppc_code-resolved_sym-attr.elemental = 1;
+ ppc_code-ext.actual = actual;
+ ppc_code-expr1 = ppc;
+ ppc_code-op = EXEC_CALL;
+ tmp = gfc_trans_call (ppc_code, true, NULL, NULL, false);
+ gfc_free_statements (ppc_code);
+ gfc_add_expr_to_block (block, tmp);
+   }
+
   if (expr-rank || gfc_is_coarray (expr))
{
  if (expr-ts.type == BT_DERIVED  
expr-ts.u.derived-attr.alloc_comp)
Index: gcc/fortran/class.c
===
--- gcc/fortran/class.c (revisione 188002)
+++ gcc/fortran/class.c (copia locale)
@@ -717,6 +717,9 @@ gfc_find_derived_vtab (gfc_symbol *derived)
   gfc_namespace *ns

Re: [Fortran, patch] PR 48831 - Constant expression (PARAMETER array element) rejected as nonconstant

[Alessandro Fanfarillo]

Hi all,

in attachment the patch which includes the review comments provided by Tobias.

The patch is bootstrapped and tested on x86_64-unknown-linux-gnu.

Regards.

Alessandro

2012/5/20 Tobias Burnus bur...@net-b.de:
 Hi Alessandro,


 Alessandro Fanfarillo wrote:

 in attachment there's a patch for PR 48831, it also includes a new
 test case suggested by Tobias Burnus.
 The patch is bootstrapped and tested on x86_64-unknown-linux-gnu.


 Please try to ensure that your patch has a text mime type - it shows up as
  Content-Type: application/octet-stream;
 which makes reading, reviewing and quoting your patch more difficult.

        PR fortran/48831
        * gfortran.h: Add non-static prototype declaration
        of check_init_expr function.
        * check.c (kind_check): Change if condition related
        to check_init_expr.
        * expr.c: Remove prototype declaration of
        check_init_expr function and static keyword.


 You should add the name of the function you change in parentheses, e.g.

    * gfortran.h (check_init_expr): Add prototype declaration of function.

 (The non-static is superfluous as static functions shouldn't be in header
 files.) For check_init_expr I'd use Remove forward declaration instead
 of Remove prototype declaration but that's personal style. But again, you
 should include the function name in parentheses. The reason is that one can
 more quickly find it, if it is always at the same spot.

 As mentioned before, the gfortran convention is to prefix functions (gfc_) -
 at least those which are nonstatic. Please change the function name.

 -  if (k-expr_type != EXPR_CONSTANT)
 +  if (check_init_expr(k) != SUCCESS)


 GNU style: Add a space before the ( of the function argument:
 check_init_expr (k).


 +/* Check an intrinsic arithmetic operation to see if it is consistent
 +   with some type of expression.  */
 +gfc_try check_init_expr (gfc_expr *);



 I have to admit that after reading only the comment, I had no idea what the
 function does - especially the some type is not really helpful. How about
 a simple Check whether an expression is an initialization/constant
 expression.  Initialization and constant expressions are well defined in
 the Fortran standard. (Actually, I find the function name speaks already for
 itself, thus, I do not see the need for a comment, but I also do not mind a
 comment.)

 (One problem with the name constant expression vs. initialization
 expression is that Fortran 90/95 distinguish between them while Fortran
 2003/2008 have merged them to a single type of expression; Fortran 2003
 calls the merged expression type initialization expression while Fortran
 2008 calls them constant expressions. In principle, gfortran should make
 the distinction with -std=f95 and reject expressions which are nonconstant
 and only initexpressions when the standard demands it, but I am not sure
 whether gfortran does. That part of gfortran is a bit unclean and the
 distinction between init/const expr is nowadays largely ignored by the
 gfortran developers.)

 Otherwise, the patch looks OK.

 Tobias
2012-06-03  Alessandro Fanfarillo  fanfarillo@gmail.com
Tobias Burnus  bur...@net-b.de

PR fortran/48831
* gfortran.h (check_init_expr): Add prototype declaration 
of function.
* check.c (kind_check): Change if condition related
to check_init_expr.
* expr.c (check_init_expr): Remove forward declaration
and static keyword. Change name in gfc_check_init_expr.

2012-06-03  Alessandro Fanfarillo  fanfarillo@gmail.com

PR fortran/48831
* gfortran.dg/parameter_array_element_2.f90: New.

Index: gcc/fortran/expr.c
===
--- gcc/fortran/expr.c  (revisione 188147)
+++ gcc/fortran/expr.c  (copia locale)
@@ -1943,12 +1943,6 @@
 }
 
 
-/* Check an intrinsic arithmetic operation to see if it is consistent
-   with some type of expression.  */
-
-static gfc_try check_init_expr (gfc_expr *);
-
-
 /* Scalarize an expression for an elemental intrinsic call.  */
 
 static gfc_try
@@ -1994,7 +1988,7 @@
   for (; a; a = a-next)
 {
   /* Check that this is OK for an initialization expression.  */
-  if (a-expr  check_init_expr (a-expr) == FAILURE)
+  if (a-expr  gfc_check_init_expr (a-expr) == FAILURE)
goto cleanup;
 
   rank[n] = 0;
@@ -2231,7 +2225,7 @@
   gfc_actual_arglist *ap;
 
   for (ap = e-value.function.actual; ap; ap = ap-next)
-if (check_init_expr (ap-expr) == FAILURE)
+if (gfc_check_init_expr (ap-expr) == FAILURE)
   return MATCH_ERROR;
 
   return MATCH_YES;
@@ -2319,7 +2313,7 @@
ap-expr-where);
  return MATCH_ERROR;
  }
-   else if (not_restricted  check_init_expr (ap-expr) == FAILURE)
+   else if (not_restricted  gfc_check_init_expr (ap-expr) == FAILURE)
  return MATCH_ERROR;
 
if (not_restricted == 0

Re: [Fortran, DRAFT patch] PR 46321 - [OOP] Polymorphic deallocation

[Alessandro Fanfarillo]

 Right, the problem is that the _free component is missing. Just as the
 _copy component, _free should be present for *every* vtype, no matter
 if there are allocatable components or not. If the _free component is
 not needed, it should be initialized to EXPR_NULL.

With an empty _free function for every type which does not have
allocatable components the problem with dynamic_dispatch_4.f03
disappears :), thank you very much. In the afternoon I'll reorganize
the code.

Bye.

Alessandro

Re: [Fortran, patch] PR 48831 - Constant expression (PARAMETER array element) rejected as nonconstant

[Alessandro Fanfarillo]

Thank you Tobias, I thought that Change name in gfc_check_init_expr
was sufficient.

2012/6/3 Tobias Burnus bur...@net-b.de:
 Hi Alessandro, hi all,


 Alessandro Fanfarillo wrote:

 in attachment the patch which includes the review comments provided by
 Tobias.


 Thanks for the patch, which I committed as Rev. 188152. Congratulation to
 your second committed patch.

 Nit: You forgot twice to add the prefix gfc_ in the ChangeLog; I corrected
 it before committal.

  * * *

 If possible, use -p when you do a diff. With svn, simply pass -x -p (or
 --diff-cmd=diff -x '-p -u'); git does this already by default. [Some prefer
 -c to -u, which is also fine.]  Without the -p flag, the result is:

 --- gcc/fortran/check.c (revisione 188147)
 +++ gcc/fortran/check.c
 @@ -163,7 +163,7 @@
   if (scalar_check (k, n) == FAILURE)


 While with -p flag, one gets:

 --- gcc/fortran/check.c    (Revision 188123)
 +++ gcc/fortran/check.c
 @@ -163,7 +163,7 @@ kind_check (gfc_expr *k, int n, bt type)
   if (scalar_check (k, n) == FAILURE)


 The difference is that the @@ line shows the function name (here:
 kind_check). That information makes it easier to review a patch as one
 then knows more about the context.

 Tobias

[Fortran, DRAFT patch] PR 46321 - [OOP] Polymorphic deallocation

[Alessandro Fanfarillo]

Dear all,

I have realized a draft patch for the PR 46321, currently it works
only with the explicit DEALLOCATE.

Running the regression tests it doesn't pass the following:

- gfortran.dg/class_19.f03 (too much __builtin_free)
- gfortran.dg/auto_dealloc_2.f90 (too much __builtin_free)
- gfortran.dg/dynamic_dispatch_4.f03 (free on invalid pointer)
- gfortran.dg/typebound_operator_9.f03 (fails during the execution test)

The first two tests fail due to the introduction of __builtin_free
in the freeing functions, so it is not a problem.

The gfortran.dg/dynamic_dispatch_4.f03 had this problem in the past
(http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43986); currently it
calls the __free_s_bar_mod_S_bar function instead of the proper
doit().

Regarding typebound_operator_9.f03, I don't know how to fix the patch...

The patch is written in a raw way due to my newbieness, so any
suggestion is well accepted.

Regards.

Alessandro
Index: gcc/fortran/class.c
===
--- gcc/fortran/class.c (revisione 188002)
+++ gcc/fortran/class.c (copia locale)
@@ -717,6 +717,7 @@
   gfc_namespace *ns;
   gfc_symbol *vtab = NULL, *vtype = NULL, *found_sym = NULL, *def_init = NULL;
   gfc_symbol *copy = NULL, *src = NULL, *dst = NULL;
+  gfc_symbol *free = NULL, *tofree = NULL;
 
   /* Find the top-level namespace (MODULE or PROGRAM).  */
   for (ns = gfc_current_ns; ns; ns = ns-parent)
@@ -907,6 +908,119 @@
  c-ts.interface = copy;
}
 
+ /* Add component _free.  */
+ gfc_component *temp = NULL;
+ bool der_comp_alloc = false, comp_alloc = false;
+ bool  class_comp_alloc = false;
+ for (temp = derived-components; temp; temp = temp-next)
+   {
+ if (temp == derived-components  derived-attr.extension)
+   continue;
+
+ if (temp-ts.type == BT_DERIVED
+  !temp-attr.pointer
+  (temp-attr.alloc_comp || temp-attr.allocatable))
+   der_comp_alloc = true;
+ else if (temp-ts.type != BT_DERIVED
+   !temp-attr.pointer
+   (temp-attr.alloc_comp
+  || temp-attr.allocatable))
+   comp_alloc = true;
+ else if (temp-ts.u.derived
+   temp-ts.type == BT_CLASS
+   CLASS_DATA (temp)
+  // (CLASS_DATA (temp)-attr.class_pointer
+  //|| CLASS_DATA (temp)-attr.allocatable))
+   CLASS_DATA (temp)-attr.allocatable)
+   class_comp_alloc = true;
+   }
+ if (derived-attr.extension
+  (!der_comp_alloc  !comp_alloc  !class_comp_alloc))
+   {
+ gfc_component *parent = derived-components;
+ gfc_component *free_proc = NULL;
+ gfc_symbol *vtab2 = NULL;
+ gfc_expr *tmp1 = NULL, *tmp2 = NULL;
+ vtab2 = gfc_find_derived_vtab (parent-ts.u.derived);
+
+ for (free_proc = vtab2-ts.u.derived-components;
+  free_proc; free_proc = free_proc-next)
+   if (free_proc-name[0] == '_'
+free_proc-name[1] == 'f')
+ break;
+
+ if (!free_proc)
+   goto end_vtab;
+
+ if (gfc_add_component (vtype, _free, c) == FAILURE)
+   goto cleanup;
+ c-attr.proc_pointer = 1;
+ c-attr.access = ACCESS_PRIVATE;
+ c-tb = XCNEW (gfc_typebound_proc);
+ c-tb-ppc = 1;
+ /* Not sure about this part */
+ tmp1 = gfc_lval_expr_from_sym (free_proc-ts.interface);
+ tmp2 = gfc_copy_expr (tmp1);
+ c-initializer = tmp2;
+ c-ts.interface = tmp2-symtree-n.sym;
+ goto end_vtab;
+
+   }
+
+ if (derived-attr.alloc_comp || der_comp_alloc
+ || class_comp_alloc)
+   {
+ gfc_alloc *head = NULL;
+ if (gfc_add_component (vtype, _free, c) == FAILURE)
+   goto cleanup;
+ c-attr.proc_pointer = 1;
+ c-attr.access = ACCESS_PRIVATE;
+ c-tb = XCNEW (gfc_typebound_proc);
+ c-tb-ppc = 1;
+ if (derived-attr.abstract)
+   c-initializer = gfc_get_null_expr (NULL);
+ else
+   {
+ /* Set up namespace.  */
+ gfc_namespace *sub_ns2 = gfc_get_namespace (ns, 0);
+ sub_ns2-sibling = ns-contained;
+ ns-contained = sub_ns2;
+ sub_ns2-resolved = 1;
+ 

Re: [Fortran, DRAFT patch] PR 46321 - [OOP] Polymorphic deallocation

[Alessandro Fanfarillo]

Hi Janus,

 Sorry, I don't understand the last sentence. Why should it call some
 __free... instead of doit? And why is that test case even affected
 by your patch (you said it would only work with explicit DEALLOCATE,
 which does not appear in that test case)?

Yes, it is as I said... In
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43986#c4 the doit() call
produces a segfault because r26 is 0 instead of the address of
__s_bar_mod_MOD_doit. With my patched version, the doit call is in
reality a _free __free_s_bar_mod_S_bar call. To better understand I
report a little portion (only the MAIN__) of the fdump-tree-original
and the testcase execution (hoping that it will be understandable...)

MAIN__ ()
{
  struct __class_foo_mod_Foo_p a;
  struct foo b;
  static struct s_bar c = {};
  static struct a_bar d = {};

  try
{
  c.a = 0B;
  d.a.data = 0B;
  (struct __vtype_foo_mod_Foo *) a._vptr = __vtab_foo_mod_Foo;
  a._data = b;
  a._vptr-doit (a);
  if (a._vptr-getit (a) != 1)
{
  _gfortran_abort ();
}
  L.1:;
  (struct __vtype_foo_mod_Foo *) a._vptr = (struct
__vtype_foo_mod_Foo *) __vtab_s_bar_mod_S_bar;
  a._data = (struct foo *) c;
  a._vptr-doit (a); IT REALLY WANTS TO CALL THE DOIT FUNCTION!
  if (a._vptr-getit (a) != 2)
{
  _gfortran_abort ();
}
  L.2:;
  (struct __vtype_foo_mod_Foo *) a._vptr = (struct
__vtype_foo_mod_Foo *) __vtab_a_bar_mod_A_bar;
  a._data = (struct foo *) d;
  a._vptr-doit (a);
  if (a._vptr-getit (a) != 3)
{
  _gfortran_abort ();
}
  L.3:;
}
  finally
{
  if (d.a.data != 0B)
{
  __builtin_free ((void *) d.a.data);
}
  d.a.data = 0B;
  if (c.a != 0B)
{
  __builtin_free ((void *) c.a);
}
  c.a = 0B;
}
}

An now the testcase execution with gdb:

Breakpoint 1, MAIN__ () at dynamic_dispatch_4.f03:82
82type(s_bar), target :: c
(gdb) next
83type(a_bar), target :: d
(gdb)
85a = b
(gdb)
86call a%doit
(gdb)
87if (a%getit () .ne. 1) call abort
(gdb)
88a = c
(gdb) step
89call a%doit
(gdb)
s_bar_mod::__free_s_bar_mod_S_bar (tofree=...) at dynamic_dispatch_4.f03:43
43  class(s_bar) :: a

I don't know if I got it across...

 The patch actually gives a few warnings:

Ok, thanks. I always use bootstrap and it works but I never look at
the compile result (unless it doesn't compile...)

Re: [Fortran, patch] PR 52158 - Regression on character function with gfortran 4.7

[Alessandro Fanfarillo]

2012/5/13 Tobias Burnus bur...@net-b.de:


 Committed as Rev. 187436. Thanks for the patch and congratulation to your
 first GCC contribution!


Thank you for your priceless support!


Alessandro

[Fortran, patch] PR 48831 - Constant expression (PARAMETER array element) rejected as nonconstant

[Alessandro Fanfarillo]

Hi All,

in attachment there's a patch for PR 48831, it also includes a new
test case suggested by Tobias Burnus.

The patch is bootstrapped and tested on x86_64-unknown-linux-gnu.

Regards.

Alessandro Fanfarillo


patch.diff
Description: Binary data

Re: [Fortran, patch] PR 52158 - Regression on character function with gfortran 4.7

[Alessandro Fanfarillo]

Hello all,

in attachment there's the new candidate patch, revisited by Tobias,
for PR fortran/45170, PR fortran/52158 and PR fortran/49430
(unexpectedly).

Please, check the Changelog, I don't know whether the descriptions are ok.

The patch is bootstrapped and tested on x86_64-unknown-linux-gnu.

gcc version 4.8.0 20120512 (experimental).


Have a nice weekend.

Alessandro.


2012/5/7 Tobias Burnus bur...@net-b.de:
 Hello,


 On 05/06/2012 09:37 PM, Alessandro Fanfarillo wrote:

 The patch is bootstrapped and tested on x86_64-unknown-linux-gnu - gcc
 version 4.8.0 20120506 (experimental)


 2012-05-06  Alessandro Fanfarillofanfarillo@gmail.com
             Paul Thomaspa...@gcc.gnu.org
             Tobias Burnusbur...@net-b.de

         PR fortran/52158
         * resolve.c (resolve_fl_derived0): Add a new condition in the if
         statement of the deferred-length character component error block.
         * trans-expr (gfc_conv_procedure_call): Add new checks in the if
         statement on component's attributes (regarding PR 45170).


 Typo: trans-expr - trans-expr.c.

 Well, either the second PR is related to the commit - then one should have
    PR fortran/52158
    PR fortran/45170
 - or it is only that vaguely related that one should only mention it in the
 patch submittal email.

 As it fixes an ICE mentioned in PR 45170, I would add it. (I think one
 should close that PR, create PRs for the remaining issues and possibly a
 meta bug for tracking. That PR is really a mess.)

 The description Add new checks in the if statement on component's
 attributes is true but it makes the implicit assumption that the reader
 knows that the commit is about ts.deferred. I had written something like
 the following:

        * resolve.c (resolve_fl_derived0): Allow TBPs with deferred-length
 results.
        * trans-expr (gfc_conv_procedure_call): Handle TBP with
 deferred-length results.


 I am sure one can write a nicer ChangeLog text, but at least it points to
 TBP (type-bound procedures) and to functions which have
 deferred-length-character result variable. (Actually, procedure-pointer
 components are also affected.)


 2012-05-06  Alessandro Fanfarillofanfarillo@gmail.com
                   Damian Rousondam...@rouson.net

        PR fortran/45170


 Typically, the bug reporters are only acknowledged via the PR itself (and
 sometimes via a comment in the test case). That should be sufficient.

 Additionally, you should quote the same PRs as in the actual patch
 (fortran/ChangeLog). The test case tests that PR 52158 is fixed - and it
 tests that the bug reported in comment 19 of PR 45170 is solved.

 -         if (ts.deferred  (sym-attr.allocatable || sym-attr.pointer))
 +         if (ts.deferred  ((!comp  (sym-attr.allocatable
 +              || sym-attr.pointer)) || (comp  (comp-attr.allocatable
 +              || comp-attr.pointer


 The spacing is wrong: You have a 14   before the || but you should have
 1 tab followed by 6 spaces. (I don't think that this is a problem of the
 email client as the if line still have a tab.)

 Additionally, the line breaks have been wrongly placed; it should be:

 +         if (ts.deferred
 +  ((!comp  (sym-attr.allocatable || sym-attr.pointer))
 +                 || (comp  (comp-attr.allocatable ||
 comp-attr.pointer


 That way one sees more easily which belongs together. The  is below
 ts; the || is one to the right of the ( to which this part of the
 expression belongs.


 --- gcc-4.8/gcc/testsuite/gfortran.dg/deferred_type_param_3.f90 1970-01-01
 01:00:00.0 +0100
 +++ gcc-4.8-patched/gcc/testsuite/gfortran.dg/deferred_type_param_3.f90
 2012-05-06
 19:26:29.498829273 +0200
 @@ -0,0 +1,21 @@
 +! { dg-do compile }
 +!
 +! PR fortran/45170


 As above: You should also list PR fortran/52158.

 As I realized when looking at the ChangeLog: Proc pointers are also
 affected. I think one could append the following code to the test case,
 which gives the same error without the patch. (Untested with the patch.)

 module test
  implicit none
  type t
    procedure(deferred_len), pointer, nopass :: ppt
  end type t
 contains
  function deferred_len()
    character(len=:), allocatable :: deferred_len
    deferred_len = 'abc'
  end function deferred_len
  subroutine doIt()
    type(t) :: x
    x%ppt = deferred_len
    if (abc /= x%ppt()) call abort ()
  end subroutine doIt
 end module test

 use test
 call doIt ()
 end


 Otherwise, the patch looks OK.

 Thanks for the contribution - and congratulation to your first GCC patch.

 Tobias

 PS: For bean counters: There is a GCC copyright assignment entry for
 Alessandro since 2012-04-18.


deferred_last.diff
Description: Binary data

[Fortran, patch] PR 52158 - Regression on character function with gfortran 4.7

[Alessandro Fanfarillo]

Hello,

my name is Alessandro, I'm a newbie of GCC and helped by Tobias Burnus
and Paul Thomas I'll try to add support for final subroutines.

The patch is bootstrapped and tested on x86_64-unknown-linux-gnu - gcc
version 4.8.0 20120506 (experimental)

Best regards.


gcc/fortran/ChangeLog

2012-05-06  Alessandro Fanfarillo  fanfarillo@gmail.com
Paul Thomas  pa...@gcc.gnu.org
Tobias Burnus  bur...@net-b.de

PR fortran/52158
* resolve.c (resolve_fl_derived0): Add a new condition in the if
statement of the deferred-length character component error block.
* trans-expr (gfc_conv_procedure_call): Add new checks in the if
statement on component's attributes (regarding PR 45170).

gcc/testsuite/ChangeLog

2012-05-06  Alessandro Fanfarillo  fanfarillo@gmail.com
      Damian Rouson  dam...@rouson.net

    PR fortran/45170
    * gfortran.dg/deferred_type_param_3.f90: New.

Patch.diff

--- gcc-4.8/gcc/fortran/resolve.c   2012-05-06 19:29:21.794825508 +0200
+++ gcc-4.8-patched/gcc/fortran/resolve.c   2012-05-06 19:24:40.770831649 
+0200
@@ -11666,7 +11666,7 @@
   for ( ; c != NULL; c = c-next)
 {
   /* See PRs 51550, 47545, 48654, 49050, 51075 - and 45170.  */
-  if (c-ts.type == BT_CHARACTER  c-ts.deferred)
+  if (c-ts.type == BT_CHARACTER  c-ts.deferred  !c-attr.function)
{
  gfc_error (Deferred-length character component '%s' at %L is not 
 yet supported, c-name, c-loc);
diff -urN gcc-4.8/gcc/fortran/trans-expr.c
gcc-4.8-patched/gcc/fortran/trans-expr.c
--- gcc-4.8/gcc/fortran/trans-expr.c2012-05-06 19:29:21.878825505 +0200
+++ gcc-4.8-patched/gcc/fortran/trans-expr.c2012-05-06 19:25:53.134830069 
+0200
@@ -4175,7 +4175,9 @@
 we take the character length of the first argument for the result.
 For dummies, we have to look through the formal argument list for
 this function and use the character length found there.*/
- if (ts.deferred  (sym-attr.allocatable || sym-attr.pointer))
+ if (ts.deferred  ((!comp  (sym-attr.allocatable
+  || sym-attr.pointer)) || (comp  (comp-attr.allocatable
+  || comp-attr.pointer
cl.backend_decl = gfc_create_var (gfc_charlen_type_node, slen);
  else if (!sym-attr.dummy)
cl.backend_decl = VEC_index (tree, stringargs, 0);
diff -urN gcc-4.8/gcc/testsuite/gfortran.dg/deferred_type_param_3.f90
gcc-4.8-patched/gcc/testsuite/gfortran.dg/deferred_type_param_3.f90
--- gcc-4.8/gcc/testsuite/gfortran.dg/deferred_type_param_3.f90 1970-01-01
01:00:00.0 +0100
+++ gcc-4.8-patched/gcc/testsuite/gfortran.dg/deferred_type_param_3.f90 
2012-05-06
19:26:29.498829273 +0200
@@ -0,0 +1,21 @@
+! { dg-do compile }
+!
+! PR fortran/45170
+!
+! Contributed by Damian Rouson
+
+module speaker_class
+  type speaker
+  contains
+procedure :: speak
+  end type
+contains
+  function speak(this)
+class(speaker) ,intent(in) :: this
+character(:) ,allocatable :: speak
+  end function
+  subroutine say_something(somebody)
+class(speaker) :: somebody
+print *,somebody%speak()
+  end subroutine
+end module