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

Alessandro Fanfarillo fanfarillo.gcc@gmail.com
Wed Jan 18 18:18:00 GMT 2017


I guess implementing this function in libcaf_single should fix the
problem, right?
Furthermore, FAIL IMAGE is a statement, it doesn't take any arguments.
Anyway, I will re-adapt the patch to the current trunk and will
address all the issues.

All comments/observations are more than welcome.

2017-01-18 11:01 GMT-07:00 Andre Vehreschild <vehre@gmx.de>:
> Hi Alessandro,
>
> unfortunately we are here on the compiler. And in its test-library the function
> fail_image is neither declared nor defined.
>
> - Andre
>
> On Wed, 18 Jan 2017 10:52:25 -0700
> Alessandro Fanfarillo <elfanfa@ucar.edu> wrote:
>
>> Hi Andre,
>> the functions related with failed images are all implemented into the
>> branch "failed-images-comm" of opencoarrays.
>>
>> On Wed, Jan 18, 2017 at 10:20 AM, Andre Vehreschild <vehre@gmx.de> wrote:
>>
>> > Hi Damian,
>> >
>> > the patch you attached showed to be empty in my mail-client. I retrieved
>> > it now
>> > from a former mail on the list. It does not apply cleanly anymore.
>> >
>> > I can't okay it, after looking at some of the lines. There is a function
>> > call
>> > declared, but no library function is declared -> link error. Parsing of the
>> > arguments to FAIL IMAGE looks dubious to me. There are numerous style
>> > violations. The API-documentation has not been updated.
>> >
>> > So no, I will have to work this over before the patch could have a chance
>> > to
>> > get accepted. Sorry.
>> >
>> > Regards,
>> >         Andre
>> >
>> > On Tue, 17 Jan 2017 22:39:06 -0800
>> > Damian Rouson <damian@sourceryinstitute.org> wrote:
>> >
>> > > *PING*
>> > >
>> > > With the 7.1.0 deadline approaching tomorrow, is there any chance of
>> > getting
>> > > Alessandro’s FAILED IMAGES patch approved for trunk?  As far as I know,
>> > > Fortran is the first internationally standardized language to incorporate
>> > > intrinsic support for fault tolerance, which is believed to be essential
>> > to
>> > > reaching exaflop performance.  As far as I know, gfortran is the first
>> > > compiler to implement this Fortran 2015 feature.  This is a huge deal
>> > and I’m
>> > > hoping Alessandro and I will be able to report that this is a released
>> > > feature in a talk we’re giving jointly in early February.  Any assistance
>> > > would be greatly appreciated.
>> > >
>> > > Damian
>> > >
>> > > > Begin forwarded message:
>> > > >
>> > > >
>> > > > From: Alessandro Fanfarillo <fanfarillo.gcc@gmail.com>
>> > > > Subject: Re: [Fortran, Patch] First patch for coarray FAILED IMAGES (TS
>> > > > 18508) Date: September 21, 2016 at 11:03:48 AM PDT
>> > > > To: Andre Vehreschild <vehre@gmx.de>
>> > > > Cc: Paul Richard Thomas <paul.richard.thomas@gmail.com>, gfortran
>> > > > <fortran@gcc.gnu.org>, gcc-patches <gcc-patches@gcc.gnu.org>, Mikael
>> > Morin
>> > > > <morin-mikael@orange.fr>, Tobias Burnus <burnus@net-b.de>
>> > > >
>> > > >
>> > > > Thanks Andre.
>> > > >
>> > > > 2016-09-19 9:55 GMT-06:00 Andre Vehreschild <vehre@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.gcc@gmail.com> wrote:
>> > > >>
>> > > >>> * PING *
>> > > >>>
>> > > >>> On Sep 7, 2016 3:01 PM, "Alessandro Fanfarillo" <
>> > fanfarillo.gcc@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.thomas@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.gcc@gmail.com> wrote:
>> > > >>>>>> * PING *
>> > > >>>>>>
>> > > >>>>>> 2016-07-21 13:05 GMT-06:00 Alessandro Fanfarillo <
>> > > >>>> fanfarillo.gcc@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-mikael@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
>> > > >>>>
>> > > >>
>> > > >>
>> > > >> --
>> > > >> Andre Vehreschild * Email: vehre ad gmx dot de
>> > > >
>> > > >
>> > >
>> > > ________________________________
>> > > Damian Rouson, Ph.D., P.E.
>> > > President, Sourcery Institute
>> > > http://www.sourceryinstitute.org
>> > > +1-510-600-2992 (mobile)
>> > >
>> >
>> >
>> > --
>> > Andre Vehreschild * Email: vehre ad gmx dot de
>> >
>>
>>
>>
>
>
> --
> Andre Vehreschild * Email: vehre ad gmx dot de



More information about the Fortran mailing list