This is an archive of the discontinued LLVM Phabricator instance.

[OpenMP] Change target memory tests to use allocators
ClosedPublic

Authored by jhuber6 on Apr 6 2022, 12:23 PM.

Details

Summary

The target allocators have been supported for NVPTX offloading for
awhile. The tests should use the allocators instead of calling the
functions manually. Also the comments indicating these being a preview
should be removed.

Diff Detail

Event Timeline

jhuber6 created this revision.Apr 6 2022, 12:23 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 6 2022, 12:23 PM
jhuber6 requested review of this revision.Apr 6 2022, 12:23 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 6 2022, 12:23 PM
jdoerfert added inline comments.Apr 7 2022, 8:07 AM
openmp/libomptarget/test/api/omp_device_managed_memory.c
11 ↗(On Diff #420970)

This sounds so confusing to me. Shared mem alloc, is not using shared/local memory but managed memory, right?
Just to make sure. Maybe leave a comment.

openmp/libomptarget/test/api/omp_host_pinned_memory.c
33 ↗(On Diff #420970)

Keep the old tests around please. No need to drop support for the function just yet.

jhuber6 added inline comments.Apr 7 2022, 8:37 AM
openmp/libomptarget/test/api/omp_device_managed_memory.c
11 ↗(On Diff #420970)

That's how they were described in the original patch, it's supposed to be "shared" between the host and device I guess.

openmp/libomptarget/test/api/omp_host_pinned_memory.c
33 ↗(On Diff #420970)

But the allocators use the function internally, this tests both that function and the allocator

jhuber6 updated this revision to Diff 421236.Apr 7 2022, 9:01 AM

Making suggested changes.

This revision is now accepted and ready to land.Apr 7 2022, 11:14 AM
This revision was automatically updated to reflect the committed changes.