[OpenMP][Clang] Add support for OMP6.1 fb_nullify and fb_preserve in need_device_ptr modifier #168905
Conversation
…in need_device_ptr modifier
zahiraam
changed the title
🐧 Linux x64 Test Results
|
abhinavgaba
changed the title
| @@ -274,10 +264,12 @@ OPENMP_DOACROSS_MODIFIER(source_omp_cur_iteration) | |||
| OPENMP_THREADSET_KIND(omp_pool) | |||
| OPENMP_THREADSET_KIND(omp_team) | |||
|
|
|||
| // OpenMP 6.1 modifiers for 'adjust_args' clause. | |||
| OPENMP_NEEDDEVICE_MODIFIER(fp_nullify) | |||
There was a problem hiding this comment.
Typo: fp should be fb.
| @@ -274,10 +264,12 @@ OPENMP_DOACROSS_MODIFIER(source_omp_cur_iteration) | |||
| OPENMP_THREADSET_KIND(omp_pool) | |||
| OPENMP_THREADSET_KIND(omp_team) | |||
|
|
|||
| // OpenMP 6.1 modifiers for 'adjust_args' clause. | |||
| OPENMP_NEEDDEVICE_MODIFIER(fp_nullify) | |||
There was a problem hiding this comment.
We should be specific and use OPENMP_NEED_DEVICE_PTR_MODIFIER, since the fallback is only applicable to need_device_ptr, not need_device_addr.
| @@ -5010,6 +4932,37 @@ bool Parser::ParseOpenMPVarList(OpenMPDirectiveKind DKind, | |||
| ConsumeToken(); | |||
| if (Tok.is(tok::colon)) | |||
| Data.ColonLoc = Tok.getLocation(); | |||
| if (getLangOpts().OpenMP >= 61) { | |||
| // Handle optional need_device_ptr modifier. | |||
There was a problem hiding this comment.
We should say "Handle the optional fallback argument for the need_device_ptr modifier."
| void __attribute__((noinline)) device_impl(int *xp, int *&xpref, int n) {} | ||
|
|
||
| #pragma omp declare variant(device_impl) \ | ||
| adjust_args(need_device_ptr(foo) : xp, xpref) // expected-error{{unknown modifier in 'need_device_ptr' clause (OpenMP 6.1 or later only)}} // expected-error{{expected 'match', 'adjust_args', or 'append_args' clause on 'omp declare variant' directive}} |
There was a problem hiding this comment.
The error message is not really accurate. The clause is adjust_args, modifier is need_device_ptr, and foo here is an argument to the need_device_ptr modifier.
Ideally, we should say something like:
unknown argument for 'need_device_ptr' complex-modifier.
Expected 'fb_nullify' or 'fb_preserve'.
Otherwise, at the very least we should say unknown modifier in 'adjust_args' clause.
| : OMPC_NEEDDEVICE_fp_preserve; | ||
| } else { | ||
| Diag(Tok, diag::err_omp_unknown_need_device_ptr_modifier) | ||
| << (getLangOpts().OpenMP >= 60 ? 1 : 0); |
There was a problem hiding this comment.
getLangOpts().OpenMP >= 60 should it be >=61. Line 4935 checks for >=61.
|
|
||
| void __attribute__((noinline)) device_impl(int *xp, int *&xpref, int n) {} | ||
|
|
||
| #pragma omp declare variant(device_impl) \ |
There was a problem hiding this comment.
I would add 2/3 more cases:
#pragma omp declare variant(device_impl) \
adjust_args(need_device_ptr(fb_nullify) : xp, xpref) // no error
void __attribute__((noinline)) host_entry_a(int *xp, int *&xpref, int n) {}
#pragma omp declare variant(device_impl) \
adjust_args(need_device_ptr(fb_preserve : xp, xpref) // no error
void __attribute__((noinline)) host_entry_a(int *xp, int *&xpref, int n) {} // expected `)`
There was a problem hiding this comment.
Also, I think the LIT tests name should start with need_device_ptr_modifier_<messages/ast_print>.cpp
3 participants
No description provided.