jdenny added inline comments.
================
Comment at: clang/include/clang/AST/StmtOpenMP.h:166
+ assert((isa<ForStmt>(S) || isa<CXXForRangeStmt>(S)) &&
+ "Canonical loop must be a for loop (range-based or otherwise)");
+ SubStmts[LOOPY_STMT] = S;
----------------
Meinersbur wrote:
> jdenny wrote:
> > To convert run-time errors into compile-time errors, what if `setLoopStmt`
> > is overloaded to take either a `ForStmt` or `CXXForRangeStmt` instead of
> > any `Stmt`?
> This would also require callers to call two different versions and just
> removes the problem up the call stack. For instance, StmtReader just receives
> a Stmt from ReadStmt. If we had setLoopStmt, we'd need a switch to call one
> of overloads.
OK, I checked the call stack for your setters this time. It looks like it's
just `create`, and I see that trying to achieve my goal would make a mess of
that call stack. Sorry for the noise.
================
Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:1872
+ LValue CapStruct = ParentCGF.InitCapturedStruct(*S);
+ CodeGenFunction CGF(ParentCGF.CGM, true);
+ std::unique_ptr<CodeGenFunction::CGCapturedStmtInfo> CSI =
----------------
================
Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:3497
+
+ bool NeedsBarrer = !S.getSingleClause<OMPNowaitClause>();
+ llvm::OpenMPIRBuilder &OMPBuilder =
----------------
================
Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:94
// been "emitted" to the outside, thus, modifications are still sensible.
- if (CGM.getLangOpts().OpenMPIRBuilder)
- CGM.getOpenMPRuntime().getOMPBuilder().finalize();
+ if (CGM.getLangOpts().OpenMPIRBuilder && CurFn)
+ CGM.getOpenMPRuntime().getOMPBuilder().finalize(CurFn);
----------------
Without this patch, `finalize` is called even if `!CurFn`?
================
Comment at: clang/lib/CodeGen/CodeGenFunction.h:288
+ /// this stack when done. Entering a new loop requires clearing this list; it
+ /// either means we start parsing an new loop nest (in which case the
previous
+ /// loop nest goes out of scope) or a second loop in the same level in which
----------------
================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:3771
+
+ void VisitOMPCanonicalLoop(OMPCanonicalLoop *S) { VisitStmt(S); }
+
----------------
Isn't `VisitStmt` called for any `Stmt` without this?
================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:5139
+/// Used to capture variable references if already parsed
statements/expressions
+/// into a CapturedStatement.
+class CaptureVars : public TreeTransform<CaptureVars> {
----------------
I think this means:
> If already parsed statements/expressions, used to capture variable references
> into a CapturedStatement.
But it reads like it means:
> If already parsed statements/expressions into a CapturedStatement, used to
> capture variable references.
================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:5215
+ Expr *NegIncAmount =
+ AssertSuccess(Actions.BuildUnaryOp(nullptr, {}, UO_Minus, NewStep));
+ Expr *BackwardDist = AssertSuccess(
----------------
It looks like the AST nodes `NewStart`, `NewStop`, and `NewStep` are shared by
multiple parent nodes here. Does that ever happen anywhere else in a Clang AST?
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
