This is an archive of the discontinued LLVM Phabricator instance.

[OpenMP][libomptarget] Emit runtime message when variable is incorrectly mapped to device
ClosedPublic

Authored by doru1004 on Sep 28 2022, 5:31 PM.

Details

Summary

Enable the runtime to emit a warning message when a mapped variable does not have an appropriate device counterpart that can be used. The current patch uses the use_device_addr as an example but other cases may be possible. The patch has been tested with the warning as an error to make sure that it does not disturb any of the existing tests. The warning currently uses MESSAGE but can be changed to REPORT if reviewers think it would be more appropriate.

Diff Detail

Event Timeline

doru1004 created this revision.Sep 28 2022, 5:31 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 28 2022, 5:31 PM
doru1004 requested review of this revision.Sep 28 2022, 5:31 PM
doru1004 updated this revision to Diff 463729.Sep 28 2022, 5:34 PM

I am not sure if we should generate this message. The spec says "Each list item must have a corresponding list item in the device data environment or be accessible on the target device."
The pointer may be in the unified_shared_memory.

trws added a comment.Sep 30 2022, 6:43 AM

The message seems useful, accidentally putting x there rather than x[:0] like the user probably intended is likely to be a relatively common mistake unfortunately. Would it be feasible to, when the message would be printed for a pointer-type variable, check if it's target is accessible and suggest the appropriate fix, or at least say "maybe you meant to use an array section, the array section target is valid"? That would make it immensely easier for an lower experience user to take action on.

The message seems useful, accidentally putting x there rather than x[:0] like the user probably intended is likely to be a relatively common mistake unfortunately. Would it be feasible to, when the message would be printed for a pointer-type variable, check if it's target is accessible and suggest the appropriate fix, or at least say "maybe you meant to use an array section, the array section target is valid"? That would make it immensely easier for an lower experience user to take action on.

I would agree to that sentiment. If the runtime can easily detect the issue and report, it should do so. Just letting the code die with a SEGFAULT is certainly fun for folks who love to debug, but is not really productive.

This message could be printed when LIBOMPTARGET_INFO="something". I don't think it should be printed by default as it could be false positive.

This message could be printed when LIBOMPTARGET_INFO="something". I don't think it should be printed by default as it could be false positive.

Happy to enable it as an INFO.

I am not sure if we should generate this message. The spec says "Each list item must have a corresponding list item in the device data environment or be accessible on the target device."
The pointer may be in the unified_shared_memory.

We don't emit the message when IsHostPtr is true (i.e. when USM is enabled and not under close modifier).

doru1004 updated this revision to Diff 466169.Oct 7 2022, 1:51 PM

@RaviNarayanaswamy can this one move forward to an LOTM Looks Ok To Me ? or better ?

Minor spelling otherwise LGTM

openmp/libomptarget/include/Debug.h
57 ↗(On Diff #466169)

cotunerpart -> counterpart

doru1004 updated this revision to Diff 517183.Apr 26 2023, 8:06 AM
doru1004 marked an inline comment as done.

@RaviNarayanaswamy LGTM, please formally approve if you are satsified , thx

ronlieb accepted this revision.Apr 27 2023, 3:05 PM
This revision is now accepted and ready to land.Apr 27 2023, 3:05 PM