[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

2019-08-21 Thread Joel E. Denny via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. jdenny marked 2 inline comments as done. Closed by commit rL369619: [OpenMP] Permit map with DSA on combined directive (authored by jdenny, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commit

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

2019-08-21 Thread Joel E. Denny via Phabricator via cfe-commits
jdenny marked 4 inline comments as done. jdenny added a comment. Thanks. Comment at: clang/lib/Sema/SemaExpr.cpp:17748 ObjCAvailabilityCheckExpr(Version, AtLoc, RParen, Context.BoolTy); -} \ No newline at end of file +} ABataev wrote: > jdenny wrote: > >

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

2019-08-21 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev accepted this revision. ABataev added a comment. This revision is now accepted and ready to land. LG Comment at: clang/lib/Sema/SemaExpr.cpp:17749 } \ No newline at end of file Still marked as changed code, better to restore it completely. CHANGES S

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

2019-08-21 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:17748 ObjCAvailabilityCheckExpr(Version, AtLoc, RParen, Context.BoolTy); -} \ No newline at end of file +} jdenny wrote: > ABataev wrote: > > Restore original code here > OK, I did. Wh

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

2019-08-21 Thread Joel E. Denny via Phabricator via cfe-commits
jdenny added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:17748 ObjCAvailabilityCheckExpr(Version, AtLoc, RParen, Context.BoolTy); -} \ No newline at end of file +} ABataev wrote: > Restore original code here OK, I did. What's the reason for n

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

2019-08-21 Thread Joel E. Denny via Phabricator via cfe-commits
jdenny updated this revision to Diff 216502. jdenny marked 8 inline comments as done. jdenny added a comment. Make suggested changes to default arguments, comments on literals, and parameter names. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65835/new/ https://reviews.llvm.org/D65835

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

2019-08-21 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added inline comments. Comment at: clang/include/clang/Sema/Sema.h:1419 + RecordDecl *RD, CapturedRegionKind K, + unsigned CaptureLevel); Better to use `OpenMPCaptureLevel` since this param is

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

2019-08-21 Thread Joel E. Denny via Phabricator via cfe-commits
jdenny updated this revision to Diff 216469. jdenny marked 7 inline comments as done. jdenny set the repository for this revision to rG LLVM Github Monorepo. jdenny added a comment. Make suggested changes for passing around the capture level. Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

2019-08-21 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added inline comments. Comment at: clang/lib/Sema/Sema.cpp:2108 void Sema::PushCapturedRegionScope(Scope *S, CapturedDecl *CD, RecordDecl *RD, CapturedRegionKind K) { + CapturedRegionScopeInfo *CSI = new CapturedRegionScopeInfo( -

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

2019-08-21 Thread Joel E. Denny via Phabricator via cfe-commits
jdenny marked an inline comment as done. jdenny added inline comments. Comment at: clang/lib/Sema/Sema.cpp:2108 void Sema::PushCapturedRegionScope(Scope *S, CapturedDecl *CD, RecordDecl *RD, CapturedRegionKind K) { + CapturedRegionScopeInfo *

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

2019-08-21 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added inline comments. Comment at: clang/lib/Sema/Sema.cpp:2108 void Sema::PushCapturedRegionScope(Scope *S, CapturedDecl *CD, RecordDecl *RD, CapturedRegionKind K) { + CapturedRegionScopeInfo *CSI = new CapturedRegionScopeInfo( -

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

2019-08-21 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added inline comments. Comment at: clang/lib/Sema/Sema.cpp:2108 void Sema::PushCapturedRegionScope(Scope *S, CapturedDecl *CD, RecordDecl *RD, CapturedRegionKind K) { + CapturedRegionScopeInfo *CSI = new CapturedRegionScopeInfo( -

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

2019-08-21 Thread Joel E. Denny via Phabricator via cfe-commits
jdenny marked an inline comment as done. jdenny added inline comments. Comment at: clang/lib/Sema/Sema.cpp:2108 void Sema::PushCapturedRegionScope(Scope *S, CapturedDecl *CD, RecordDecl *RD, CapturedRegionKind K) { + CapturedRegionScopeInfo *

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

2019-08-21 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added inline comments. Comment at: clang/include/clang/Sema/ScopeInfo.h:763 RecordDecl *RD, ImplicitParamDecl *Context, CapturedRegionKind K, unsigned OpenMPLevel) : CapturingScopeInfo(Diag, ImpCap_CapturedRegio

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

2019-08-21 Thread Joel E. Denny via Phabricator via cfe-commits
jdenny updated this revision to Diff 216444. jdenny edited the summary of this revision. jdenny added a comment. Restore previous version of patch, and rebase. I tried, and this patch is not sufficient to fix PR40253. If there are indeed common changes, it seems it's just as well to put them he

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

2019-08-21 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added a comment. In D65835#1639739 , @jdenny wrote: > In D65835#1639700 , @ABataev wrote: > > > Chwck 2 ifs: `if (IsVariableUsedInMapClause)` and the second `if (IsByRef > > && Ty.getNonReferenceType()->isS

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

2019-08-21 Thread Joel E. Denny via Phabricator via cfe-commits
jdenny added a comment. In D65835#1639700 , @ABataev wrote: > Chwck 2 ifs: `if (IsVariableUsedInMapClause)` and the second `if (IsByRef && > Ty.getNonReferenceType()->isScalarType())`. If the variable is mapped, > `IsByRef` is set to `true`, but later it

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

2019-08-21 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added a comment. In D65835#1639685 , @jdenny wrote: > In D65835#1639622 , @ABataev wrote: > > > We don't need this new level counter to correctly handle this situation. > > Just check for the combined targe

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

2019-08-21 Thread Joel E. Denny via Phabricator via cfe-commits
jdenny added a comment. In D65835#1639622 , @ABataev wrote: > We don't need this new level counter to correctly handle this situation. Just > check for the combined target directive in `Sema::isOpenMPCapturedByRef` and > return true if it is mapped as to

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

2019-08-21 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added a comment. In D65835#1639617 , @jdenny wrote: > In D65835#1639612 , @ABataev wrote: > > > In D65835#1639593 , @jdenny wrote: > > > > > In D65835#1639585

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

2019-08-21 Thread Joel E. Denny via Phabricator via cfe-commits
jdenny added a comment. In D65835#1639612 , @ABataev wrote: > In D65835#1639593 , @jdenny wrote: > > > In D65835#1639585 , @ABataev wrote: > > > > > In D65835#1639584

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

2019-08-21 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added a comment. In D65835#1639593 , @jdenny wrote: > In D65835#1639585 , @ABataev wrote: > > > In D65835#1639584 , @jdenny wrote: > > > > > I want to be sure we're o

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

2019-08-21 Thread Joel E. Denny via Phabricator via cfe-commits
jdenny added a comment. In D65835#1639585 , @ABataev wrote: > In D65835#1639584 , @jdenny wrote: > > > I want to be sure we're on the same page. Due to the changes I just backed > > out, the following two examples

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

2019-08-21 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev requested changes to this revision. ABataev added a comment. This revision now requires changes to proceed. Need to fix the mapping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65835/new/ https://reviews.llvm.org/D65835

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

2019-08-21 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added a comment. In D65835#1639584 , @jdenny wrote: > I want to be sure we're on the same page. Due to the changes I just backed > out, the following two examples now generate different code: > > int a = 0; > #pragma omp target map(a) > #pr

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

2019-08-21 Thread Joel E. Denny via Phabricator via cfe-commits
jdenny added a comment. I want to be sure we're on the same page. Due to the changes I just backed out, the following two examples now generate different code: int a = 0; #pragma omp target map(a) #pragma omp teams firstprivate(a) ; int a = 0; #pragma omp target teams firstpriv

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

2019-08-21 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev accepted this revision. ABataev added a comment. This revision is now accepted and ready to land. LG Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65835/new/ https://reviews.llvm.org/D65835 ___

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

2019-08-21 Thread Joel E. Denny via Phabricator via cfe-commits
jdenny updated this revision to Diff 216420. jdenny edited the summary of this revision. jdenny set the repository for this revision to rG LLVM Github Monorepo. jdenny added a comment. As requested, backed out the changes related to firstprivate scalars, and updated tests. Repository: rG LLVM

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

2019-08-21 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added a comment. In D65835#1630732 , @jdenny wrote: > I think you're referring to stuff like `OpenMPCaptureLevel` in `ScopeInfo.h`. > I wrote those changes for this patch first, as can be seen in phabricator > history. I needed them for the oth

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

2019-08-21 Thread Joel E. Denny via Phabricator via cfe-commits
jdenny added a comment. Ping. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65835/new/ https://reviews.llvm.org/D65835 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

2019-08-14 Thread Joel E. Denny via Phabricator via cfe-commits
jdenny added a comment. I think you're referring to stuff like `OpenMPCaptureLevel` in `ScopeInfo.h`. I wrote those changes for this patch first, as can be seen in phabricator history. I needed them for the other patch too, and I thought we were going to end up applying that patch first, so I

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

2019-08-14 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added a comment. Why there are the changes from the another patch? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65835/new/ https://reviews.llvm.org/D65835 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.o

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

2019-08-14 Thread Joel E. Denny via Phabricator via cfe-commits
jdenny updated this revision to Diff 215298. jdenny added a comment. Rebase. Add more tests, as requested at D66247#1630441 . CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65835/new/ https://reviews.llvm.org/D65835 Files: clang/include/clang

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

2019-08-11 Thread Joel E. Denny via Phabricator via cfe-commits
jdenny added a comment. In D65835#1624659 , @ABataev wrote: > In D65835#1624629 , @jdenny wrote: > > > In D65835#1624624 , @ABataev wrote: > > > > > In D65835#1624619

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

2019-08-11 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added a comment. In D65835#1624629 , @jdenny wrote: > In D65835#1624624 , @ABataev wrote: > > > In D65835#1624619 , @jdenny wrote: > > > > > In D65835#1624617

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

2019-08-11 Thread Joel E. Denny via Phabricator via cfe-commits
jdenny added a comment. In D65835#1624624 , @ABataev wrote: > In D65835#1624619 , @jdenny wrote: > > > In D65835#1624617 , @ABataev wrote: > > > > > Target teams private map

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

2019-08-11 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added a comment. In D65835#1624619 , @jdenny wrote: > In D65835#1624617 , @ABataev wrote: > > > Target teams private map will produce extra private in target context, > > other constructs either. > > > Here

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

2019-08-11 Thread Joel E. Denny via Phabricator via cfe-commits
jdenny added a comment. In D65835#1624617 , @ABataev wrote: > Target teams private map will produce extra private in target context, other > constructs either. Here's what I tried: int a; #pragma omp target teams private(a) map(a) ; The same c

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

2019-08-11 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added a comment. In D65835#1624616 , @jdenny wrote: > In D65835#1624615 , @ABataev wrote: > > > This is wrong. It affects all possible combinations and not only fof scalar > > types, all of them are affecte

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

2019-08-11 Thread Joel E. Denny via Phabricator via cfe-commits
jdenny added a comment. In D65835#1624615 , @ABataev wrote: > This is wrong. It affects all possible combinations and not only fof scalar > types, all of them are affected. Are you saying the patch isn't sufficient because other types need to be fixed

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

2019-08-11 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added a comment. In D65835#1624610 , @jdenny wrote: > In D65835#1624477 , @ABataev wrote: > > > Try something like `target parallel firstprivate (a) map(a)`. Currently it > > will create a firstprivate copy

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

2019-08-11 Thread Joel E. Denny via Phabricator via cfe-commits
jdenny updated this revision to Diff 214559. jdenny marked 3 inline comments as done. jdenny edited the summary of this revision. jdenny set the repository for this revision to rG LLVM Github Monorepo. jdenny added a comment. In D65835#1624477 , @ABataev wr

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

2019-08-10 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added a comment. In D65835#1624471 , @jdenny wrote: > I made the following changes, as suggested: > > - Add back restriction for OpenMP < 5.0. > - Remove `is_device_ptr` changes. > > Alexey, you said: > > > Plus, these changes are not enough to s

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

2019-08-10 Thread Joel E. Denny via Phabricator via cfe-commits
jdenny updated this revision to Diff 214542. jdenny retitled this revision from "[OpenMP] Fix map/is_device_ptr with DSA on combined directive" to "[OpenMP] Permit map with DSA on combined directive". jdenny edited the summary of this revision. jdenny added a comment. I made the following changes