ABataev added inline comments.
================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:3211-3213
+ VD->getStorageClass() == SC_Static &&
+ (CanonicalVD->getDeclContext()->isNamespace() ||
+ !VD->isLocalVarDeclOrParm())) {
----------------
atmnpatel wrote:
> ABataev wrote:
> > atmnpatel wrote:
> > > ABataev wrote:
> > > > I think just `!VD->hasLocalStorage()` should be enough here. Shall it
> > > > work for static locals too or just for globals?
> > > Just for globals.
> > What about static data members?
> The TR doesn't specify that static data members should inherit DSAs / have
> them explicitly defined.
What about non-static globals? Your current check works only for something like
`static int var;`, but not `int var;`.
================
Comment at: clang/test/OpenMP/parallel_master_codegen.cpp:145
+
+// CK31: define internal {{.*}}void [[OMP_OUTLINED]](i32* noalias
[[GTID:%.+]], i32* noalias [[BTID:%.+]], i32* dereferenceable(4) [[A_VAL]])
+// CK31: [[GTID_ADDR:%.+]] = alloca i32*
----------------
atmnpatel wrote:
> ABataev wrote:
> > atmnpatel wrote:
> > > ABataev wrote:
> > > > Some extra work is required. The variable should not be captured by
> > > > reference, must be captured by value. Also, a test with calling
> > > > constructors/destructors is required.
> > > Sorry, I added a test with constructors/destructors with codegen. The
> > > variable capture discussion was had above and I'm pretty sure that the
> > > conclusion above (correct me if I'm wrong) was that its not ideal, but
> > > its fine for now and will be adjusted with the upcoming OMPIRBuilder.
> > It shall emit the correct code anyway. With `default(firstprivate)` and
> > explicit `firstprivate` clause the codegen will be different and it may
> > lead to an incompatibility with the existing libraries/object files.
> Ohh I see your concern now. Is the conclusion that I should just table this
> until something new pops up? I couldn't follow up on @fghanim's advice (sorry
> I've been on and off trying different things for weeks), it's beyond my
> capabilities.
You need to modify function `Sema::isOpenMPCapturedByRef`. There is a check for
firstprivates in there, you need to add a check that the variable has no
explicit data-sharing attributes and the default data sharing is set to
`firstprivate`.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D75591/new/
https://reviews.llvm.org/D75591
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
