public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Paul Richard Thomas <paul.richard.thomas@gmail.com>
To: Alessandro Fanfarillo <fanfarillo.gcc@gmail.com>
Cc: Mikael Morin <morin-mikael@orange.fr>,
	Andre Vehreschild <vehre@gmx.de>, gfortran <fortran@gcc.gnu.org>,
		gcc-patches <gcc-patches@gcc.gnu.org>,
	Tobias Burnus <burnus@net-b.de>
Subject: Re: [Fortran, Patch] First patch for coarray FAILED IMAGES (TS 18508)
Date: Tue, 09 Aug 2016 11:23:00 -0000	[thread overview]
Message-ID: <CAGkQGiJCkmTQrbBBMckKty+qGjtO9CPoseg_BHRvAvFW4e8PQQ@mail.gmail.com> (raw)
In-Reply-To: <CAHqFgjWxRi_0EqVowQRMxMi70zKfkGF_oB8iPy_ZjJ6nf1DkOQ@mail.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

  reply	other threads:[~2016-08-09 11:23 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-06 21:05 Alessandro Fanfarillo
2016-06-21 16:59 ` Alessandro Fanfarillo
2016-07-04 22:46   ` Alessandro Fanfarillo
2016-07-15 17:34     ` Alessandro Fanfarillo
2016-07-19 18:56       ` Mikael Morin
2016-07-20  9:39         ` Andre Vehreschild
2016-07-20 19:18           ` Mikael Morin
2016-07-21 19:05             ` Alessandro Fanfarillo
2016-08-04  3:09               ` Alessandro Fanfarillo
2016-08-09 11:23                 ` Paul Richard Thomas [this message]
2016-08-09 17:44                   ` Alessandro Fanfarillo
2016-09-07 21:04                   ` Alessandro Fanfarillo
     [not found]                     ` <CAHqFgjXbwQQnnZp5N+WtWnxNxWducGcU9QSdHRhCdPwNf1tdBQ@mail.gmail.com>
2016-09-19 15:58                       ` Andre Vehreschild
2016-09-21 18:37                         ` Alessandro Fanfarillo
2016-09-28 13:52                           ` Alessandro Fanfarillo
2016-08-08 17:12   ` Dan Nagle

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAGkQGiJCkmTQrbBBMckKty+qGjtO9CPoseg_BHRvAvFW4e8PQQ@mail.gmail.com \
    --to=paul.richard.thomas@gmail.com \
    --cc=burnus@net-b.de \
    --cc=fanfarillo.gcc@gmail.com \
    --cc=fortran@gcc.gnu.org \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=morin-mikael@orange.fr \
    --cc=vehre@gmx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).