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

[Alessandro Fanfarillo <fanfarillo dot gcc at gmail dot com>]

* PING *

2016-09-21 19:03 GMT+01:00 Alessandro Fanfarillo <fanfarillo.gcc@gmail.com>:
> 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