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

Thu Jan 3 14:24:52 PST 2019

jdenny added a comment.

In D56113#1345529 <https://reviews.llvm.org/D56113#1345529>, @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.
>
>
> I'm planning to let this affect the behavior of `default(none)` (predetermined shared means no explicit attribute is needed).
>
> I don't plan to let it affect which version of the diagnostics are produced.  I think the newer diagnostics are clearer even though they are not expressed precisely in terms of 3.1 semantics.

I missed something: 3.1 restricts const and not mutable variables for lastprivate and private too.  However, clang didn't previously implement those restrictions probably because the check for conflicts with predetermined shared was sufficient to catch those cases.

So, for 3.1, it should be correct for clang to report either diagnostic, but again I think the new diagnostic is clearer.  For 4.0 and later, only the new diagnostic makes sense.

I had already updated all the tests for the new diagnostic, so it's easiest to keep the new diagnostic for all versions.  Moreover, it means less cases to check in the test suite.

Of course, I can adjust if this doesn't make sense to you.

Repository:
  rC Clang

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

https://reviews.llvm.org/D56113