Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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 @@ -153,7 +153,7 @@ implementation.
+------------------------------+--------------------------------------------------------------+--------------------------+-----------------------------------------------------------------------+
| device | clause: extended device | :good:`done` | |
+------------------------------+--------------------------------------------------------------+--------------------------+-----------------------------------------------------------------------+
| device | clause: uses_allocators clause | :good:`done` | |
| device | clause: uses_allocators clause | :good:`done` | https://github.com/llvm/llvm-project/pull/157025 |
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to do anything in codegen? If yes, this should be partial, I think

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's just a parsing change for the new uses_allocator syntax, no codegen required; works the same way, so keeping it as done.

+------------------------------+--------------------------------------------------------------+--------------------------+-----------------------------------------------------------------------+
| device | clause: in_reduction | :part:`worked on` | r308768 |
+------------------------------+--------------------------------------------------------------+--------------------------+-----------------------------------------------------------------------+
Expand Down
1 change: 1 addition & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -531,6 +531,7 @@ OpenMP Support
- Allow array length to be omitted in array section subscript expression.
- Fixed non-contiguous strided update in the ``omp target update`` directive with the ``from`` clause.
- Properly handle array section/assumed-size array privatization in C/C++.
- Added support to handle new syntax of the ``uses_allocators`` clause.
- Added support for ``variable-category`` modifier in ``default clause``.
- Added support for ``defaultmap`` directive implicit-behavior ``storage``.

Expand Down
2 changes: 2 additions & 0 deletions clang/include/clang/Basic/DiagnosticParseKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -1497,6 +1497,8 @@ def err_omp_multiple_step_or_linear_modifier : Error<
"multiple %select{'step size'|'linear modifier'}0 found in linear clause">;
def err_omp_deprecate_old_syntax: Error<
"old syntax '%0' on '%1' clause was deprecated, use new syntax '%2'">;
def err_omp_allocator_comma_separator :
Error<"',' not allowed as separator in 'uses_allocators' clause, use ';' instead">;
Comment on lines +1500 to +1501
Copy link
Member

Choose a reason for hiding this comment

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

Can you try to reuse existing message? I think there should be something similar already

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I checked through the file to see if there’s an existing message we could reuse, but didn’t find anything relevant (using ; instead of ,) If that’s fine, can we move ahead with this as is?

def warn_omp_future_directive_spelling: Warning<
"directive spelling '%0' is introduced in a later OpenMP version">,
InGroup<OpenMPFuture>;
Expand Down
75 changes: 75 additions & 0 deletions clang/lib/Parse/ParseOpenMP.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2959,6 +2959,73 @@ OMPClause *Parser::ParseOpenMPUsesAllocatorClause(OpenMPDirectiveKind DKind) {
return nullptr;
SmallVector<SemaOpenMP::UsesAllocatorsData, 4> Data;
do {
// Parse 'traits(expr) : Allocator' for >=5.2
if (getLangOpts().OpenMP >= 52 && Tok.is(tok::identifier) &&
Tok.getIdentifierInfo()->getName() == "traits") {

SemaOpenMP::UsesAllocatorsData &D = Data.emplace_back();

ConsumeToken();

// Parse '(' <expr> ')'
BalancedDelimiterTracker TraitParens(*this, tok::l_paren,
tok::annot_pragma_openmp_end);
TraitParens.consumeOpen();
ExprResult AllocatorTraits =
getLangOpts().CPlusPlus ? ParseCXXIdExpression() : ParseExpression();
TraitParens.consumeClose();

if (AllocatorTraits.isInvalid()) {
SkipUntil(
{tok::comma, tok::semi, tok::r_paren, tok::annot_pragma_openmp_end},
StopBeforeMatch);
break;
}

// Expect ':'
if (Tok.isNot(tok::colon)) {
Diag(Tok, diag::err_expected) << tok::colon;
SkipUntil(
{tok::comma, tok::semi, tok::r_paren, tok::annot_pragma_openmp_end},
StopBeforeMatch);
continue;
}
ConsumeToken();

CXXScopeSpec SS;
Token Replacement;
ExprResult AllocatorExpr =
getLangOpts().CPlusPlus
? ParseCXXIdExpression()
: tryParseCXXIdExpression(SS, /*isAddressOfOperand=*/false,
Replacement);

if (AllocatorExpr.isInvalid()) {
SkipUntil(
{tok::comma, tok::semi, tok::r_paren, tok::annot_pragma_openmp_end},
StopBeforeMatch);
break;
}

D.Allocator = AllocatorExpr.get();
D.AllocatorTraits = AllocatorTraits.get();
D.LParenLoc = TraitParens.getOpenLocation();
D.RParenLoc = TraitParens.getCloseLocation();

// Separator handling(;)
if (Tok.is(tok::comma)) {
// In 5.2, comma is invalid
Diag(Tok.getLocation(), diag::err_omp_allocator_comma_separator)
<< FixItHint::CreateReplacement(Tok.getLocation(), ";");
ConsumeAnyToken();
} else if (Tok.is(tok::semi)) {
ConsumeAnyToken(); // valid separator
}

continue;
}

// Parse 'Allocator(expr)' for <5.2
CXXScopeSpec SS;
Token Replacement;
ExprResult Allocator =
Expand Down Expand Up @@ -2988,6 +3055,14 @@ OMPClause *Parser::ParseOpenMPUsesAllocatorClause(OpenMPDirectiveKind DKind) {
D.AllocatorTraits = AllocatorTraits.get();
D.LParenLoc = T.getOpenLocation();
D.RParenLoc = T.getCloseLocation();

// Deprecation diagnostic in >= 5.2
if (getLangOpts().OpenMP >= 52) {
Diag(Loc, diag::err_omp_deprecate_old_syntax)
<< "allocator(expr)" // %0: old form
<< "uses_allocators" // %1: clause name
<< "traits(expr): alloc"; // %2: suggested new form
}
}
if (Tok.isNot(tok::comma) && Tok.isNot(tok::r_paren))
Diag(Tok, diag::err_omp_expected_punc) << "uses_allocators" << 0;
Expand Down
10 changes: 6 additions & 4 deletions clang/lib/Sema/SemaOpenMP.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24177,10 +24177,12 @@ OMPClause *SemaOpenMP::ActOnOpenMPUsesAllocatorClause(
// OpenMP [2.12.5, target Construct]
// Non-predefined allocators appearing in a uses_allocators clause must
// have traits specified.
if (!IsPredefinedAllocator && !D.AllocatorTraits) {
Diag(D.Allocator->getExprLoc(),
diag::err_omp_nonpredefined_allocator_without_traits);
continue;
if (getLangOpts().OpenMP < 52) {
if (!IsPredefinedAllocator && !D.AllocatorTraits) {
Diag(D.Allocator->getExprLoc(),
diag::err_omp_nonpredefined_allocator_without_traits);
continue;
}
}
// No allocator traits - just convert it to rvalue.
if (!D.AllocatorTraits)
Expand Down
27 changes: 23 additions & 4 deletions clang/test/OpenMP/target_uses_allocators_messages.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

// RUN: %clang_cc1 -verify -fopenmp-simd %s -Wuninitialized

// RUN: %clang_cc1 -DOMP52 -verify=omp52 -fopenmp -fopenmp-version=52 %s -Wuninitialized

// RUN: %clang_cc1 -DOMP52 -verify=omp52 -fopenmp-simd -fopenmp-version=52 %s -Wuninitialized

struct omp_alloctrait_t {};

typedef void **omp_allocator_handle_t;
Expand All @@ -16,10 +20,15 @@ extern const omp_allocator_handle_t omp_pteam_mem_alloc;
extern const omp_allocator_handle_t omp_thread_mem_alloc;

int main(int argc, char **argv) {
omp_alloctrait_t traits[10];
omp_alloctrait_t my_traits[10];
omp_alloctrait_t *ptraits;
omp_allocator_handle_t my_alloc = nullptr;
const omp_allocator_handle_t c_my_alloc = my_alloc;
omp_alloctrait_t aligned_traits[10];
omp_allocator_handle_t aligned_alloc;

#ifndef OMP52

#pragma omp target uses_allocators // expected-error {{expected '(' after 'uses_allocator'}}
{}
#pragma omp target uses_allocators( // expected-error {{expected ')'}} expected-note {{to match this '('}} expected-error {{expected unqualified-id}}
Expand All @@ -34,7 +43,7 @@ int main(int argc, char **argv) {
{}
#pragma omp target uses_allocators(omp_default_mem_alloc, omp_large_cap_mem_alloc, omp_const_mem_alloc, omp_high_bw_mem_alloc, omp_low_lat_mem_alloc, omp_cgroup_mem_alloc, omp_pteam_mem_alloc, omp_thread_mem_alloc)
{}
#pragma omp target uses_allocators(omp_default_mem_alloc(traits), omp_large_cap_mem_alloc(traits), omp_const_mem_alloc(traits), omp_high_bw_mem_alloc(traits), omp_low_lat_mem_alloc(traits), omp_cgroup_mem_alloc(traits), omp_pteam_mem_alloc(traits), omp_thread_mem_alloc(traits)) // expected-error 8 {{predefined allocator cannot have traits specified}} expected-note-re 8 {{predefined trait '{{omp_default_mem_alloc|omp_large_cap_mem_alloc|omp_const_mem_alloc|omp_high_bw_mem_alloc|omp_low_lat_mem_alloc|omp_cgroup_mem_alloc|omp_pteam_mem_alloc|omp_thread_mem_alloc}}' used here}}
#pragma omp target uses_allocators(omp_default_mem_alloc(my_traits), omp_large_cap_mem_alloc(my_traits), omp_const_mem_alloc(my_traits), omp_high_bw_mem_alloc(my_traits), omp_low_lat_mem_alloc(my_traits), omp_cgroup_mem_alloc(my_traits), omp_pteam_mem_alloc(my_traits), omp_thread_mem_alloc(my_traits)) // expected-error 8 {{predefined allocator cannot have traits specified}} expected-note-re 8 {{predefined trait '{{omp_default_mem_alloc|omp_large_cap_mem_alloc|omp_const_mem_alloc|omp_high_bw_mem_alloc|omp_low_lat_mem_alloc|omp_cgroup_mem_alloc|omp_pteam_mem_alloc|omp_thread_mem_alloc}}' used here}}
{}
#pragma omp target uses_allocators(my_alloc, c_my_alloc) // expected-error {{non-predefined allocator must have traits specified}} expected-error {{expected variable of the 'omp_allocator_handle_t' type, not 'const omp_allocator_handle_t' (aka 'void **const')}}
{}
Expand All @@ -46,10 +55,20 @@ int main(int argc, char **argv) {
{}
#pragma omp target uses_allocators(my_alloc(ptraits)) // expected-error {{expected constant sized array of 'omp_alloctrait_t' elements, not 'omp_alloctrait_t *'}}
{}
#pragma omp target uses_allocators(my_alloc(traits)) private(my_alloc) // expected-error {{allocators used in 'uses_allocators' clause cannot appear in other data-sharing or data-mapping attribute clauses}} expected-note {{defined as private}}
#pragma omp target uses_allocators(my_alloc(my_traits)) private(my_alloc) // expected-error {{allocators used in 'uses_allocators' clause cannot appear in other data-sharing or data-mapping attribute clauses}} expected-note {{defined as private}}
{}
#pragma omp target map(my_alloc, my_traits) uses_allocators(my_alloc(my_traits)) // expected-error {{allocators used in 'uses_allocators' clause cannot appear in other data-sharing or data-mapping attribute clauses}} expected-note {{used here}}
{}
#else
#pragma omp target teams uses_allocators(traits(my_traits): my_alloc; traits(aligned_traits): aligned_alloc)
{}
#pragma omp target teams uses_allocators(my_alloc)
{}
#pragma omp target teams uses_allocators(traits(my_traits): my_alloc, traits(aligned_traits): aligned_alloc) // omp52-error {{',' not allowed as separator in 'uses_allocators' clause, use ';' instead}}
{}
#pragma omp target map(my_alloc, traits) uses_allocators(my_alloc(traits)) // expected-error {{allocators used in 'uses_allocators' clause cannot appear in other data-sharing or data-mapping attribute clauses}} expected-note {{used here}}
#pragma omp target teams uses_allocators(my_alloc(my_traits), aligned_alloc(aligned_traits)) // omp52-error {{old syntax 'allocator(expr)' on 'uses_allocators' clause was deprecated, use new syntax 'traits(expr): alloc'}} //omp52-error {{old syntax 'allocator(expr)' on 'uses_allocators' clause was deprecated, use new syntax 'traits(expr): alloc'}}
{}
#endif
return 0;
}