[PATCH] D79677: [Clang][OpenMP][OMPBuilder] (1/4) Privatize `parallel` for `OMPBuilder`

[Kiran Chandramohan via Phabricator via cfe-commits]

kiranchandramohan added a comment.

Is the ordering of code generation for clauses important?
copyin -> firstprivate -> barrier -> private



================
Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:1673
     //
     // TODO: This defaults to shared right now.
     auto PrivCB = [](InsertPointTy AllocaIP, InsertPointTy CodeGenIP,
----------------
Should this change in this patch?


================
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);
----------------
Nit: What do these three cases correspond to? A comment might be useful.


================
Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:1743
+      Builder.SetInsertPoint(&ContinuationBB);
+      PrivateScope.ForceCleanup();
+      Builder.Insert(ContTI);
----------------
Not a comment about this patch: While the context makes it clear, the name does not suggest that this function is emitting something.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79677/new/

https://reviews.llvm.org/D79677