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:
> > 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?


================
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:
> > 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.


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

Reply via email to