sfantao added a comment.

Hi Alexey,

Thanks for the review!


================
Comment at: lib/CodeGen/CGStmtOpenMP.cpp:3410-3411
@@ +3409,4 @@
+  llvm::DenseSet<const VarDecl *> EmittedAsFirstprivate;
+  CGCapturedStmtInfo CapturesInfo(cast<CapturedStmt>(*D.getAssociatedStmt()));
+  for (const auto *C : D.getClausesOfKind<OMPUseDevicePtrClause>()) {
+    auto OrigVarIt = C->varlist_begin();
----------------
ABataev wrote:
> EmitOMPUseDevicePtrClause() should emit the code for single clause. The outer 
> loop must be used in directive emission function.
Ok ,makes sense, Doing as you say.

================
Comment at: lib/CodeGen/CGStmtOpenMP.cpp:3477-3495
@@ +3476,21 @@
+  // Codegen with device pointer privatization.
+  auto &&PvtCodeGen = [&S, &Info](CodeGenFunction &CGF, PrePostActionTy &) {
+    auto &&CodeGen = [&S, &Info](CodeGenFunction &CGF, PrePostActionTy &) {
+      OMPPrivateScope PrivateScope(CGF);
+      CGF.EmitOMPUseDevicePtrClause(S, PrivateScope, 
Info.CaptureDeviceAddrMap);
+      (void)PrivateScope.Privatize();
+      CGF.EmitStmt(
+          cast<CapturedStmt>(S.getAssociatedStmt())->getCapturedStmt());
+    };
+    // Notwithstanding the body of the region is emitted as inlined directive,
+    // we don't use an inline scope as changes in the references inside the
+    // region are expected to be visible outside, so we do not privative them.
+    OMPLexicalScope Scope(CGF, S);
+    CGF.CGM.getOpenMPRuntime().emitInlinedDirective(CGF, OMPD_target_data,
+                                                    CodeGen);
+  };
+
+  // Codegen with no device pointer privatization.
+  auto &&NoPvtCodeGen = [&S, &Info](CodeGenFunction &CGF, PrePostActionTy &) {
+    auto &&CodeGen = [&S, &Info](CodeGenFunction &CGF, PrePostActionTy &) {
+      CGF.EmitStmt(
----------------
ABataev wrote:
> PvtCodeGen and NoPvtCodeGen are pretty similar. Could you merge them somehow 
> into the one? Maybe some custom PrePostAction could be used here?
I replaced this code by a single region codegen controlling the privatization 
with a flag and a pre-action. Let me know if this what you had in mind.

================
Comment at: lib/Sema/SemaOpenMP.cpp:11858
@@ +11857,3 @@
+    // similar properties of a first private variable.
+    DSAStack->addDSA(D, RefExpr->IgnoreParens(), OMPC_firstprivate, Ref);
+
----------------
ABataev wrote:
> Are the restrictions for firstprivates applied also? What if inner directive 
> has some clauses that cannot be used with variables previously marked as 
> firstprivates, but can be used with variables, used in use_device_ptr?
I was revisiting the spec and I couldn't find any conflict with the 
`firstprivate` restrictions. There are conflicts for variables used in a 
firstprivate clause that are also used in clauses of enclosing worksharing 
constructs but not the other way around. Note that we are using the 
firstprivate attribute but do not do all the firstprivate checks, so the 
conflicts would only arise from other enclosed constructs, not other enclosing 
constructs. Let me know if you have any specific case in mind or if you'd like 
me to do this differently.  


https://reviews.llvm.org/D22691



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to