sfantao added a comment.
Hi Alexey,
Thanks for the review.
================
Comment at: lib/Sema/SemaOpenMP.cpp:816-822
@@ -801,6 +815,9 @@
+
+ // A DSA refers to this captured region if the parent contexts match.
+ auto *ParentContext = RSI->TheCapturedDecl->getParent();
for (auto I = Stack.rbegin(), EE = Stack.rend(); I != EE; ++I)
- if (I->CurScope == S)
+ if (I->ParentDeclContext == ParentContext)
return I->Directive;
return OMPD_unknown;
}
----------------
ABataev wrote:
> Actually, I think, we need to rework the whole IsOpenMPCapturedByRef()
> function. I'm not sure that it is even required. It looks like some
> optimization hack in the frontend. I'm against any not-necessary
> optimizations in frontend. If scalar value is a firstprivate, it must be
> handled in codegen, not by handling it by copy.
'IsOpenMPCapturedByRef' goal is to change the signature of the outlined
function. We don't want to have reference types in target region arguments
unless they are really required. We have seen performance being greatly
affected just because of that. Of course a consequence of this is to have
variables that become first private.
The current implementation of OpenMP firstprivate doesn't change the the
function signatures, only the codegen inside the region is affected. So, this
is not a complete overlap.
With this, I am not saying that this cannot be done in codegen. The reason I am
doing it here is your initial guideline that we should attempt to fix most of
the things in Sema and avoid complicating codegen which is a more sensitive
part.
Doing this in Codegen would require changing the implementation of
`GenerateOpenMPCapturedVars` and `GenerateOpenMPCapturedStmtFunction`. We
wouldn't need the tracking of the context anymore (checking the directive would
be sufficient), but we would still need to see what't in the map clauses.
So, do you think we should change this?
http://reviews.llvm.org/D18110
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
