jdoerfert added a comment.
This seems sensible. The clang parts look OK but I haven't thought everything
through to the last detail. I left some questions below, I guess we can go down
this road and slowly transition to the new scheme.
================
Comment at: clang/lib/AST/Stmt.cpp:1275
- ->isScalarType())) &&
- "captures by copy are expected to have a scalar type!");
break;
----------------
Why does this have to go? Is that avoidable?
================
Comment at: clang/lib/Sema/SemaExpr.cpp:17274
+ if (IsTopScope && Kind != Sema::TryCapture_Implicit) {
+ ByRef = (Kind == Sema::TryCapture_ExplicitByRef);
+ } else if (S.getLangOpts().OpenMP && RSI->CapRegionKind == CR_OpenMP) {
----------------
This doesn't impact anyone else negatively? I don't understand why we need this
now.
================
Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:305
+ /// Modifies the canonical loop to be a workshare loop.
+ CanonicalLoopInfo *createWorkshareLoop(const LocationDescription &Loc,
----------------
Nit: Add the argument description (from above).
================
Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:865
+ Function *getFunction() const { return Header->getParent(); }
+
----------------
Nit: documentation, also above.
================
Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:986
+ if (BodyGenCB)
+ BodyGenCB(CL->getBodyIP(), CL->getIndVar());
----------------
I'm unsure I understand when it would make sense to not have a body generator.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D94973/new/
https://reviews.llvm.org/D94973
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
