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

Andre Vehreschild vehre@gmx.de
Wed Jan 18 17:20:00 GMT 2017


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 



More information about the Fortran mailing list