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

ABataev added inline comments. Comment at: clang/lib/Parse/ParseOpenMP.cpp:2787 + if (getLangOpts().OpenMP < 51 && Kind == OMPC_default && + static_cast(Val.getValue().Type) == + OMP_DEFAULT_firstprivate) { atmnpatel wrote: > ABataev wrote: > > atm

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

atmnpatel marked an inline comment as done. atmnpatel added inline comments. Comment at: clang/lib/Parse/ParseOpenMP.cpp:2787 + if (getLangOpts().OpenMP < 51 && Kind == OMPC_default && + static_cast(Val.getValue().Type) == + OMP_DEFAULT_firstprivate) {

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

ABataev added inline comments. Comment at: clang/lib/Parse/ParseOpenMP.cpp:2787 + if (getLangOpts().OpenMP < 51 && Kind == OMPC_default && + static_cast(Val.getValue().Type) == + OMP_DEFAULT_firstprivate) { atmnpatel wrote: > ABataev wrote: > > ABa

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

atmnpatel added inline comments. Comment at: clang/lib/Parse/ParseOpenMP.cpp:2787 + if (getLangOpts().OpenMP < 51 && Kind == OMPC_default && + static_cast(Val.getValue().Type) == + OMP_DEFAULT_firstprivate) { ABataev wrote: > ABataev wrote: > > No

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

ABataev accepted this revision. ABataev added a comment. LGTM with a nit. Comment at: clang/lib/Parse/ParseOpenMP.cpp:2787 + if (getLangOpts().OpenMP < 51 && Kind == OMPC_default && + static_cast(Val.getValue().Type) == + OMP_DEFAULT_firstprivate) { -

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

ABataev added a comment. In D75591#2134301 , @atmnpatel wrote: > Ping @ABataev. Rebase, please Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75591/new/ https://reviews.llvm.org/D75591 _

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

atmnpatel added a comment. Herald added a subscriber: aaron.ballman. Ping @ABataev. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75591/new/ https://reviews.llvm.org/D75591 ___ cfe-commits mailing list

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

atmnpatel updated this revision to Diff 265788. atmnpatel added a comment. Herald added a subscriber: sstefan1. Updated tests and includes changes made by @ABataev. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75591/new/ https://reviews.llvm.org/D

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

ABataev added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:3434-3435 + if (Stack->getDefaultDSA() == DSA_firstprivate && + VD->getStorageDuration() == SD_Static && + CanonicalVD->getDeclContext()->isFileContext() && !Stack->isLoopCo

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

atmnpatel added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:3434-3435 + if (Stack->getDefaultDSA() == DSA_firstprivate && + VD->getStorageDuration() == SD_Static && + CanonicalVD->getDeclContext()->isFileContext() && !Stack->isLoop

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

atmnpatel updated this revision to Diff 264365. atmnpatel added a comment. Necessary cleanups and adds a basic check for static local variables and static data members. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75591/new/ https://reviews.llvm.

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

ABataev added a comment. Do you have the tests for static locals and static data members with `default(firstprivate)`? Comment at: clang/lib/Parse/ParseOpenMP.cpp:2787 + if (getLangOpts().OpenMP < 51 && Kind == OMPC_default && + static_cast(Val.getValue().Type) == +

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

atmnpatel updated this revision to Diff 263832. atmnpatel added a comment. Addresses inline comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75591/new/ https://reviews.llvm.org/D75591 Files: clang-tools-extra/docs/clang-tidy/checks/openmp

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

ABataev added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:2053 + D, [](OpenMPClauseKind K) -> bool { return K != OMPC_unknown; }, + Level, /*NotLastprivate=*/true) && + !DSAStack->isLoopControlVariable(D, Level).first) { `/*N

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

atmnpatel updated this revision to Diff 263326. atmnpatel added a comment. Fixed check for scalar types. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75591/new/ https://reviews.llvm.org/D75591 Files: clang-tools-extra/docs/clang-tidy/checks/ope

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

ABataev added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:38 #include "llvm/Frontend/OpenMP/OMPConstants.h" +#include "llvm/Support/ErrorHandling.h" #include Why do you need this here? Comment at: clang/lib/Sema/SemaOpenMP.cpp

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

atmnpatel added a comment. ping 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/c

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

atmnpatel added a comment. ping 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/c

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

atmnpatel added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:2016-2023 + if (DSAStack->getDefaultDSA() == DSA_firstprivate && + VarsWithInheritedDSAType().count(D) == 0 && + !DSAStack->hasExplicitDSA( + D, [](OpenMPClauseKind K) -> bool { return

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

atmnpatel updated this revision to Diff 259122. atmnpatel added a comment. Add capture by value for scalars. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75591/new/ https://reviews.llvm.org/D75591 Files: clang-tools-extra/docs/clang-tidy/checks

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

atmnpatel added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:3211-3213 + VD->getStorageClass() == SC_Static && + (CanonicalVD->getDeclContext()->isNamespace() || + !VD->isLocalVarDeclOrParm())) { ABataev wrote: > atmnpate

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

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

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

atmnpatel marked 2 inline comments as done. atmnpatel added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:3211-3213 + VD->getStorageClass() == SC_Static && + (CanonicalVD->getDeclContext()->isNamespace() || + !VD->isLocalVarDeclOrParm()))

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

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

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

atmnpatel updated this revision to Diff 259073. atmnpatel marked 3 inline comments as done. atmnpatel added a comment. Addresses inline comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75591/new/ https://reviews.llvm.org/D75591 Files: cla

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

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() || +

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

ABataev added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:1125 +DVar.CKind = OMPC_firstprivate; +DVar.PrivateCopy = nullptr; +return DVar; Need to add `DVar.ImplicitDSALoc = Iter->DefaultAttrLoc;` Comment at: clang/l

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

lebedev.ri resigned from this revision. lebedev.ri added a comment. This revision is now accepted and ready to land. No further comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75591/new/ https://reviews.llvm.org/D75591 __

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

atmnpatel updated this revision to Diff 256152. atmnpatel added a comment. Addresses D77731 and fixes the comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75591/new/ https://reviews.llvm.org/D75591 Files:

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

jdoerfert accepted this revision. jdoerfert added a comment. Apologies for my delay. LGTM. Wait for @lebedev.ri though. Comment at: clang-tools-extra/docs/clang-tidy/checks/openmp-use-default-none.rst:56 + // ``parallel`` directive can have ``default`` clause, and said claus

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

atmnpatel updated this revision to Diff 254917. atmnpatel added a comment. Fixes clang-tidy check, and hopefully the other unrelated test failure too. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75591/new/ https://reviews.llvm.org/D75591 Files:

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

atmnpatel updated this revision to Diff 254392. atmnpatel added a comment. Rebase. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75591/new/ https://reviews.llvm.org/D75591 Files: clang-tools-extra/docs/clang-tidy/checks/openmp-use-default-none.r

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

atmnpatel updated this revision to Diff 254379. atmnpatel added a comment. Herald added a subscriber: yaxunl. Fixes to tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75591/new/ https://reviews.llvm.org/D75591 Files: clang-tools-extra/docs/

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

atmnpatel updated this revision to Diff 253498. atmnpatel added a comment. Second try at generating a patch file. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75591/new/ https://reviews.llvm.org/D75591 Files: clang-tools-extra/docs/clang-tidy/c

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

atmnpatel updated this revision to Diff 253470. atmnpatel added a comment. Rebased. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75591/new/ https://reviews.llvm.org/D75591 Files: clang-tools-extra/docs/clang-tidy/checks/openmp-use-default-none.

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

atmnpatel updated this revision to Diff 253463. atmnpatel added a comment. Added error based on OpenMP version. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75591/new/ https://reviews.llvm.org/D75591 Files: clang-tools-extra/docs/clang-tidy/che

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

atmnpatel added a comment. > In D75591#1936152 , @lebedev.ri > wrote: > It should be handled consistently with how other newer openmp features are > handled with older openmp version specified. In terms of consistency, the newer openmp features are hand

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

lebedev.ri added a comment. In D75591#1936116 , @jdoerfert wrote: > Did you add the warning @lebedev.ri mentioned? (@lebedev.ri I assume/hope a > warning is sufficient here?) It should be handled consistently with how other newer openmp features are ha

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

jdoerfert added a comment. Did you add the warning @lebedev.ri mentioned? (@lebedev.ri I assume/hope a warning is sufficient here?) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75591/new/ https://reviews.llvm.org/D75591 ___

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

atmnpatel updated this revision to Diff 251854. atmnpatel added a comment. Fixed linter error. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75591/new/ https://reviews.llvm.org/D75591 Files: clang-tools-extra/docs/clang-tidy/checks/openmp-use-de

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

atmnpatel updated this revision to Diff 251847. atmnpatel added a comment. Comment fixes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75591/new/ https://reviews.llvm.org/D75591 Files: clang-tools-extra/docs/clang-tidy/checks/openmp-use-default

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

atmnpatel marked 3 inline comments as done. atmnpatel added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:5102 Diag(P.second->getExprLoc(), diag::err_omp_no_dsa_for_variable) << P.first << P.second->getSourceRange(); Diag(DSAStack->getDefault

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

fghanim added a comment. My comments were nits so. LGTM 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

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

lebedev.ri requested changes to this revision. lebedev.ri added a comment. This revision now requires changes to proceed. In D75591#1904969 , @lebedev.ri wrote: > IIUC `default(firstprivate)` is being added in openmp-5.1, > it is not in openmp-4.5, so it

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

jdoerfert added a comment. I'm satisfied with this, @fghanim wdyt? Comment at: clang/lib/Sema/SemaOpenMP.cpp:5102 Diag(P.second->getExprLoc(), diag::err_omp_no_dsa_for_variable) << P.first << P.second->getSourceRange(); Diag(DSAStack->getDefaultDSALocat

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

atmnpatel added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:5102 Diag(P.second->getExprLoc(), diag::err_omp_no_dsa_for_variable) << P.first << P.second->getSourceRange(); Diag(DSAStack->getDefaultDSALocation(), diag::note_omp_default_dsa_no

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

fghanim added a comment. @jdoerfert To answer your question as to why it is being codegened as shared, codegen doesn't handle `default` clause- at least I didn't come across such a thing- and if you think about it, it shouldn't need to. You either have `none` or `shared`: - `none` is largly to

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

atmnpatel added a comment. Yep, it is still an open issue. I am actually unsure of how to do that, and I'd love some pointers. As far as default(private) goes, I am also unsure of how to do it without retaining the prior values of the variables. I will take a look over this weekend and see if

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

jdoerfert added a comment. I think this is functionally correct even if we pass the pointer for now as it is properly "privatized" in the callee. I will go over this once more (after some sleep) but I think it is all good. Once @fghanim puts the new code path on phab we need to ensure it works

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

fghanim added a comment. Done with `private` `firstprivate` `copyin` - working on modifying the clang test, and fixing some bugs. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75591/new/ https://reviews.llvm.org/D75591 _

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

jdoerfert added a subscriber: fghanim. jdoerfert added a comment. In D75591#1916433 , @atmnpatel wrote: > Modified the unit test for CodeGen of default(firstprivate) to more > accurately reflect the IR. > > From the perspective of the AST, the variables t

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

atmnpatel updated this revision to Diff 249570. atmnpatel added a comment. Modified the unit test for CodeGen of default(firstprivate) to more accurately reflect the IR. >From the perspective of the AST, the variables that are firstprivate from a >default clause are managed in a way that is sim

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

jdoerfert added inline comments. Comment at: clang/test/OpenMP/parallel_master_codegen.cpp:143 +// CK31: [[A_VAL:%.+]] = alloca i32 +// CK31: call void (%struct.ident_t*, i32, void (i32*, i32*, ...)*, ...) @__kmpc_fork_call(%struct.ident_t* {{.+}}, i32 1, void (i32*,

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

atmnpatel updated this revision to Diff 248310. atmnpatel added a comment. Herald added a reviewer: lebedev.ri. Modifies clang-tidy to include the new clause, and changes the value of DSA_firstprivate. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

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

lebedev.ri added a comment. IIUC `default(firstprivate)` is being added in openmp-5.1, it is not in openmp-4.5, so it should not be accepted in pre-openmp-5.1-mode (i.e. it should be diagnosed) Comment at: clang/lib/Sema/SemaOpenMP.cpp:56 + DSA_firstprivate = + 1 << 1 | 1

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

atmnpatel updated this revision to Diff 248098. atmnpatel added a comment. Fixes typos that causes certain unit tests to fail. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75591/new/ https://reviews.llvm.org/D75591 Files: clang/docs/LibASTMatch

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

atmnpatel created this revision. atmnpatel added a reviewer: jdoerfert. Herald added subscribers: llvm-commits, cfe-commits, guansong. Herald added projects: clang, LLVM. This implements the default(firstprivate) clause as defined in OpenMP Technical Report 8 (2.22.4). Repository: rG LLVM Gith