ABataev added inline comments.
================
Comment at: clang/include/clang/Sema/Sema.h:11083
+ bool CheckLastPrivateForMappedDirectives(ArrayRef<OMPClause *> Clauses);
+
----------------
`checkLastPrivateForMappedDirectives`
================
Comment at: clang/lib/AST/StmtPrinter.cpp:745-746
+ OpenMPDirectiveKind MappedDirective = Node->getMappedDirective();
+ if (MappedDirective != llvm::omp::OMPD_unknown &&
+ MappedDirective == llvm::omp::OMPD_loop) {
+ Indent() << "#pragma omp loop bind(thread)";
----------------
Just `MappedDirective == llvm::omp::OMPD_loop`
================
Comment at: clang/lib/AST/StmtPrinter.cpp:746-748
+ MappedDirective == llvm::omp::OMPD_loop) {
+ Indent() << "#pragma omp loop bind(thread)";
+ } else
----------------
Remove braces
================
Comment at: clang/lib/AST/StmtPrinter.cpp:765-766
+ OpenMPDirectiveKind MappedDirective = Node->getMappedDirective();
+ if (MappedDirective != llvm::omp::OMPD_unknown &&
+ MappedDirective == llvm::omp::OMPD_loop) {
+ Indent() << "#pragma omp loop bind(parallel)";
----------------
Just `MappedDirective == llvm::omp::OMPD_loop`
================
Comment at: clang/lib/AST/StmtPrinter.cpp:766-768
+ MappedDirective == llvm::omp::OMPD_loop) {
+ Indent() << "#pragma omp loop bind(parallel)";
+ } else
----------------
Remove braces
================
Comment at: clang/lib/AST/StmtPrinter.cpp:1005-1006
+ OpenMPDirectiveKind MappedDirective = Node->getMappedDirective();
+ if (MappedDirective != llvm::omp::OMPD_unknown &&
+ MappedDirective == llvm::omp::OMPD_loop) {
+ Indent() << "#pragma omp loop bind(teams)";
----------------
Just `MappedDirective == llvm::omp::OMPD_loop`
================
Comment at: clang/lib/AST/StmtPrinter.cpp:1006-1008
+ MappedDirective == llvm::omp::OMPD_loop) {
+ Indent() << "#pragma omp loop bind(teams)";
+ } else
----------------
Remove braces
================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:343
+ /// This may also be used in a similar way for other constructs.
+ OpenMPDirectiveKind MappedDirective = OMPD_unknown;
+
----------------
I assume it shall be a member of SharingMapType, otherwise there might be
problems with handling of enclosed constructs.
================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:646
+ void setCurrentDirective(OpenMPDirectiveKind NewDK) {
+ SharingMapTy *Top = (SharingMapTy *)getTopOfStackOrNull();
+ assert(Top != NULL &&
----------------
Why need a cast here?
================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:647-648
+ SharingMapTy *Top = (SharingMapTy *)getTopOfStackOrNull();
+ assert(Top != NULL &&
+ "Before calling setCurrentDirective Top of Stack not to be NULL.");
+ // Store the old into MappedDirective & assign argument NewDK to Directive.
----------------
use `nullptr` or just `assert(Top &&`
================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:6140-6201
+ llvm::SmallVector<OMPClause *, 8> ClausesWithoutBind;
+ bool UseClausesWithoutBind = false;
+
+ // Restricting to "#pragma omp loop bind"
+ if (Kind == OMPD_loop) {
+ if (BindKind == OMPC_BIND_unknown) {
+ // Setting the enclosing teams or parallel construct for the loop
----------------
Could you outline this new code as a separate function?
================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:10419
setFunctionHasBranchProtectedScope();
- return OMPSimdDirective::Create(Context, StartLoc, EndLoc, NestedLoopCount,
- Clauses, AStmt, B);
+ OMPSimdDirective *tmp = OMPSimdDirective::Create(
+ Context, StartLoc, EndLoc, NestedLoopCount, Clauses, AStmt, B);
----------------
tmp is bad name. Give better name and follow LLVM coding style recommendations.
================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:10461
setFunctionHasBranchProtectedScope();
- return OMPForDirective::Create(
+ OMPForDirective *tmp = OMPForDirective::Create(
Context, StartLoc, EndLoc, NestedLoopCount, Clauses, AStmt, B,
----------------
Same
================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:10474
+ /* Check for syntax of lastprivate */
+ if (DSAStack->getMappedDirective() == OMPD_loop) {
----------------
Use c++ style of comments
================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:10475-10476
+ /* Check for syntax of lastprivate */
+ if (DSAStack->getMappedDirective() == OMPD_loop) {
+ if (checkGenericLoopLastprivate(*this, Clauses, OMPD_loop, DSAStack))
+ return StmtError();
----------------
`if (DSAStack->getMappedDirective() == OMPD_loop &&
checkGenericLoopLastprivate(*this, Clauses, OMPD_loop, DSAStack))`. Avoid
structured complexity.
================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:13993
+
+ /* Check for syntax of lastprivate */
+ if (DSAStack->getMappedDirective() == OMPD_loop) {
----------------
Same, c++
================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:13994-13995
+ /* Check for syntax of lastprivate */
+ if (DSAStack->getMappedDirective() == OMPD_loop) {
+ if (checkGenericLoopLastprivate(*this, Clauses, OMPD_loop, DSAStack))
+ return false;
----------------
Same, avoid complexity
================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:13994-13998
+ if (DSAStack->getMappedDirective() == OMPD_loop) {
+ if (checkGenericLoopLastprivate(*this, Clauses, OMPD_loop, DSAStack))
+ return false;
+ }
+ return true;
----------------
ABataev wrote:
> Same, avoid complexity
`return DSAStack->getMappedDirective() == OMPD_loop &&
checkGenericLoopLastprivate(*this, Clauses, OMPD_loop, DSAStack);
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D144634/new/
https://reviews.llvm.org/D144634
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
