OpenMP in Flang and Clang

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

OpenMP in Flang and Clang

Fangrui Song via cfe-dev
Since we now merged F18, yay!, we should start reusing OpenMP
"infrastructure" between LLVM/Clang and LLVM/Flang. This will reduce
duplication (coding & maintenance), improve testing, and thereby should
get us to OpenMP 5.X support faster.

I started to look at `parse-tree.h` and `openmp-parsers.cpp` which
contain a lot of code dealing with the OpenMP "grammar". Arguably, we
don't want the duplication with
   `llvm/include/llvm/Frontend/OpenMP/OMPKinds.def`
(and the almost empty "old" clang file `OpenMPKinds.def`).

I think there are multiple steps we should take. Note that I have to
list them in some order, please don't interpret this as the order I
suggest we work on them.

   Move and merge the ENUM_CLASS definitions that can be shared into
   OMPKinds.def. This might require to extend the macros such that we
   have a flag for C/C++ or Fortran only but that should not be too hard.
   The existing Clang code (or better OpenMPIRBuilder by now) could
   benefit from new enum classes already in Flang, e.g. OmpCancelType,
   while things like the kind of an OmpScheduleClause should only be
   defined once.

   Determine how we can merge the definitions in `parse-tree.h` with the
   ones in `OpenMPClauses.h`. I was hoping to trim down the latter anyway
   as it seems there is quite a bit of duplication in there. I was also
   hoping we could generate them from the macro file (or a table-gen
   file) so we don't have to write all the boilerplate when new
   directives and clauses are added. As an example, in the new OpenMP
   context selector support for Clang new selector sets, selectors, and
   traits can be added via macros in the OMPKinds.def file. Boilerplate
   like parsing, pretty printing, error messages, will work right away.
   Only if the existing logic to determine if the selector matches a
   context is not sufficient you need to modify one more place.
   (This is not true for implementation defined extensions that change
    the behavior of the entire thing but anyway.)

   Rethink the use of `std::tuple` in and `std::variant` in (the OpenMP
   parts) of parse-tree.h. The latter is used for OmpLinearClause for
   example. As far as I can tell, the two members of the variant have two
   common fields and one has a third, an enum. In clang this is usually
   solved by having an "unknown" or "none" member in the enum which
   allows to remove the duplication and variant completely. I would like
   to replace tuples so we can give their constitutes proper names. I
   think a similar point was made before when we discussed tooling with
   (or extending) Flang.

I hope to get some feedback and thoughts on this.

Thanks,
   Johannes

_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: OpenMP in Flang and Clang

Fangrui Song via cfe-dev
Flang today has overlapping enums for OpenMP directives,
e.g. OmpLoopDirective and OmpDirective; both of these enums
have names for PARALLEL DO, for example.

Perhaps before https://reviews.llvm.org/D77812 lands we
should merge the multiple sets of flang enumerations with
the other enumerations as Johannes describes.

On 4/9/20, 9:55 AM, "cfe-dev on behalf of Johannes Doerfert via cfe-dev" <[hidden email] on behalf of [hidden email]> wrote:

    External email: Use caution opening links or attachments


    Since we now merged F18, yay!, we should start reusing OpenMP
    "infrastructure" between LLVM/Clang and LLVM/Flang. This will reduce
    duplication (coding & maintenance), improve testing, and thereby should
    get us to OpenMP 5.X support faster.

    I started to look at `parse-tree.h` and `openmp-parsers.cpp` which
    contain a lot of code dealing with the OpenMP "grammar". Arguably, we
    don't want the duplication with
       `llvm/include/llvm/Frontend/OpenMP/OMPKinds.def`
    (and the almost empty "old" clang file `OpenMPKinds.def`).

    I think there are multiple steps we should take. Note that I have to
    list them in some order, please don't interpret this as the order I
    suggest we work on them.

       Move and merge the ENUM_CLASS definitions that can be shared into
       OMPKinds.def. This might require to extend the macros such that we
       have a flag for C/C++ or Fortran only but that should not be too hard.
       The existing Clang code (or better OpenMPIRBuilder by now) could
       benefit from new enum classes already in Flang, e.g. OmpCancelType,
       while things like the kind of an OmpScheduleClause should only be
       defined once.

       Determine how we can merge the definitions in `parse-tree.h` with the
       ones in `OpenMPClauses.h`. I was hoping to trim down the latter anyway
       as it seems there is quite a bit of duplication in there. I was also
       hoping we could generate them from the macro file (or a table-gen
       file) so we don't have to write all the boilerplate when new
       directives and clauses are added. As an example, in the new OpenMP
       context selector support for Clang new selector sets, selectors, and
       traits can be added via macros in the OMPKinds.def file. Boilerplate
       like parsing, pretty printing, error messages, will work right away.
       Only if the existing logic to determine if the selector matches a
       context is not sufficient you need to modify one more place.
       (This is not true for implementation defined extensions that change
        the behavior of the entire thing but anyway.)

       Rethink the use of `std::tuple` in and `std::variant` in (the OpenMP
       parts) of parse-tree.h. The latter is used for OmpLinearClause for
       example. As far as I can tell, the two members of the variant have two
       common fields and one has a third, an enum. In clang this is usually
       solved by having an "unknown" or "none" member in the enum which
       allows to remove the duplication and variant completely. I would like
       to replace tuples so we can give their constitutes proper names. I
       think a similar point was made before when we discussed tooling with
       (or extending) Flang.

    I hope to get some feedback and thoughts on this.

    Thanks,
       Johannes

    _______________________________________________
    cfe-dev mailing list
    [hidden email]
    https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev


-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------
_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: OpenMP in Flang and Clang

Fangrui Song via cfe-dev
I agree that we should align with clang's enums here in principle, however my preference would still be to merge that patch so that we have the checks and tests implemented as a first step, and then take a more holistic look at aligning with clang on OpenMP semantic checks.

Another consideration is that clang currently tracks OpenMP 5 (and only that specification) whereas we've been implementing 4.5 up to now. When we try and share semantics checks with clang we will probably want to do a more general version bump to OpenMP 5 at around the same time, otherwise our semantics checks and our parser won't match with regards to which version of the specification they implement.

Thanks
David Truby

From: flang-dev <[hidden email]> on behalf of Steve Scalpone via flang-dev <[hidden email]>
Sent: 14 April 2020 15:02
To: Johannes Doerfert <[hidden email]>; [hidden email] <[hidden email]>
Cc: [hidden email] <[hidden email]>
Subject: Re: [flang-dev] [cfe-dev] OpenMP in Flang and Clang
 
Flang today has overlapping enums for OpenMP directives,
e.g. OmpLoopDirective and OmpDirective; both of these enums
have names for PARALLEL DO, for example.

Perhaps before https://reviews.llvm.org/D77812 lands we
should merge the multiple sets of flang enumerations with
the other enumerations as Johannes describes.

On 4/9/20, 9:55 AM, "cfe-dev on behalf of Johannes Doerfert via cfe-dev" <[hidden email] on behalf of [hidden email]> wrote:

    External email: Use caution opening links or attachments


    Since we now merged F18, yay!, we should start reusing OpenMP
    "infrastructure" between LLVM/Clang and LLVM/Flang. This will reduce
    duplication (coding & maintenance), improve testing, and thereby should
    get us to OpenMP 5.X support faster.

    I started to look at `parse-tree.h` and `openmp-parsers.cpp` which
    contain a lot of code dealing with the OpenMP "grammar". Arguably, we
    don't want the duplication with
       `llvm/include/llvm/Frontend/OpenMP/OMPKinds.def`
    (and the almost empty "old" clang file `OpenMPKinds.def`).

    I think there are multiple steps we should take. Note that I have to
    list them in some order, please don't interpret this as the order I
    suggest we work on them.

       Move and merge the ENUM_CLASS definitions that can be shared into
       OMPKinds.def. This might require to extend the macros such that we
       have a flag for C/C++ or Fortran only but that should not be too hard.
       The existing Clang code (or better OpenMPIRBuilder by now) could
       benefit from new enum classes already in Flang, e.g. OmpCancelType,
       while things like the kind of an OmpScheduleClause should only be
       defined once.

       Determine how we can merge the definitions in `parse-tree.h` with the
       ones in `OpenMPClauses.h`. I was hoping to trim down the latter anyway
       as it seems there is quite a bit of duplication in there. I was also
       hoping we could generate them from the macro file (or a table-gen
       file) so we don't have to write all the boilerplate when new
       directives and clauses are added. As an example, in the new OpenMP
       context selector support for Clang new selector sets, selectors, and
       traits can be added via macros in the OMPKinds.def file. Boilerplate
       like parsing, pretty printing, error messages, will work right away.
       Only if the existing logic to determine if the selector matches a
       context is not sufficient you need to modify one more place.
       (This is not true for implementation defined extensions that change
        the behavior of the entire thing but anyway.)

       Rethink the use of `std::tuple` in and `std::variant` in (the OpenMP
       parts) of parse-tree.h. The latter is used for OmpLinearClause for
       example. As far as I can tell, the two members of the variant have two
       common fields and one has a third, an enum. In clang this is usually
       solved by having an "unknown" or "none" member in the enum which
       allows to remove the duplication and variant completely. I would like
       to replace tuples so we can give their constitutes proper names. I
       think a similar point was made before when we discussed tooling with
       (or extending) Flang.

    I hope to get some feedback and thoughts on this.

    Thanks,
       Johannes

    _______________________________________________
    cfe-dev mailing list
    [hidden email]
    https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev


-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------
_______________________________________________
flang-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/flang-dev

_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: [flang-dev] OpenMP in Flang and Clang

Fangrui Song via cfe-dev
In reply to this post by Fangrui Song via cfe-dev
Thanks Johannes for your mail. Yes, we should try to share code with Clang for the OpenMP portion.

Of the options you suggest (macrofile vs tablegen) my preference will be to write the structure and restrictions of OpenMP constructs and clauses as a tablegen file. This can be then used to generate the checks and definitions for OpenMP constructs and clauses for Clang and Flang as is there currently today. 

We should also switch to the OpenMP module and header in llvm. This was previously discussed in the link below.

--Kiran


From: flang-dev <[hidden email]> on behalf of Johannes Doerfert via flang-dev <[hidden email]>
Sent: 09 April 2020 17:46
To: [hidden email] <[hidden email]>
Cc: [hidden email] <[hidden email]>
Subject: [flang-dev] OpenMP in Flang and Clang
 
Since we now merged F18, yay!, we should start reusing OpenMP
"infrastructure" between LLVM/Clang and LLVM/Flang. This will reduce
duplication (coding & maintenance), improve testing, and thereby should
get us to OpenMP 5.X support faster.

I started to look at `parse-tree.h` and `openmp-parsers.cpp` which
contain a lot of code dealing with the OpenMP "grammar". Arguably, we
don't want the duplication with
   `llvm/include/llvm/Frontend/OpenMP/OMPKinds.def`
(and the almost empty "old" clang file `OpenMPKinds.def`).

I think there are multiple steps we should take. Note that I have to
list them in some order, please don't interpret this as the order I
suggest we work on them.

   Move and merge the ENUM_CLASS definitions that can be shared into
   OMPKinds.def. This might require to extend the macros such that we
   have a flag for C/C++ or Fortran only but that should not be too hard.
   The existing Clang code (or better OpenMPIRBuilder by now) could
   benefit from new enum classes already in Flang, e.g. OmpCancelType,
   while things like the kind of an OmpScheduleClause should only be
   defined once.

   Determine how we can merge the definitions in `parse-tree.h` with the
   ones in `OpenMPClauses.h`. I was hoping to trim down the latter anyway
   as it seems there is quite a bit of duplication in there. I was also
   hoping we could generate them from the macro file (or a table-gen
   file) so we don't have to write all the boilerplate when new
   directives and clauses are added. As an example, in the new OpenMP
   context selector support for Clang new selector sets, selectors, and
   traits can be added via macros in the OMPKinds.def file. Boilerplate
   like parsing, pretty printing, error messages, will work right away.
   Only if the existing logic to determine if the selector matches a
   context is not sufficient you need to modify one more place.
   (This is not true for implementation defined extensions that change
    the behavior of the entire thing but anyway.)

   Rethink the use of `std::tuple` in and `std::variant` in (the OpenMP
   parts) of parse-tree.h. The latter is used for OmpLinearClause for
   example. As far as I can tell, the two members of the variant have two
   common fields and one has a third, an enum. In clang this is usually
   solved by having an "unknown" or "none" member in the enum which
   allows to remove the duplication and variant completely. I would like
   to replace tuples so we can give their constitutes proper names. I
   think a similar point was made before when we discussed tooling with
   (or extending) Flang.

I hope to get some feedback and thoughts on this.

Thanks,
   Johannes

_______________________________________________
flang-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/flang-dev

_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: [flang-dev] OpenMP in Flang and Clang

Fangrui Song via cfe-dev

On 4/14/20 10:23 AM, Kiran Chandramohan wrote:
 > Thanks Johannes for your mail. Yes, we should try to share code with
 > Clang for the OpenMP portion.
 >
 > Of the options you suggest (macrofile vs tablegen) my preference will
 > be to write the structure and restrictions of OpenMP constructs and
 > clauses as a tablegen file. This can be then used to generate the
 > checks and definitions for OpenMP constructs and clauses for Clang and
 > Flang as is there currently today.

I agree, tablegen is unfortunately the better choice. I did extend on
the macro approach started by Clang so far as it is "simpler". However,
tablegen provides more flexibility. We can not only generate checks,
codegen, etc. but later use the formulation to create tests as well.
Granted, those tests have the same systematic errors as clang/flang, but
assuming we can validate them against other compilers we should be able
to cover quite a substantial subset of the semantics in a maintainable
way. I mean, if something changes we regenerate the tests. We can
include the tablegen rules that went into a test as well, etc.


 > We should also switch to the OpenMP module and header in llvm. This
 > was previously discussed in the link below.
 > https://github.com/flang-compiler/f18/pull/685#discussion_r317987276

I'm unsure how this ties in, sorry.

Cheers,
   Johannes

 >
 > --Kiran
 >
 > ________________________________
 > From: flang-dev <[hidden email]> on behalf of
Johannes Doerfert via flang-dev <[hidden email]>
 > Sent: 09 April 2020 17:46
 > To: [hidden email] <[hidden email]>
 > Cc: [hidden email] <[hidden email]>
 > Subject: [flang-dev] OpenMP in Flang and Clang
 >
 > Since we now merged F18, yay!, we should start reusing OpenMP
 > "infrastructure" between LLVM/Clang and LLVM/Flang. This will reduce
 > duplication (coding & maintenance), improve testing, and thereby should
 > get us to OpenMP 5.X support faster.
 >
 > I started to look at `parse-tree.h` and `openmp-parsers.cpp` which
 > contain a lot of code dealing with the OpenMP "grammar". Arguably, we
 > don't want the duplication with
 >    `llvm/include/llvm/Frontend/OpenMP/OMPKinds.def`
 > (and the almost empty "old" clang file `OpenMPKinds.def`).
 >
 > I think there are multiple steps we should take. Note that I have to
 > list them in some order, please don't interpret this as the order I
 > suggest we work on them.
 >
 >    Move and merge the ENUM_CLASS definitions that can be shared into
 >    OMPKinds.def. This might require to extend the macros such that we
 >    have a flag for C/C++ or Fortran only but that should not be too hard.
 >    The existing Clang code (or better OpenMPIRBuilder by now) could
 >    benefit from new enum classes already in Flang, e.g. OmpCancelType,
 >    while things like the kind of an OmpScheduleClause should only be
 >    defined once.
 >
 >    Determine how we can merge the definitions in `parse-tree.h` with the
 >    ones in `OpenMPClauses.h`. I was hoping to trim down the latter anyway
 >    as it seems there is quite a bit of duplication in there. I was also
 >    hoping we could generate them from the macro file (or a table-gen
 >    file) so we don't have to write all the boilerplate when new
 >    directives and clauses are added. As an example, in the new OpenMP
 >    context selector support for Clang new selector sets, selectors, and
 >    traits can be added via macros in the OMPKinds.def file. Boilerplate
 >    like parsing, pretty printing, error messages, will work right away.
 >    Only if the existing logic to determine if the selector matches a
 >    context is not sufficient you need to modify one more place.
 >    (This is not true for implementation defined extensions that change
 >     the behavior of the entire thing but anyway.)
 >
 >    Rethink the use of `std::tuple` in and `std::variant` in (the OpenMP
 >    parts) of parse-tree.h. The latter is used for OmpLinearClause for
 >    example. As far as I can tell, the two members of the variant have two
 >    common fields and one has a third, an enum. In clang this is usually
 >    solved by having an "unknown" or "none" member in the enum which
 >    allows to remove the duplication and variant completely. I would like
 >    to replace tuples so we can give their constitutes proper names. I
 >    think a similar point was made before when we discussed tooling with
 >    (or extending) Flang.
 >
 > I hope to get some feedback and thoughts on this.
 >
 > Thanks,
 >    Johannes
 >
 > _______________________________________________
 > flang-dev mailing list
 > [hidden email]
 > https://lists.llvm.org/cgi-bin/mailman/listinfo/flang-dev
 >

_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: OpenMP in Flang and Clang

Fangrui Song via cfe-dev
In reply to this post by Fangrui Song via cfe-dev

On 4/14/20 9:58 AM, David Truby wrote:
 > I agree that we should align with clang's enums here in principle,
 > however my preference would still be to merge that patch so that we
 > have the checks and tests implemented as a first step, and then take a
 > more holistic look at aligning with clang on OpenMP semantic checks.

Agreed. I do not want to block your patch for this.


 > Another consideration is that clang currently tracks OpenMP 5 (and
 > only that specification) whereas we've been implementing 4.5 up to
 > now. When we try and share semantics checks with clang we will
 > probably want to do a more general version bump to OpenMP 5 at around
 > the same time, otherwise our semantics checks and our parser won't
 > match with regards to which version of the specification they
 > implement.

So this is not that simple. Clang defaults to 4.5 and has parts of 5.0
and 5.1 implemented. I suspect the situation would be similar in Flang
at some point soon. Since Clang supports to choose a version, in both
directions from the default, we already require extra logic to deal with
semantic changes. However, centralizing things and de-duplicating will
still help us. One of the latest changes in this regard was D77113. We
now explicitly specify in the macro file  what clause is available for
a directive and version (range).

I imagine C/C++/Fortran to be another degree of freedom here, as well as
the "only once" flag Flang already has. Maybe the latter should even be
extended to constraints on the set of clauses for a directive.

I hope this makes some sense.

Cheers,
   Johannes


 > Thanks
 > David Truby
 > ________________________________
 > From: flang-dev <[hidden email]> on behalf of Steve
Scalpone via flang-dev <[hidden email]>
 > Sent: 14 April 2020 15:02
 > To: Johannes Doerfert <[hidden email]>;
[hidden email] <[hidden email]>
 > Cc: [hidden email] <[hidden email]>
 > Subject: Re: [flang-dev] [cfe-dev] OpenMP in Flang and Clang
 >
 > Flang today has overlapping enums for OpenMP directives,
 > e.g. OmpLoopDirective and OmpDirective; both of these enums
 > have names for PARALLEL DO, for example.
 >
 > Perhaps before https://reviews.llvm.org/D77812 lands we
 > should merge the multiple sets of flang enumerations with
 > the other enumerations as Johannes describes.
 >
 > On 4/9/20, 9:55 AM, "cfe-dev on behalf of Johannes Doerfert via
cfe-dev" <[hidden email] on behalf of
[hidden email]> wrote:
 >
 >     External email: Use caution opening links or attachments
 >
 >
 >     Since we now merged F18, yay!, we should start reusing OpenMP
 >     "infrastructure" between LLVM/Clang and LLVM/Flang. This will reduce
 >     duplication (coding & maintenance), improve testing, and thereby
should
 >     get us to OpenMP 5.X support faster.
 >
 >     I started to look at `parse-tree.h` and `openmp-parsers.cpp` which
 >     contain a lot of code dealing with the OpenMP "grammar". Arguably, we
 >     don't want the duplication with
 >        `llvm/include/llvm/Frontend/OpenMP/OMPKinds.def`
 >     (and the almost empty "old" clang file `OpenMPKinds.def`).
 >
 >     I think there are multiple steps we should take. Note that I have to
 >     list them in some order, please don't interpret this as the order I
 >     suggest we work on them.
 >
 >        Move and merge the ENUM_CLASS definitions that can be shared into
 >        OMPKinds.def. This might require to extend the macros such that we
 >        have a flag for C/C++ or Fortran only but that should not be
too hard.
 >        The existing Clang code (or better OpenMPIRBuilder by now) could
 >        benefit from new enum classes already in Flang, e.g.
OmpCancelType,
 >        while things like the kind of an OmpScheduleClause should only be
 >        defined once.
 >
 >        Determine how we can merge the definitions in `parse-tree.h`
with the
 >        ones in `OpenMPClauses.h`. I was hoping to trim down the
latter anyway
 >        as it seems there is quite a bit of duplication in there. I
was also
 >        hoping we could generate them from the macro file (or a table-gen
 >        file) so we don't have to write all the boilerplate when new
 >        directives and clauses are added. As an example, in the new OpenMP
 >        context selector support for Clang new selector sets,
selectors, and
 >        traits can be added via macros in the OMPKinds.def file.
Boilerplate
 >        like parsing, pretty printing, error messages, will work right
away.
 >        Only if the existing logic to determine if the selector matches a
 >        context is not sufficient you need to modify one more place.
 >        (This is not true for implementation defined extensions that
change
 >         the behavior of the entire thing but anyway.)
 >
 >        Rethink the use of `std::tuple` in and `std::variant` in (the
OpenMP
 >        parts) of parse-tree.h. The latter is used for OmpLinearClause for
 >        example. As far as I can tell, the two members of the variant
have two
 >        common fields and one has a third, an enum. In clang this is
usually
 >        solved by having an "unknown" or "none" member in the enum which
 >        allows to remove the duplication and variant completely. I
would like
 >        to replace tuples so we can give their constitutes proper names. I
 >        think a similar point was made before when we discussed
tooling with
 >        (or extending) Flang.
 >
 >     I hope to get some feedback and thoughts on this.
 >
 >     Thanks,
 >        Johannes
 >
 >     _______________________________________________
 >     cfe-dev mailing list
 >     [hidden email]
 >     https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
 >
 >
 >
-----------------------------------------------------------------------------------
 > This email message is for the sole use of the intended recipient(s)
and may contain
 > confidential information.  Any unauthorized review, use, disclosure
or distribution
 > is prohibited.  If you are not the intended recipient, please contact
the sender by
 > reply email and destroy all copies of the original message.
 >
-----------------------------------------------------------------------------------
 > _______________________________________________
 > flang-dev mailing list
 > [hidden email]
 > https://lists.llvm.org/cgi-bin/mailman/listinfo/flang-dev

_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: [flang-dev] OpenMP in Flang and Clang

Fangrui Song via cfe-dev
In reply to this post by Fangrui Song via cfe-dev

 "> We should also switch to the OpenMP module and header in llvm. This
 > was previously discussed in the link below.
 > https://github.com/flang-compiler/f18/pull/685#discussion_r317987276

I'm unsure how this ties in, sorry."

This is in the same spirit of "Now that we are in LLVM, we should reuse or share LLVM code/components" and this is related to OpenMP. Agree that this is not about sharing between Clang and Flang.

--Kiran


From: Johannes Doerfert <[hidden email]>
Sent: 15 April 2020 03:07
To: Kiran Chandramohan <[hidden email]>; [hidden email] <[hidden email]>
Cc: [hidden email] <[hidden email]>
Subject: Re: [flang-dev] OpenMP in Flang and Clang
 

On 4/14/20 10:23 AM, Kiran Chandramohan wrote:
 > Thanks Johannes for your mail. Yes, we should try to share code with
 > Clang for the OpenMP portion.
 >
 > Of the options you suggest (macrofile vs tablegen) my preference will
 > be to write the structure and restrictions of OpenMP constructs and
 > clauses as a tablegen file. This can be then used to generate the
 > checks and definitions for OpenMP constructs and clauses for Clang and
 > Flang as is there currently today.

I agree, tablegen is unfortunately the better choice. I did extend on
the macro approach started by Clang so far as it is "simpler". However,
tablegen provides more flexibility. We can not only generate checks,
codegen, etc. but later use the formulation to create tests as well.
Granted, those tests have the same systematic errors as clang/flang, but
assuming we can validate them against other compilers we should be able
to cover quite a substantial subset of the semantics in a maintainable
way. I mean, if something changes we regenerate the tests. We can
include the tablegen rules that went into a test as well, etc.


 > We should also switch to the OpenMP module and header in llvm. This
 > was previously discussed in the link below.
 > https://github.com/flang-compiler/f18/pull/685#discussion_r317987276

I'm unsure how this ties in, sorry.

Cheers,
   Johannes

 >
 > --Kiran
 >
 > ________________________________
 > From: flang-dev <[hidden email]> on behalf of
Johannes Doerfert via flang-dev <[hidden email]>
 > Sent: 09 April 2020 17:46
 > To: [hidden email] <[hidden email]>
 > Cc: [hidden email] <[hidden email]>
 > Subject: [flang-dev] OpenMP in Flang and Clang
 >
 > Since we now merged F18, yay!, we should start reusing OpenMP
 > "infrastructure" between LLVM/Clang and LLVM/Flang. This will reduce
 > duplication (coding & maintenance), improve testing, and thereby should
 > get us to OpenMP 5.X support faster.
 >
 > I started to look at `parse-tree.h` and `openmp-parsers.cpp` which
 > contain a lot of code dealing with the OpenMP "grammar". Arguably, we
 > don't want the duplication with
 >    `llvm/include/llvm/Frontend/OpenMP/OMPKinds.def`
 > (and the almost empty "old" clang file `OpenMPKinds.def`).
 >
 > I think there are multiple steps we should take. Note that I have to
 > list them in some order, please don't interpret this as the order I
 > suggest we work on them.
 >
 >    Move and merge the ENUM_CLASS definitions that can be shared into
 >    OMPKinds.def. This might require to extend the macros such that we
 >    have a flag for C/C++ or Fortran only but that should not be too hard.
 >    The existing Clang code (or better OpenMPIRBuilder by now) could
 >    benefit from new enum classes already in Flang, e.g. OmpCancelType,
 >    while things like the kind of an OmpScheduleClause should only be
 >    defined once.
 >
 >    Determine how we can merge the definitions in `parse-tree.h` with the
 >    ones in `OpenMPClauses.h`. I was hoping to trim down the latter anyway
 >    as it seems there is quite a bit of duplication in there. I was also
 >    hoping we could generate them from the macro file (or a table-gen
 >    file) so we don't have to write all the boilerplate when new
 >    directives and clauses are added. As an example, in the new OpenMP
 >    context selector support for Clang new selector sets, selectors, and
 >    traits can be added via macros in the OMPKinds.def file. Boilerplate
 >    like parsing, pretty printing, error messages, will work right away.
 >    Only if the existing logic to determine if the selector matches a
 >    context is not sufficient you need to modify one more place.
 >    (This is not true for implementation defined extensions that change
 >     the behavior of the entire thing but anyway.)
 >
 >    Rethink the use of `std::tuple` in and `std::variant` in (the OpenMP
 >    parts) of parse-tree.h. The latter is used for OmpLinearClause for
 >    example. As far as I can tell, the two members of the variant have two
 >    common fields and one has a third, an enum. In clang this is usually
 >    solved by having an "unknown" or "none" member in the enum which
 >    allows to remove the duplication and variant completely. I would like
 >    to replace tuples so we can give their constitutes proper names. I
 >    think a similar point was made before when we discussed tooling with
 >    (or extending) Flang.
 >
 > I hope to get some feedback and thoughts on this.
 >
 > Thanks,
 >    Johannes
 >
 > _______________________________________________
 > flang-dev mailing list
 > [hidden email]
 > https://lists.llvm.org/cgi-bin/mailman/listinfo/flang-dev
 >


_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev