This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PR fortran/63858] Fix mix of OpenACC and OpenMP sentinels in continuations
- From: Bernhard Reutner-Fischer <rep dot dot dot nop at gmail dot com>
- To: Cesar Philippidis <cesar at codesourcery dot com>,Jakub Jelinek <jakub at redhat dot com>,Thomas Schwinge <thomas at codesourcery dot com>
- Cc: gcc-patches at gcc dot gnu dot org,fortran at gcc dot gnu dot org,i dot usmanov at samsung dot com,Ilmir Usmanov <me at ilmir dot us>
- Date: Wed, 25 Nov 2015 20:10:15 +0100
- Subject: Re: [PR fortran/63858] Fix mix of OpenACC and OpenMP sentinels in continuations
- Authentication-results: sourceware.org; auth=none
- References: <3008431435623821 at web14j dot yandex dot ru> <5591E54E dot 90509 at ilmir dot us> <5575ADD2 dot 8030007 at codesourcery dot com> <87r3ntr8li dot fsf at kepler dot schwinge dot homeip dot net> <878u7cny9v dot fsf at kepler dot schwinge dot homeip dot net> <20151020093733 dot GV478 at tucnak dot redhat dot com> <5655C6CB dot 5020901 at codesourcery dot com>
On November 25, 2015 3:33:47 PM GMT+01:00, Cesar Philippidis <cesar@codesourcery.com> wrote:
>On 10/20/2015 02:37 AM, Jakub Jelinek wrote:
>> On Fri, Oct 09, 2015 at 12:15:24PM +0200, Thomas Schwinge wrote:
>>> diff --git gcc/fortran/scanner.c gcc/fortran/scanner.c
>>> index bfb7d45..1e1ea84 100644
>>> --- gcc/fortran/scanner.c
>>> +++ gcc/fortran/scanner.c
>>> @@ -935,6 +935,63 @@ skip_free_comments (void)
>>> return false;
>>> }
>>>
>>> +/* Return true if MP was matched in fixed form. */
>>> +static bool
>>> +skip_omp_attribute_fixed (locus *start)
>>
>> Technically, this isn't attribute, but sentinel.
>> So, skip_fixed_omp_sentinel? I know the free functions
>> are called attribute, perhaps we should rename them too,
>> patch to do so preapproved.
>
>I've renamed those functions in this patch. The free variants are named
>skip_free_*_sentinel.
>
>>> +{
>>> + gfc_char_t c;
>>> + if (((c = next_char ()) == 'm' || c == 'M')
>>> + && ((c = next_char ()) == 'p' || c == 'P'))
>>> + {
>>> + c = next_char ();
>>> + if (c != '\n'
>>> + && (continue_flag
>>
>> The original code checked here
>> (openmp_flag && continue_flag)
>> instead. Is that change intentional?
>
>I think so. Without it, continuations won't work with -fopenmp-simd.
>Note how that function call is guarded by
>
> if ((flag_openmp || flag_openmp_simd) && !flag_openacc)
>
>> Looking around, we probably don't have a testcase coverage for say
>> fixed form:
>>
>> C*OMP+PARALLEL DO
>> do ...
>
>That's going to be tricky. In fixed mode, the only way that we can tell
>if there is an omp/acc continuation is by inspecting the character at
>position 6. So, there could be a comment like
>
>C*ACCELERATOR
>
>which would get picked up as an acc continuation.
>
>> (i.e. where it starts with an OpenMP (or OpenACC) continuation,
>without
>> non-continued line first), or for free form where:
>>
>> something &
>> !$omp & parallel
>>
>> (ditto for OpenACC).
>
>What type of error should this be reporting? Right now it does report
>an
>error because this gets expanded to
>
>something parallel
>
>That's clearly not correct. But at the same time, it would still be an
>error if the user placed !$omp/acc between a continuation.
>
>>> + while (gfc_is_whitespace (c));
gfc_gobble_whitespace () ?
TIA,
PS: this thread has the relevant citations from the openmp standard, in case https://gcc.gnu.org/ml/fortran/2007-02/msg00312.html
I didn't look to see what acc says in this regard.
>>> + if (c != '\n' && c != '!')
>>> + {
>>> + /* Canonicalize to *$omp. */
>>
>> The comment has a pasto, by storing * you canonicalize to *$acc.
>
>Fixed.