[PATCH] [OPENMP] Bugfix for processing of global variables in OpenMP regions.

Alexey Bataev a.bataev at hotmail.com
Fri Dec 12 04:05:23 PST 2014


================
Comment at: lib/CodeGen/CGExpr.cpp:1912
@@ +1911,3 @@
+        }
+      } else if (auto BD = dyn_cast_or_null<BlockDecl>(CurCodeDecl))
+        if (BD->capturesVariable(VD))
----------------
rjmccall wrote:
> This should be exhaustive, right?  In what situation can we have a DRE that refersToCapturedVariable() and we're not in a lambda, captured statement, or block?
> 
> If so, you should just use cast<> here.
I had a lot of troubles with this, but I fixed it.

================
Comment at: lib/Sema/SemaExpr.cpp:12141
@@ +12140,3 @@
+    if (!ParentDC) {
+      if (!Var->hasLocalStorage()) {
+        FunctionScopesIndex = MaxFunctionScopesIndex - 1;
----------------
rjmccall wrote:
> I think this is correct, but it deserves a comment.  We need to check for the parent *first* because, if we *have* private-captured a global variable, we need to recursively capture it in intermediate blocks, lambdas, etc.
> 
> Also, instead of recomputing hasLocalStorage(), please remember it from above.
Ok, will do.

================
Comment at: lib/Sema/SemaExpr.cpp:12373
@@ -12365,1 +12372,3 @@
 
+bool Sema::NeedToCaptureVariable(VarDecl *Var, SourceLocation Loc) {
+  QualType CaptureType;
----------------
rjmccall wrote:
> This method name suggests that it's non-state-changing, but I'm pretty sure that tryCaptureVariable isn't.  Why do you need this routine at all?
tryCaptureVariable is non-state-changing if BuildAndDiagnose arg is false. I need this function to check that we need to set flag RefersToCapturedVariable for DRE for the specified variable. (see line 1608 in lib/Sema/SemaExpr.cpp)

================
Comment at: lib/Sema/SemaOpenMP.cpp:555
@@ +554,3 @@
+bool Sema::IsOpenMPCapturedVar(VarDecl *VD) {
+  if (LangOpts.OpenMP && DSAStack->getCurrentDirective() != OMPD_unknown) {
+    auto DVarPrivate = DSAStack->getTopDSA(VD, /*FromParent=*/false);
----------------
rjmccall wrote:
> Just assert LangOpts.OpenMP.
Ok

http://reviews.llvm.org/D6259

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the cfe-commits mailing list