[PATCH] D56113: [OpenMP] Replace predetermined shared for const variable

Alexey Bataev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Jan 3 10:22:29 PST 2019


ABataev added a comment.

In D56113#1345336 <https://reviews.llvm.org/D56113#1345336>, @jdenny wrote:

> In D56113#1345238 <https://reviews.llvm.org/D56113#1345238>, @ABataev wrote:
>
> > >>> By the way, is there any value to keeping the predetermined shared for const if -openmp-version=3.1 or earlier?
> > >> 
> > >> Yes, you can check for the value of `LangOpts.OpenMP`. For OpenMP 3.1 it will have the value `31`.
> > > 
> > > How far back should we take this?  I'm inclined to check for `30` and `31` only and assume anything else is newer, but let me know if we need to check for earlier versions.
> >
> > I think `<= 31` is good. Clang always supported only OpenMP 3.1 and higher.
>
>
> Ah, for some reason I assumed it was a string not an integer.
>
> In D56113#1345238 <https://reviews.llvm.org/D56113#1345238>, @ABataev wrote:
>
> > In D56113#1345232 <https://reviews.llvm.org/D56113#1345232>, @jdenny wrote:
> >
> > > In D56113#1345047 <https://reviews.llvm.org/D56113#1345047>, @ABataev wrote:
> > >
> > > > In D56113#1344210 <https://reviews.llvm.org/D56113#1344210>, @jdenny wrote:
> > > >
> > > > > In D56113#1342879 <https://reviews.llvm.org/D56113#1342879>, @ABataev wrote:
> > > > >
> > > > > > But you will need another serie of patches for `reduction` and `linear` clauses to update their error messages.
> > > > >
> > > > >
> > > > > Those already had their own checks for const.  I'm not aware of any cases not handled, and the test suite does pass after my patch.
> > > >
> > > >
> > > > Yes, they have the checks for constness, but you need to  update those checks to emit this new error message for const values with mutable fields.
> > >
> > >
> > > I believe you mean we should reuse `rejectConstNotMutableType` for `reduction` and `linear` rather than leave some of its implementation duplicated for those.
> >
>
>
> For reductions, this will change the message from "const-qualified list item cannot be reduction" to "const-qualified variable cannot be reduction".  Is that OK?  (There are many tests to update, so I want to ask first.)


Mmmm. For the reductions better to keep the original message, because the list items also might be the array sections, not only the variables.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56113/new/

https://reviews.llvm.org/D56113





More information about the cfe-commits mailing list