Re: [PATCH] D14940: [OpenMP] Update target directive codegen to use 4.5 implicit data mappings.

[Samuel Antao via cfe-commits]

sfantao added a comment.

Hi Alexey,

Thanks for the review!



Comment at: include/clang/AST/Stmt.h:2018-2020
@@ -2016,2 +2017,5 @@
 break;
+  case VCK_ByCopy:
+assert(Var && "capturing by copy must have a variable!");
+break;
   case VCK_VLAType:

ABataev wrote:
> Also add assertion that only pointers anmd scalars are allowed to be captured 
> by value
Done! Note however that VarDecl was an incomplete type here. So I moved the 
code to Stmt.cpp.


Comment at: lib/Sema/SemaOpenMP.cpp:818-819
@@ +817,4 @@
+// captures in that those cases.
+IsByRef = isa(Ty) || isa(Ty) ||
+  isa(Ty);
+  }

ABataev wrote:
> Wow. I think it is just enough to check !Ty->isScalarType().
Oh, right. Changed that to use `isScalarType`.


http://reviews.llvm.org/D14940



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

Re: [PATCH] D14940: [OpenMP] Update target directive codegen to use 4.5 implicit data mappings.

[Alexey Bataev via cfe-commits]

ABataev added inline comments.


Comment at: include/clang/AST/Stmt.h:2018-2020
@@ -2016,2 +2017,5 @@
 break;
+  case VCK_ByCopy:
+assert(Var && "capturing by copy must have a variable!");
+break;
   case VCK_VLAType:

Also add assertion that only pointers anmd scalars are allowed to be captured 
by value


Comment at: lib/CodeGen/CGStmtOpenMP.cpp:139-140
@@ -112,2 +138,4 @@
   I = S.captures().begin();
-  for (auto *FD : RD->fields()) {
+  for (auto FI = RD->field_begin(), FE = RD->field_end(); FI != FE;
+   ++FI, ++Cnt, ++I) {
+auto *FD = *FI;

Turn back to range-based loop


Comment at: lib/Sema/SemaOpenMP.cpp:818-819
@@ +817,4 @@
+// captures in that those cases.
+IsByRef = isa(Ty) || isa(Ty) ||
+  isa(Ty);
+  }

Wow. I think it is just enough to check !Ty->isScalarType().


http://reviews.llvm.org/D14940



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