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

John McCall rjmccall at gmail.com
Mon Nov 17 15:14:44 PST 2014


Ugh.  Copy-captured global variables?  Really?

Well, it does seem to be very explicitly specified that way, and it's also very explicitly specified that references to the original global (e.g. from outside the pragma) may or may not refer to the private copy.  So I guess this is what we're doing now.

I think it would make more sense to track the reference-to-capture / reference-to-non-capture difference reliably in the AST.  We already have a refersToEnclosingLocal() bit on DeclRefExpr; making that a general "this is a reference to a captured variable" flag makes sense.  We then won't have to recompute it during IRGen.

================
Comment at: lib/Sema/SemaExpr.cpp:12119
@@ -12114,3 +12118,3 @@
   
   // If the variable is declared in the current context (and is not an 
   // init-capture), there is no need to capture it.
----------------
This comment needs to be changed, I guess.

================
Comment at: lib/Sema/SemaExpr.cpp:12122
@@ -12117,2 +12121,3 @@
   if (!Var->isInitCapture() && Var->getDeclContext() == DC) return true;
-  if (!Var->hasLocalStorage()) return true;
+  if (!Var->hasLocalStorage() && !NeedToCaptureGlobalVariable(*this, Var))
+    return true;
----------------
Please avoid even making a call for globals if we aren't in OpenMP mode.  (NeedToCapture will be reliably inlined; IsOpenMPPrivateVar won't be.  You can make IsOpenMPPrivateVar assert that we're in OpenMP mode.)

http://reviews.llvm.org/D6259






More information about the cfe-commits mailing list