Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion clang/docs/OpenMPSupport.rst
Original file line number Diff line number Diff line change
Expand Up @@ -559,7 +559,7 @@ implementation.
+-------------------------------------------------------------+---------------------------+---------------------------+--------------------------------------------------------------------------+
| Clarifications to Fortran map semantics | :none:`unclaimed` | :none:`unclaimed` | |
+-------------------------------------------------------------+---------------------------+---------------------------+--------------------------------------------------------------------------+
| default clause at target construct | :part:`In Progress` | :none:`unclaimed` | |
| default clause at target construct | :good:`done` | :none:`unclaimed` | https://github.com/llvm/llvm-project/pull/162910 |
+-------------------------------------------------------------+---------------------------+---------------------------+--------------------------------------------------------------------------+
| ref count update use_device_{ptr, addr} | :none:`unclaimed` | :none:`unclaimed` | |
+-------------------------------------------------------------+---------------------------+---------------------------+--------------------------------------------------------------------------+
Expand Down
1 change: 1 addition & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -689,6 +689,7 @@ OpenMP Support
- Added support for 'omp fuse' directive.
- Updated parsing and semantic analysis support for ``nowait`` clause to accept
optional argument in OpenMP >= 60.
- Added support for ``default`` clause on ``target`` directive.

Improvements
^^^^^^^^^^^^
Expand Down
96 changes: 76 additions & 20 deletions clang/lib/Sema/SemaOpenMP.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17316,45 +17316,101 @@ OMPClause *SemaOpenMP::ActOnOpenMPDefaultClause(
<< getOpenMPClauseNameForDiag(OMPC_default);
return nullptr;
}
if (VCKind == OMPC_DEFAULT_VC_unknown) {
Diag(VCKindLoc, diag::err_omp_default_vc)
<< getOpenMPSimpleClauseTypeName(OMPC_default, unsigned(M));
return nullptr;
}
bool IsTargetDefault = getLangOpts().OpenMP >= 60 &&
DSAStack->getCurrentDirective() == OMPD_target;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about combined directives?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are correct. This should apply for combined directives. Thanks for catching this, Alexey.


// OpenMP 6.0, page 224, lines 3-4 default Clause, Semantics
// If data-sharing-attribute is shared then the clause has no effect
// on a target construct;
if (IsTargetDefault && M == OMP_DEFAULT_shared)
return nullptr;

OpenMPDefaultmapClauseModifier DefMapMod;
OpenMPDefaultmapClauseKind DefMapKind;
std::function<void(SourceLocation)> SetDefaultDSA;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
auto &&SetDefaultClauseAttrs = [&](llvm::omp::DefaultKind M,
auto SetDefaultClauseAttrs = [&](llvm::omp::DefaultKind M,

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Thanks, Alexey.

std::function<void(SourceLocation)> SetDefaultDSAVC;
// default data-sharing-attribute
switch (M) {
case OMP_DEFAULT_none:
DSAStack->setDefaultDSANone(MLoc);
break;
case OMP_DEFAULT_shared:
DSAStack->setDefaultDSAShared(MLoc);
DefMapMod = OMPC_DEFAULTMAP_MODIFIER_none;
SetDefaultDSA = [&](SourceLocation MLoc) {
DSAStack->setDefaultDSANone(MLoc);
};
break;
case OMP_DEFAULT_firstprivate:
DSAStack->setDefaultDSAFirstPrivate(MLoc);
DefMapMod = OMPC_DEFAULTMAP_MODIFIER_firstprivate;
SetDefaultDSA = [&](SourceLocation MLoc) {
DSAStack->setDefaultDSAFirstPrivate(MLoc);
};
break;
case OMP_DEFAULT_private:
DSAStack->setDefaultDSAPrivate(MLoc);
DefMapMod = OMPC_DEFAULTMAP_MODIFIER_private;
SetDefaultDSA = [&](SourceLocation MLoc) {
DSAStack->setDefaultDSAPrivate(MLoc);
};
break;
case OMP_DEFAULT_shared:
assert(!IsTargetDefault && "DSA shared invalid with target directive");
SetDefaultDSA = [&](SourceLocation MLoc) {
DSAStack->setDefaultDSAShared(MLoc);
};
break;
default:
llvm_unreachable("DSA unexpected in OpenMP default clause");
llvm_unreachable("unexpected DSA in OpenMP default clause");
}

// default variable-category
switch (VCKind) {
case OMPC_DEFAULT_VC_aggregate:
DSAStack->setDefaultDSAVCAggregate(VCKindLoc);
break;
case OMPC_DEFAULT_VC_all:
DSAStack->setDefaultDSAVCAll(VCKindLoc);
break;
case OMPC_DEFAULT_VC_allocatable:
DSAStack->setDefaultDSAVCAllocatable(VCKindLoc);
DefMapKind = OMPC_DEFAULTMAP_aggregate;
SetDefaultDSAVC = [&](SourceLocation VCKindLoc) {
DSAStack->setDefaultDSAVCAggregate(VCKindLoc);
};
break;
case OMPC_DEFAULT_VC_pointer:
DSAStack->setDefaultDSAVCPointer(VCKindLoc);
DefMapKind = OMPC_DEFAULTMAP_pointer;
SetDefaultDSAVC = [&](SourceLocation VCKindLoc) {
DSAStack->setDefaultDSAVCPointer(VCKindLoc);
};
break;
case OMPC_DEFAULT_VC_scalar:
DSAStack->setDefaultDSAVCScalar(VCKindLoc);
DefMapKind = OMPC_DEFAULTMAP_scalar;
SetDefaultDSAVC = [&](SourceLocation VCKindLoc) {
DSAStack->setDefaultDSAVCScalar(VCKindLoc);
};
break;
case OMPC_DEFAULT_VC_all:
DefMapKind = OMPC_DEFAULTMAP_all;
SetDefaultDSAVC = [&](SourceLocation VCKindLoc) {
DSAStack->setDefaultDSAVCAll(VCKindLoc);
};
break;
default:
Diag(VCKindLoc, diag::err_omp_default_vc)
<< getOpenMPSimpleClauseTypeName(OMPC_default, unsigned(M));
llvm_unreachable("unexpected variable category in OpenMP default clause");
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why just not put this whole logic in lambda and call it, where required, instead of having std:function?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this suggestion, Alexey. I'll try adapting the code to use this approach and see how it works out.


// OpenMP 6.0, page 224, lines 4-5 default Clause, Semantics
// otherwise, its effect on a target construct is equivalent to
// specifying the defaultmap clause with the same data-sharing-attribute
// and variable-category.
if (IsTargetDefault) {
if (DefMapKind == OMPC_DEFAULTMAP_all) {
DSAStack->setDefaultDMAAttr(DefMapMod, OMPC_DEFAULTMAP_aggregate, MLoc);
DSAStack->setDefaultDMAAttr(DefMapMod, OMPC_DEFAULTMAP_scalar, MLoc);
DSAStack->setDefaultDMAAttr(DefMapMod, OMPC_DEFAULTMAP_pointer, MLoc);
} else {
DSAStack->setDefaultDMAAttr(DefMapMod, DefMapKind, MLoc);
}
} else {
// If earlier than OpenMP 6.0, or not a target directive, then set
// default DSA as before.
SetDefaultDSA(MLoc);
SetDefaultDSAVC(VCKindLoc);
}

return new (getASTContext())
OMPDefaultClause(M, MLoc, VCKind, VCKindLoc, StartLoc, LParenLoc, EndLoc);
}
Expand Down
Loading