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
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
