[PATCH] [OPENMP] Fixed codegen for arrays in 'copyprivate' clause.

[Alexey Bataev]

John, thanks for the review!


================
Comment at: lib/CodeGen/CGOpenMPRuntime.h:362
@@ -360,1 +361,3 @@
+               const VarDecl * /*DestVD*/, const VarDecl * /*SrcVD*/,
+               const Expr * /*AssignOp*/)> &SingleCopyprivateCodeGen);
 
----------------
rjmccall wrote:
> You can just leave these parameter names there without commenting them out.
> 
> Should SingleOpGen also be a llvm::function_ref?
I think I'd better create a corresponding method in CodeGenFunction and call it instead of passing function reference as an argument. The same function is used in many places in CodeGenFunction and in CGOpenMPRuntime.

================
Comment at: lib/CodeGen/CGStmtOpenMP.cpp:117
@@ +116,3 @@
+
+  // Shift the address back by one element.
+  auto NegativeOne = llvm::ConstantInt::get(SizeTy, -1, true);
----------------
rjmccall wrote:
> Is there a reason why this is back-to-front instead of front-to-back?  In C++, arrays are generally initialized front-to-back, so that when destruction goes back-to-front it happens in reverse order of construction.
I'll rework this

================
Comment at: lib/CodeGen/CGStmtOpenMP.cpp:162
@@ +161,3 @@
+          auto *AssignExpr = VD->getInit();
+          if (!isa<CXXConstructExpr>(AssignExpr) ||
+              isTrivialInitializer(AssignExpr)) {
----------------
rjmccall wrote:
> If this is an initialization expression, please don't call it an assignment expression.  If it's an assignment expression, checking for CXXConstructExpr is not going to work.
Ok, renamed to Init

================
Comment at: lib/CodeGen/CGStmtOpenMP.cpp:173
@@ +172,3 @@
+                                           llvm::Value *SrcElement) {
+                  // Create RunCleanScope to cleanup possible temps.
+                  RunCleanupsScope Init(*this);
----------------
rjmccall wrote:
> It's called "RunCleanupsScope", but that part of the comment doesn't add much anyway.  I'd say something like "Clean up any temporaries needed by the assignment expression."
Changed to "Clean up any temporaries needed by the initialization"

================
Comment at: lib/CodeGen/CGStmtOpenMP.cpp:973
@@ +972,3 @@
+      *this,
+      [&]() -> void {
+        InlinedOpenMPRegionScopeRAII Region(*this, S);
----------------
rjmccall wrote:
> You don't need the "()->void", so unless you find it helpful, it's best to leave it off.
Removed.

http://reviews.llvm.org/D8914

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/