[PATCH] D70726: [OpenMP50] Add parallel master construct

[Chi Chun Chen via Phabricator via cfe-commits]

cchen marked 2 inline comments as done.
cchen added inline comments.


================
Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:2933
+    OMPPrivateScope PrivateScope(CGF);
+    bool Copyins = CGF.EmitOMPCopyinClause(S);
+    (void)CGF.EmitOMPFirstprivateClause(S, PrivateScope);
----------------
ABataev wrote:
> Do we need copyins here at all? We have only master thread active here in this construct.
I think we still need to emit copyins since the assignment to the threadprivate copies of the other threads must be observable after the parallel region.


================
Comment at: clang/test/OpenMP/nesting_of_regions.cpp:2999
+  {
+#pragma omp simd
+    for (int i = 0; i < 10; ++i)
----------------
ABataev wrote:
> Are simds allowed in master?
I didn't see restriction about simds in master and there is also a test under "MASTER DIRECTIVE" allows simd directive inside master construct.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70726