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

[Alessandro Fanfarillo <fanfarillo dot gcc at gmail dot 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

Attachment: first_complete_patch_REV1.diff
Description: Text document