fghanim marked 3 inline comments as done. fghanim added a comment. In D79677#2119019 <https://reviews.llvm.org/D79677#2119019>, @kiranchandramohan wrote:
> Is the ordering of code generation for clauses important? > copyin -> firstprivate -> barrier -> private if we emitted a copyin, then prob. yes. otherwise no. ================ Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:1673 // // TODO: This defaults to shared right now. auto PrivCB = [](InsertPointTy AllocaIP, InsertPointTy CodeGenIP, ---------------- kiranchandramohan wrote: > Should this change in this patch? No, still used for shared variables. but the todo should go away. ================ Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:1729-1736 + if (!OMP_Entry->getTerminator()) { + OMP_Entry->getInstList().push_back(EntryBI); + } else if (Builder.GetInsertBlock()->getTerminator()) { + EntryBI->dropAllReferences(); + EntryBI->deleteValue(); + } else { + Builder.Insert(EntryBI); ---------------- kiranchandramohan wrote: > Nit: What do these three cases correspond to? A comment might be useful. Just checks to see what to do with the terminator of the entry block. I'll add a comment in an update ================ Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:1743 + Builder.SetInsertPoint(&ContinuationBB); + PrivateScope.ForceCleanup(); + Builder.Insert(ContTI); ---------------- kiranchandramohan wrote: > Not a comment about this patch: While the context makes it clear, the name > does not suggest that this function is emitting something. you mean `ForceClean()`? it forces the emission of cleanup code - instead of just as part of the Privatescope Dctor. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79677/new/ https://reviews.llvm.org/D79677 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits