jyu2 added inline comments.

================
Comment at: clang/lib/Sema/SemaExprMember.cpp:1873-1877
+    if (auto *PrivateCopy =
+            isOpenMPFDCaptureDecl(Field, Base.get(), IsArrow, OpLoc, &SS,
+                                  /*TemplateKWLoc=*/SourceLocation(), Field,
+                                  FoundDecl, /*HadMultipleCandidates=*/false,
+                                  MemberNameInfo, MemberType, VK, OK))
----------------
ABataev wrote:
> Why do we need this function? Implicit private rule should apply (if should) 
> only to this-Юашдув kind of expressions, if something like parent.field 
> expression is used, parent should be private, not private.field. Or I'm 
> missing something?
You are right for "parent.a" only privatize "parent".

But if 'a' is a field access in a member function then 'a' is privatized, not 
"this".  It is same with firstprivate(a).  But for the explicit 
firstprivate(a), the capture is build in ActOnOpenMPFirstprivateClause, so it 
can be mapped to reference in the omp region.  For Implicit, I need to build 
capture for the first reference in the omp region with defalut(first|private) 
is used.  And used that to generate firstprivate clause at end when the call to 
ActOnOpenMPFirstprivateClause when generating implicit clause.

Is this reasonable?


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:201
+    using ImplicitFDCapTy = std::pair<ImpilitFDLevelTy, VarDecl *>;
+    // List of captuer fields
+    llvm::SmallVector<ImplicitFDCapTy, 8> ImplicitDefaultFirstprivateFDs;
----------------
ABataev wrote:
> ///
Thanks Changed


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:2410
         D,
-        [](OpenMPClauseKind C, bool AppliedToPointee) {
+        [](OpenMPClauseKind C, bool AppliedToPointee, ...) {
           return isOpenMPPrivate(C) && !AppliedToPointee;
----------------
ABataev wrote:
> Better to add an actual param type here but without Param name
Changed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127803

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

Reply via email to