Re: [PR fortran/91496] !GCC$ directives error if mistyped or unknown

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: [PR fortran/91496] !GCC$ directives error if mistyped or unknown

Paul Richard Thomas
Hi Harald,

This is OK for trunk.

Thanks!

Paul

On Mon, 26 Aug 2019 at 22:13, Harald Anlauf <[hidden email]> wrote:

>
> Dear all,
>
> the attached patch adds Fortran support for the following pragmas
> (loop annotations): IVDEP (ignore vector dependencies), VECTOR, and
> NOVECTOR.  Furthermore, it downgrades unsupported directives from
> error to warning (by default, it stays an error with -pedantic),
> thus fixing the PR.
>
> It has no effect on existing code (thus regtested cleanly on
> x86_64-pc-linux-gnu), but gives users an option for fine-grained
> control of optimization.  The above pragmas are supported by other
> compilers (with different sentinels, e.g. !DIR$ for Intel, Cray,
> sometimes with slightly different keywords).
>
> OK for trunk, and backport to 9?
>
> Thanks,
> Harald
>
>
> 2019-08-26  Harald Anlauf  <[hidden email]>
>
>         PR fortran/91496
>         * gfortran.h: Extend struct gfc_iterator for loop annotations.
>         * array.c (gfc_copy_iterator): Copy loop annotations by IVDEP,
>         VECTOR, and NOVECTOR pragmas.
>         * decl.c (gfc_match_gcc_ivdep, gfc_match_gcc_vector)
>         (gfc_match_gcc_novector): New matcher functions handling IVDEP,
>         VECTOR, and NOVECTOR pragmas.
>         * match.h: Declare prototypes of matcher functions handling IVDEP,
>         VECTOR, and NOVECTOR pragmas.
>         * parse.c (decode_gcc_attribute, parse_do_block)
>         (parse_executable): Decode IVDEP, VECTOR, and NOVECTOR pragmas;
>         emit warning for unrecognized pragmas instead of error.
>         * trans-stmt.c (gfc_trans_simple_do, gfc_trans_do): Add code to
>         emit annotations for IVDEP, VECTOR, and NOVECTOR pragmas.
>         * gfortran.texi: Document IVDEP, VECTOR, and NOVECTOR pragmas.
>
> 2019-08-26  Harald Anlauf  <[hidden email]>
>
>         PR fortran/91496
>         * gfortran.dg/pr91496.f90: New testcase.
>


--
"If you can't explain it simply, you don't understand it well enough"
- Albert Einstein
Reply | Threaded
Open this post in threaded view
|

Re: [PR fortran/91496] !GCC$ directives error if mistyped or unknown

Harald Anlauf-3
Committed to trunk as svn revision 274966, after removing some
accidentally left-over unused variable declarations (copy&paste).
The actual committed version is attached.

Thanks, Paul, for the quick review!

Unless there are strong objections, I'd like to commit this to
9-branch, too, so that this can be used in the 9.3 release.
Applies/regtests cleanly.  Will wait for a week or so.

Harald

On 08/27/19 10:33, Paul Richard Thomas wrote:

> Hi Harald,
>
> This is OK for trunk.
>
> Thanks!
>
> Paul
>
> On Mon, 26 Aug 2019 at 22:13, Harald Anlauf <[hidden email]> wrote:
>>
>> Dear all,
>>
>> the attached patch adds Fortran support for the following pragmas
>> (loop annotations): IVDEP (ignore vector dependencies), VECTOR, and
>> NOVECTOR.  Furthermore, it downgrades unsupported directives from
>> error to warning (by default, it stays an error with -pedantic),
>> thus fixing the PR.
>>
>> It has no effect on existing code (thus regtested cleanly on
>> x86_64-pc-linux-gnu), but gives users an option for fine-grained
>> control of optimization.  The above pragmas are supported by other
>> compilers (with different sentinels, e.g. !DIR$ for Intel, Cray,
>> sometimes with slightly different keywords).
>>
>> OK for trunk, and backport to 9?
>>
>> Thanks,
>> Harald
>>
>>
>> 2019-08-26  Harald Anlauf  <[hidden email]>
>>
>>         PR fortran/91496
>>         * gfortran.h: Extend struct gfc_iterator for loop annotations.
>>         * array.c (gfc_copy_iterator): Copy loop annotations by IVDEP,
>>         VECTOR, and NOVECTOR pragmas.
>>         * decl.c (gfc_match_gcc_ivdep, gfc_match_gcc_vector)
>>         (gfc_match_gcc_novector): New matcher functions handling IVDEP,
>>         VECTOR, and NOVECTOR pragmas.
>>         * match.h: Declare prototypes of matcher functions handling IVDEP,
>>         VECTOR, and NOVECTOR pragmas.
>>         * parse.c (decode_gcc_attribute, parse_do_block)
>>         (parse_executable): Decode IVDEP, VECTOR, and NOVECTOR pragmas;
>>         emit warning for unrecognized pragmas instead of error.
>>         * trans-stmt.c (gfc_trans_simple_do, gfc_trans_do): Add code to
>>         emit annotations for IVDEP, VECTOR, and NOVECTOR pragmas.
>>         * gfortran.texi: Document IVDEP, VECTOR, and NOVECTOR pragmas.
>>
>> 2019-08-26  Harald Anlauf  <[hidden email]>
>>
>>         PR fortran/91496
>>         * gfortran.dg/pr91496.f90: New testcase.
>>
>
>


patch-pr91496 (12K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PR fortran/91496] !GCC$ directives error if mistyped or unknown

Bernhard Reutner-Fischer
On Tue, 27 Aug 2019 21:49:38 +0200
Harald Anlauf <[hidden email]> wrote:

> Committed to trunk as svn revision 274966, after removing some
> accidentally left-over unused variable declarations (copy&paste).
> The actual committed version is attached.
>
> Thanks, Paul, for the quick review!
>
> Unless there are strong objections, I'd like to commit this to
> 9-branch, too, so that this can be used in the 9.3 release.
> Applies/regtests cleanly.  Will wait for a week or so.

I see that you copied the unfortunate error-message "commence a loop"
and i see that i completely forgot to adjust it as per Mike's
preference in
 https://gcc.gnu.org/ml/fortran/2015-05/msg00166.html

So can you please change your new errors (and the unroll error message
too) to something like the suggested
"directive not at the start of a loop at %C" please?

Many thanks in advance!
PS: This is border obvious, i'd send the patch for review anyway,
maybe a native speaker can provide a better wording.
PPS: I'm still a bit unhappy about the following kludges in unroll even
more so since you copied the concept:
(1) The globals to diagnose misplaced directives are very ugly.
(2) putting the payload into gfc_iterator is ugly, too. I don't
remember offhand how ugly or intrusive it would be to provide means for
passing down an additional optional structure to act as sink for the
directive payload data. Putting those into the iterator is AFAIR not
all that clean. Maybe you could have a look if you can extend this area
to look less clumsy? "hinted_iterator" maybe, or maybe the iterator is
seldomly used anyway and hence we don't care?

thanks and cheers,

>
> Harald
>
> On 08/27/19 10:33, Paul Richard Thomas wrote:
> > Hi Harald,
> >
> > This is OK for trunk.
> >
> > Thanks!
> >
> > Paul
> >
> > On Mon, 26 Aug 2019 at 22:13, Harald Anlauf <[hidden email]> wrote:  
> >>
> >> Dear all,
> >>
> >> the attached patch adds Fortran support for the following pragmas
> >> (loop annotations): IVDEP (ignore vector dependencies), VECTOR, and
> >> NOVECTOR.  Furthermore, it downgrades unsupported directives from
> >> error to warning (by default, it stays an error with -pedantic),
> >> thus fixing the PR.
> >>
> >> It has no effect on existing code (thus regtested cleanly on
> >> x86_64-pc-linux-gnu), but gives users an option for fine-grained
> >> control of optimization.  The above pragmas are supported by other
> >> compilers (with different sentinels, e.g. !DIR$ for Intel, Cray,
> >> sometimes with slightly different keywords).
> >>
> >> OK for trunk, and backport to 9?
> >>
> >> Thanks,
> >> Harald
> >>
> >>
> >> 2019-08-26  Harald Anlauf  <[hidden email]>
> >>
> >>         PR fortran/91496
> >>         * gfortran.h: Extend struct gfc_iterator for loop annotations.
> >>         * array.c (gfc_copy_iterator): Copy loop annotations by IVDEP,
> >>         VECTOR, and NOVECTOR pragmas.
> >>         * decl.c (gfc_match_gcc_ivdep, gfc_match_gcc_vector)
> >>         (gfc_match_gcc_novector): New matcher functions handling IVDEP,
> >>         VECTOR, and NOVECTOR pragmas.
> >>         * match.h: Declare prototypes of matcher functions handling IVDEP,
> >>         VECTOR, and NOVECTOR pragmas.
> >>         * parse.c (decode_gcc_attribute, parse_do_block)
> >>         (parse_executable): Decode IVDEP, VECTOR, and NOVECTOR pragmas;
> >>         emit warning for unrecognized pragmas instead of error.
> >>         * trans-stmt.c (gfc_trans_simple_do, gfc_trans_do): Add code to
> >>         emit annotations for IVDEP, VECTOR, and NOVECTOR pragmas.
> >>         * gfortran.texi: Document IVDEP, VECTOR, and NOVECTOR pragmas.
> >>
> >> 2019-08-26  Harald Anlauf  <[hidden email]>
> >>
> >>         PR fortran/91496
> >>         * gfortran.dg/pr91496.f90: New testcase.
> >>  
> >
> >  
>

Reply | Threaded
Open this post in threaded view
|

Re: [PR fortran/91496] !GCC$ directives error if mistyped or unknown

Harald Anlauf-3
Hi Bernhard,

On 08/28/19 20:57, Bernhard Reutner-Fischer wrote:

> I see that you copied the unfortunate error-message "commence a loop"
> and i see that i completely forgot to adjust it as per Mike's
> preference in
>  https://gcc.gnu.org/ml/fortran/2015-05/msg00166.html
>
> So can you please change your new errors (and the unroll error message
> too) to something like the suggested
> "directive not at the start of a loop at %C" please?
>
> Many thanks in advance!
> PS: This is border obvious, i'd send the patch for review anyway,
> maybe a native speaker can provide a better wording.
see attachment and below for Changelog.  Regtested OK.

I have opted for the variant "directive not at the start of a loop",
which is what Mike preferred, and what matches the current capabilities.

Is that OK?

> PPS: I'm still a bit unhappy about the following kludges in unroll even
> more so since you copied the concept:
> (1) The globals to diagnose misplaced directives are very ugly.
> (2) putting the payload into gfc_iterator is ugly, too. I don't
> remember offhand how ugly or intrusive it would be to provide means for
> passing down an additional optional structure to act as sink for the
> directive payload data. Putting those into the iterator is AFAIR not
> all that clean. Maybe you could have a look if you can extend this area
> to look less clumsy? "hinted_iterator" maybe, or maybe the iterator is
> seldomly used anyway and hence we don't care?
Could you please open a new PR for this?

There are many things that could be improved in this context.
There is a lot that could be learned from e.g. the Cray or NEC
compilers, where you can explicitly control loop fusion, loop
blocking, which are very important in (my) practice.

The current implementation also does not support array notation,
which I consider a major limitation.  I'll have to learn how this
could be done.

> thanks and cheers,
>

Thanks for the feedback!

Harald

2019-08-28  Harald Anlauf  <[hidden email]>

        PR fortran/91496
        * parse.c (parse_executable): Improve error messages for
        improperly placed pragmas not preceeding a loop.

2019-08-28  Harald Anlauf  <[hidden email]>

        PR fortran/91496
        * gfortran.dg/directive_unroll_5.f90: Adjust error message.


patch-pr91496-part2 (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PR fortran/91496] !GCC$ directives error if mistyped or unknown

Harald Anlauf-3
Ping!

On 08/28/19 21:58, Harald Anlauf wrote:

> Hi Bernhard,
>
> On 08/28/19 20:57, Bernhard Reutner-Fischer wrote:
>> I see that you copied the unfortunate error-message "commence a loop"
>> and i see that i completely forgot to adjust it as per Mike's
>> preference in
>>  https://gcc.gnu.org/ml/fortran/2015-05/msg00166.html
>>
>> So can you please change your new errors (and the unroll error message
>> too) to something like the suggested
>> "directive not at the start of a loop at %C" please?
>>
>> Many thanks in advance!
>> PS: This is border obvious, i'd send the patch for review anyway,
>> maybe a native speaker can provide a better wording.
>
> see attachment and below for Changelog.  Regtested OK.
>
> I have opted for the variant "directive not at the start of a loop",
> which is what Mike preferred, and what matches the current capabilities.
>
> Is that OK?
>
>> PPS: I'm still a bit unhappy about the following kludges in unroll even
>> more so since you copied the concept:
>> (1) The globals to diagnose misplaced directives are very ugly.
>> (2) putting the payload into gfc_iterator is ugly, too. I don't
>> remember offhand how ugly or intrusive it would be to provide means for
>> passing down an additional optional structure to act as sink for the
>> directive payload data. Putting those into the iterator is AFAIR not
>> all that clean. Maybe you could have a look if you can extend this area
>> to look less clumsy? "hinted_iterator" maybe, or maybe the iterator is
>> seldomly used anyway and hence we don't care?
>
> Could you please open a new PR for this?
>
> There are many things that could be improved in this context.
> There is a lot that could be learned from e.g. the Cray or NEC
> compilers, where you can explicitly control loop fusion, loop
> blocking, which are very important in (my) practice.
>
> The current implementation also does not support array notation,
> which I consider a major limitation.  I'll have to learn how this
> could be done.
>
>> thanks and cheers,
>>
>
> Thanks for the feedback!
>
> Harald
>
> 2019-08-28  Harald Anlauf  <[hidden email]>
>
> PR fortran/91496
> * parse.c (parse_executable): Improve error messages for
> improperly placed pragmas not preceeding a loop.
>
> 2019-08-28  Harald Anlauf  <[hidden email]>
>
> PR fortran/91496
> * gfortran.dg/directive_unroll_5.f90: Adjust error message.
>

Reply | Threaded
Open this post in threaded view
|

Re: [PR fortran/91496] !GCC$ directives error if mistyped or unknown

Steve Kargl
Looks like a straight forward mechanical change.
Probably doesn't need a review.

OK.

--
steve



On Thu, Sep 05, 2019 at 09:55:58PM +0200, Harald Anlauf wrote:

> Ping!
>
> On 08/28/19 21:58, Harald Anlauf wrote:
> > Hi Bernhard,
> >
> > On 08/28/19 20:57, Bernhard Reutner-Fischer wrote:
> >> I see that you copied the unfortunate error-message "commence a loop"
> >> and i see that i completely forgot to adjust it as per Mike's
> >> preference in
> >>  https://gcc.gnu.org/ml/fortran/2015-05/msg00166.html
> >>
> >> So can you please change your new errors (and the unroll error message
> >> too) to something like the suggested
> >> "directive not at the start of a loop at %C" please?
> >>
> >> Many thanks in advance!
> >> PS: This is border obvious, i'd send the patch for review anyway,
> >> maybe a native speaker can provide a better wording.
> >
> > see attachment and below for Changelog.  Regtested OK.
> >
> > I have opted for the variant "directive not at the start of a loop",
> > which is what Mike preferred, and what matches the current capabilities.
> >
> > Is that OK?
> >
> >> PPS: I'm still a bit unhappy about the following kludges in unroll even
> >> more so since you copied the concept:
> >> (1) The globals to diagnose misplaced directives are very ugly.
> >> (2) putting the payload into gfc_iterator is ugly, too. I don't
> >> remember offhand how ugly or intrusive it would be to provide means for
> >> passing down an additional optional structure to act as sink for the
> >> directive payload data. Putting those into the iterator is AFAIR not
> >> all that clean. Maybe you could have a look if you can extend this area
> >> to look less clumsy? "hinted_iterator" maybe, or maybe the iterator is
> >> seldomly used anyway and hence we don't care?
> >
> > Could you please open a new PR for this?
> >
> > There are many things that could be improved in this context.
> > There is a lot that could be learned from e.g. the Cray or NEC
> > compilers, where you can explicitly control loop fusion, loop
> > blocking, which are very important in (my) practice.
> >
> > The current implementation also does not support array notation,
> > which I consider a major limitation.  I'll have to learn how this
> > could be done.
> >
> >> thanks and cheers,
> >>
> >
> > Thanks for the feedback!
> >
> > Harald
> >
> > 2019-08-28  Harald Anlauf  <[hidden email]>
> >
> > PR fortran/91496
> > * parse.c (parse_executable): Improve error messages for
> > improperly placed pragmas not preceeding a loop.
> >
> > 2019-08-28  Harald Anlauf  <[hidden email]>
> >
> > PR fortran/91496
> > * gfortran.dg/directive_unroll_5.f90: Adjust error message.
> >
>

--
Steve
20170425 https://www.youtube.com/watch?v=VWUpyCsUKR4
20161221 https://www.youtube.com/watch?v=IbCHE-hONow