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

Alexey Bataev a.bataev at hotmail.com
Tue Nov 18 05:04:45 PST 2014


Hi John,
Thanks a lot for the review.
> Ugh. Copy-captured global variables? Really?
Yes, if you have global var explicitly marked as private in OpenMP region, like:
```
static int g;
void foo() {
int i;
#pragma omp parallel firstprivate(g)
i = ++g;
}
```
you have to capture 'g' and generate a private copy with initial value of 'g'.
> 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.
I'll try to implement it. I ran into several troubles in my first attempt, trying to resolve them now.

================
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.
----------------
rjmccall wrote:
> This comment needs to be changed, I guess.
Ok, I'll change it.

================
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;
----------------
rjmccall wrote:
> 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.)
Actually IsOpenMPPrivateVar() already checks that OpenMP mode is on. But I can add a check for OpenMP mode like 'LangOpts.OpenMP && IsOpenMPPrivateVar()'.

http://reviews.llvm.org/D6259






More information about the cfe-commits mailing list