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

jdenny added a comment. Thanks for the review! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56113/new/ https://reviews.llvm.org/D56113 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

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

ABataev accepted this revision. ABataev added a comment. This revision is now accepted and ready to land. LG Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56113/new/ https://reviews.llvm.org/D56113 ___ cfe-commits

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

ABataev added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:1115 + +static bool isConstNotMutableType(Sema , ValueDecl *D) { + return checkConstNotMutableType(SemaRef, D, /*ReportDiag*/ false, I would say it is better to outlined check as a

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

jdenny marked an inline comment as done. jdenny added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:9736 +static bool rejectConstNotMutableType(Sema , ValueDecl *D, + OpenMPClauseKind CKind, ABataev wrote: >

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

ABataev added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:9736 +static bool rejectConstNotMutableType(Sema , ValueDecl *D, + OpenMPClauseKind CKind, ABataev wrote: > This function and the original code has a

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

ABataev added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:9736 +static bool rejectConstNotMutableType(Sema , ValueDecl *D, + OpenMPClauseKind CKind, This function and the original code has a lot of common

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

jdenny added a comment. In D56113#1345529 , @jdenny wrote: > In D56113#1345238 , @ABataev wrote: > > > >>> By the way, is there any value to keeping the predetermined shared for > > >>> const if

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

ABataev added a comment. In D56113#1345588 , @jdenny wrote: > In D56113#1345578 , @ABataev wrote: > > > >>> By the way, LangOpts.OpenMP currently defaults to 0. Should it? > > >> > > >> Yes, it means it is

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

jdenny added a comment. In D56113#1345578 , @ABataev wrote: > >>> 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

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

ABataev added a comment. In D56113#1345564 , @jdenny wrote: > In D56113#1345535 , @ABataev wrote: > > > In D56113#1345529 , @jdenny wrote: > > > > > In D56113#1345238

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

jdenny added a comment. In D56113#1345535 , @ABataev wrote: > In D56113#1345529 , @jdenny wrote: > > > In D56113#1345238 , @ABataev wrote: > > > > > >>> By the way, is there

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

ABataev added a comment. In D56113#1345529 , @jdenny wrote: > In D56113#1345238 , @ABataev wrote: > > > >>> By the way, is there any value to keeping the predetermined shared for > > >>> const if

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

jdenny added a comment. In 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

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

ABataev added a comment. In D56113#1345372 , @jdenny wrote: > In D56113#1345337 , @ABataev wrote: > > > > In D56113#1345238 , @ABataev > > > wrote: > > > > > >> In

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

jdenny added a comment. In D56113#1345337 , @ABataev wrote: > > In D56113#1345238 , @ABataev wrote: > > > >> In D56113#1345232 , @jdenny wrote: > >> > >> > In

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

ABataev added a comment. In D56113#1345336 , @jdenny wrote: > In D56113#1345238 , @ABataev wrote: > > > >>> By the way, is there any value to keeping the predetermined shared for > > >>> const if

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

jdenny added a comment. In 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

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

ABataev added a comment. In D56113#1345232 , @jdenny wrote: > In D56113#1345047 , @ABataev wrote: > > > In D56113#1344210 , @jdenny wrote: > > > > > In D56113#1342879

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

jdenny added a comment. In D56113#1345047 , @ABataev wrote: > In D56113#1344210 , @jdenny wrote: > > > In D56113#1342879 , @ABataev wrote: > > > > > But you will need

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

ABataev added a comment. In D56113#1344210 , @jdenny wrote: > In D56113#1342879 , @ABataev wrote: > > > But you will need another serie of patches for `reduction` and `linear` > > clauses to update their error

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

jdenny marked 2 inline comments as done. jdenny added a comment. In 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

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

ABataev added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:9702 +static bool RejectConstNotMutableType(Sema , ValueDecl *D, + OpenMPClauseKind CKind, Functions must start from the small letters.

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

ABataev added a comment. In D56113#1342878 , @jdenny wrote: > Hi Alexey, > > > In D56113#1342076 , @jdenny wrote: > > > >> In D56113#1341940 , @ABataev > >> wrote: > >> >

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

jdenny added a comment. Hi Alexey, > In D56113#1342076 , @jdenny wrote: > >> In D56113#1341940 , @ABataev wrote: >> >> > The patch does not seem quite correct. I committed another fix for this >> > problem. >> >>

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

ABataev added a comment. In D56113#1342076 , @jdenny wrote: > In D56113#1341940 , @ABataev wrote: > > > The patch does not seem quite correct. I committed another fix for this > > problem. > > > Thanks for the

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

jdenny added a comment. In D56113#1341940 , @ABataev wrote: > The patch does not seem quite correct. I committed another fix for this > problem. Thanks for the fix, r350127, which does seem to address the use cases I care about right now. However,

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

ABataev added a comment. The patch does not seem quite correct. I committed another fix for this problem. There is still the problem with the explicitly specified `shared` clause on `target teams` directive, but I don't think that it must cause some troubles. The variable still should not be