[PATCH] D94973: [clang][OpenMP] Use OpenMPIRBuilder for workshare loops.

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur added inline comments.


================
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) {
----------------
jdoerfert wrote:
> This doesn't impact anyone else negatively? I don't understand why we need this now.
This taken from how implicit lambda captures are handled: for nested lambdas, only the outermost lambda captures byval, the remaining use that existing copy. I think there are no other uses of CaptureStmt that have explicit captures, I am introducing the first.

With removing the assertion and therefore effectively allowing byval captures for CapturedStmts, I intended to make its behaviour resemble that of lambdas (`captureInLambda`).


================
Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:986
+  if (BodyGenCB)
+    BodyGenCB(CL->getBodyIP(), CL->getIndVar());
 
----------------
jdoerfert wrote:
> I'm unsure I understand when it would make sense to not have a body generator.
As done by EmitOMPCanonicalLoop, the body code can also added to `CL->getBodyIP()` to the CL returned by this function. Calling the callback is the last action done anyway, using the callback just makes the code harder to understand.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94973