ABataev added inline comments.
================ Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:9792 + "construct">; +def error_loop_reduction_clause : Error< + "reduction clause not handled with '#pragma omp loop bind(teams)'">; ---------------- err_omp_... ================ Comment at: clang/lib/Sema/SemaOpenMP.cpp:6146 + + const OpenMPDirectiveKind parentDirective = + DSAStack->getParentDirective(); ---------------- ParentDirective ================ Comment at: clang/lib/Sema/SemaOpenMP.cpp:6167-6168 + // Spec restriction : bind(teams) and reduction not permitted. + if ((BindKind == OMPC_BIND_teams) && + (C->getClauseKind() == llvm::omp::Clause::OMPC_reduction)) + Diag(SourceLocation(), diag::error_loop_reduction_clause); ---------------- Remove extra parens ================ Comment at: clang/lib/Sema/SemaOpenMP.cpp:6173-6175 + if (C->getClauseKind() != llvm::omp::Clause::OMPC_bind) { + ClausesWithoutBind.push_back(C); + } ---------------- Remove extra braces ================ Comment at: clang/lib/Sema/SemaOpenMP.cpp:6181-6189 + DSAStack->setCurrentDirective(OMPD_for); + break; + case OMPC_BIND_teams: + Kind = OMPD_distribute; + DSAStack->setCurrentDirective(OMPD_distribute); + break; + case OMPC_BIND_thread: ---------------- ABataev wrote: > 1. You're overriding the directive kind here and do not restore it then. It > may cause the compiler crash. > 2. I think you need to here to create a new OpenMP region rather than > overriding the existing one. I see why you're doing this now. Would be good to see some semantic tests, like nesting of regions, use of unsupported clauses (e.g. linear on loop bind(teams)), etc. ================ Comment at: clang/lib/Sema/SemaOpenMP.cpp:6199-6202 + if (UseClausesWithoutBind) { + ClausesWithImplicit.append(ClausesWithoutBind.begin(), + ClausesWithoutBind.end()); + } else { ---------------- Why need to drop bind clause here? ================ Comment at: clang/test/OpenMP/loop_bind_codegen.cpp:51-56 +void thread_loop2() { + #pragma omp loop bind(thread) + for (int j = 0 ; j < NNN ; j++) { + aaa[j] = j*NNN; + } +} ---------------- koops wrote: > ABataev wrote: > > I think it should trigger the assert in setCurrentDirective > I did not see any crash at this point. Can you please give me other examples > where I can see this crash? Ah, you're overriding the loop directive kind. 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