[PATCH] D75591: [OpenMP] Add firstprivate as a default data-sharing attribute to clang

[Atmn Patel via Phabricator via llvm-commits]

atmnpatel marked an inline comment as done and an inline comment as not done.
atmnpatel added inline comments.


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:3211-3213
+          VD->getStorageClass() == SC_Static &&
+          (CanonicalVD->getDeclContext()->isNamespace() ||
+           !VD->isLocalVarDeclOrParm())) {
----------------
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.


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


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75591/new/

https://reviews.llvm.org/D75591