[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