jdenny added a comment. In D56113#1345535 <https://reviews.llvm.org/D56113#1345535>, @ABataev wrote:
> 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. Moreover, there are > > less cases to test this way. Let me know if you think this is wrong. If > > you want to review the updated patch first, I'll be posting it soon. > > > It would be good if could keep the original implementation for 3.1. Should I parameterize all tests affected by this change to test both sets of diagnostics? Or should we pick one version and just add a few additional tests for the other? (I'm sorry to ask so many questions, but again there are many tests to update.) >> By the way, LangOpts.OpenMP currently defaults to 0. Should it? > > Yes, it means it is disabled by default. Ah, it becomes 1 when we have `-fopenmp` but not `-fopenmp-version`. But still `LangOpts.OpenMP <= 31` is true by default, so the new behavior would be off by default. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56113/new/ https://reviews.llvm.org/D56113 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
