This is an archive of the discontinued LLVM Phabricator instance.

[OpenMP] Fix `present` diagnostic for array extension
ClosedPublic

Authored by jdenny on Aug 5 2020, 8:45 AM.

Details

Summary

For example, without this patch, the following fails as expected with
or without the present modifier, but the present modifier doesn't
produce its usual diagnostic:

#pragma omp target data map(alloc: arr[0:2])
{
  #pragma omp target map(present, tofrom: arr[0:100]) // not fully present
  ;
}

Diff Detail

Event Timeline

jdenny created this revision.Aug 5 2020, 8:45 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 5 2020, 8:45 AM
jdenny requested review of this revision.Aug 5 2020, 8:45 AM
grokos added inline comments.Aug 5 2020, 11:19 AM
openmp/libomptarget/src/device.cpp
194–202

On second thoughts, maybe this should also become a MESSAGE. It's a diagnostic which will help the user understand why the present-related message follows.

vzakhari added inline comments.
openmp/libomptarget/src/device.cpp
197

Can you please use PRId64 instead? '%ld' tends to break Windows builds.

jdenny added inline comments.Aug 5 2020, 12:04 PM
openmp/libomptarget/src/device.cpp
194–202

Are you suggesting it generally or only when HasPresentModifier?

197

Thanks for catching that. I'll fix this here and propose another patch to fix all the other places I made this mistake.

grokos added inline comments.Aug 5 2020, 12:10 PM
openmp/libomptarget/src/device.cpp
194–202

Generally.

197

I'm preparing a patch to replace all instances of %ld with PRId64 for int64_t variables throughout libomptarget. There are many more instances than introduced by your patches.

jdenny added inline comments.Aug 5 2020, 12:13 PM
openmp/libomptarget/src/device.cpp
197

Thanks. I'll just fix this one then.

jdenny updated this revision to Diff 283364.Aug 5 2020, 1:17 PM

Add user diagnostic for mapping extension. Use PRId64 where appropriate.

jdenny marked 3 inline comments as done.Aug 5 2020, 1:18 PM
vzakhari added inline comments.Aug 5 2020, 1:19 PM
openmp/libomptarget/src/device.cpp
197

Thank you!

grokos accepted this revision.Aug 5 2020, 1:19 PM

LGTM. Thanks!

This revision is now accepted and ready to land.Aug 5 2020, 1:19 PM
jdenny added a comment.Aug 5 2020, 1:23 PM

Thanks for the quick reviews!

This revision was automatically updated to reflect the committed changes.