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

[Alessandro Fanfarillo]

* PING *

2016-09-21 19:03 GMT+01:00 Alessandro Fanfarillo :
> Thanks Andre.
>
> 2016-09-19 9:55 GMT-06:00 Andre Vehreschild :
>> 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  wrote:
>>
>>> * PING *
>>>
>>> On Sep 7, 2016 3:01 PM, "Alessandro Fanfarillo" 
>>> 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
>>> > >  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 :
>>> >  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;
>>> > >>> +
>>> > 

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 :
> 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  wrote:
>
>> * PING *
>>
>> On Sep 7, 2016 3:01 PM, "Alessandro Fanfarillo" 
>> 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
>> > >  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 :
>> >  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 == 

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

[Andre Vehreschild]

Hi Alessandro,

there are still some violations of the style guide:

contrib/check_GNU_style.sh first_complete_patch_REV2.diff

emits:

Lines should not exceed 80 characters.
154:+  add_sym_2 ("failed_images", GFC_ISYM_FAILED_IMAGES,
CLASS_TRANSFORMATIONAL, ACTUAL_NO, BT_INTEGER, 155:+ dd,
GFC_STD_F2008_TS, gfc_check_failed_images, gfc_simplify_failed_images,
156:+ gfc_resolve_failed_images, "team", BT_INTEGER, di, OPTIONAL,
"kind", BT_INTEGER, di, OPTIONAL); 165:+  add_sym_2 ("image_status",
GFC_ISYM_IMAGE_STATUS, CLASS_ELEMENTAL, ACTUAL_NO, BT_INTEGER,
166:+ di, GFC_STD_F2008_TS, gfc_check_image_status,
gfc_simplify_image_status, 167:+ gfc_resolve_image_status, "image",
BT_INTEGER, di, REQUIRED, "team", BT_INTEGER, di, OPTIONAL);
247:+gfc_resolve_image_status (gfc_expr *f, gfc_expr *image, gfc_expr *team
ATTRIBUTE_UNUSED) 409:+  result = transformational_result (result, 0,
BT_INTEGER, kind->ts.kind, _current_locus);
420:+gfc_simplify_image_status(gfc_expr *image ATTRIBUTE_UNUSED, gfc_expr *team
ATTRIBUTE_UNUSED) 469:+  gfor_fndecl_caf_failed_images =
gfc_build_library_function_decl_with_spec ( 471:+pvoid_type_node, 3,
pvoid_type_node, integer_type_node, integer_type_node); 

 The remainder of the script output needs no fix, because its in Fortran
code.

You should fix the above, where they are not in a Fortran testcases. This
allows people with 80 column terminals to read the whole line without scrolling.

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.

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

Regards,
Andre

On Mon, 19 Sep 2016 08:30:12 -0700
Alessandro Fanfarillo  wrote:

> * PING *
> 
> On Sep 7, 2016 3:01 PM, "Alessandro Fanfarillo" 
> 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
> > >  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 :
> >  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")) */
> > >>> +  

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 :
> 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
>  wrote:
>> * PING *
>>
>> 2016-07-21 13:05 GMT-06:00 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 :
 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
commit 13213642603b4941a2e4ea085b0bfd5cb37f
Author: Alessandro Fanfarillo 
Date:   Wed Sep 7 13:00:17 2016 -0600

Second Review of failed image patch

diff --git a/gcc/fortran/check.c b/gcc/fortran/check.c
index ff5e80b..110bec0 100644
--- a/gcc/fortran/check.c
+++ b/gcc/fortran/check.c
@@ -1217,6 +1217,82 @@ 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 

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 :
> 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
>  wrote:
>> * PING *
>>
>> 2016-07-21 13:05 GMT-06:00 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 :
 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)

[Paul Richard Thomas]

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
 wrote:
> * PING *
>
> 2016-07-21 13:05 GMT-06:00 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 :
>>> 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)

[Dan Nagle]

Hi,

The failed images features of gfortran are exciting,
and folks here would like to start testing with their scientific codes.
I’d like to build a new gfortran to support them,
but I must build from a trusted source,
which means from trunk, without custom patches.

Can Alessandro’s patch get a review?

Many thanks!

> On Jun 21, 2016, at 10:59, Alessandro Fanfarillo  
> wrote:
> 
> * PING *
> 
> 2016-06-06 15:05 GMT-06:00 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


--

Cheers!
Dan Nagle

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

[Alessandro Fanfarillo]

* PING *

2016-07-21 13:05 GMT-06:00 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 :
>> 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 :
> 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 
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);
+
+  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, 

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

[Mikael Morin]

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)

[Andre Vehreschild]

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.

> > +
> > +  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.

Sorry for the comments ...

- Andre
-- 
Andre Vehreschild * Email: vehre ad gmx dot de 

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

[Mikael Morin]

Hello,

this is mostly good in general, but is lacking tests.
Especially, tests for successfull matching, and tests for every error 
you are adding in the patch (except maybe the -fcoarray= one).
Also tests that the code executes successfullly with -fcoarray=single, 
and that it produces the right function calls with -fcoarray=lib.


more specific comments below.

Mikael

Le 15/07/2016 à 19:34, Alessandro Fanfarillo a écrit :

Third *PING*

2016-07-04 16:46 GMT-06:00 Alessandro Fanfarillo :

* PING *

2016-06-21 10:59 GMT-06:00 Alessandro Fanfarillo :

* PING *

2016-06-06 15:05 GMT-06:00 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


first_complete_patch.diff

commit b3bca5b09f4cbcf18f2409dae2485a16a7c06498
Author: Alessandro Fanfarillo 
Date:   Mon Jun 6 14:27:37 2016 -0600

First patch Failed Images CAF TS-18508

diff --git a/gcc/fortran/match.c b/gcc/fortran/match.c
index f3a4a43..9f519ff 100644
--- a/gcc/fortran/match.c
+++ b/gcc/fortran/match.c
@@ -1594,6 +1594,7 @@ gfc_match_if (gfc_statement *if_type)
   match ("event post", gfc_match_event_post, ST_EVENT_POST)
   match ("event wait", gfc_match_event_wait, ST_EVENT_WAIT)
   match ("exit", gfc_match_exit, ST_EXIT)
+  match ("fail image", gfc_match_fail_image, ST_FAIL_IMAGE)
   match ("flush", gfc_match_flush, ST_FLUSH)
   match ("forall", match_simple_forall, ST_FORALL)
   match ("go to", gfc_match_goto, ST_GOTO)
@@ -3073,6 +3074,41 @@ gfc_match_event_wait (void)
   return event_statement (ST_EVENT_WAIT);
 }

+/* Match a FAIl IMAGE statement */
+
+static match
+fail_image_statement (gfc_statement st)
+{
+  if (flag_coarray == GFC_FCOARRAY_NONE)
+{
+  gfc_fatal_error ("Coarrays disabled at %C, use %<-fcoarray=%> to 
enable");
+  return MATCH_ERROR;
+}
+
+  if (gfc_match_char ('(') == MATCH_YES)
+goto syntax;
+
+  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 ();

+
+  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?


+  tmp = build_call_expr_loc (input_location, gfor_fndecl_caf_image_status, 
2,
+args[0], build_int_cst (integer_type_node, 
-1));
+  se->expr = tmp;
+}
+}

 static void
diff --git a/gcc/fortran/trans-stmt.c b/gcc/fortran/trans-stmt.c
index 7d3cf8c..ce0eae7 100644
--- a/gcc/fortran/trans-stmt.c
+++ b/gcc/fortran/trans-stmt.c
@@ -674,6 +674,31 @@ gfc_trans_stop (gfc_code *code, bool error_stop)
   return gfc_finish_block ();
 }

+/* Translate the FAIL IMAGE statement.  We have to translate this statement
+   to a runtime library call.  */
+
+tree
+gfc_trans_fail_image (gfc_code *code ATTRIBUTE_UNUSED)
+{
+  tree gfc_int4_type_node = gfc_get_int_type (4);
+  gfc_se se;
+  tree tmp;
+
+  /* Start a new block for this statement.  */
+  gfc_init_se (, NULL);
+  gfc_start_block ();
+
+  tmp = build_int_cst (gfc_int4_type_node, 0);

This tmp doesn't seem to be used.


+  tmp = build_call_expr_loc (input_location,
+gfor_fndecl_caf_fail_image, 1,
+build_int_cst (pchar_type_node, 0));
+
+  gfc_add_expr_to_block (, tmp);
+
+  gfc_add_block_to_block (, );
+
+  return gfc_finish_block ();
+}

 tree
 gfc_trans_lock_unlock (gfc_code *code, 

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 :
> * PING *
>
> 2016-06-21 10:59 GMT-06:00 Alessandro Fanfarillo :
>> * PING *
>>
>> 2016-06-06 15:05 GMT-06:00 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

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

[Alessandro Fanfarillo]

* PING *

2016-06-21 10:59 GMT-06:00 Alessandro Fanfarillo :
> * PING *
>
> 2016-06-06 15:05 GMT-06:00 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

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

[Alessandro Fanfarillo]

* PING *

2016-06-06 15:05 GMT-06:00 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

[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 
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_resolve_failed_images, "team", BT_INTEGER, di, OPTIONAL, 
"kind", BT_INTEGER, di, OPTIONAL);
+
   add_sym_0 ("fdate",  GFC_ISYM_FDATE, CLASS_IMPURE,