This is an archive of the discontinued LLVM Phabricator instance.

[OpenMP] [OMPT] [8/8] Added lit tests for OMPT target callbacks
ClosedPublic

Authored by dhruvachak on Jun 8 2022, 6:58 PM.

Details

Summary

Added a new target ompt mode that depends on libomptarget OMPT support.
Added tests that verify callbacks for target regions, kernel launch,
and data transfer operations. All of them should pass on amdgpu using
make check-libomptarget.

Diff Detail

Event Timeline

dhruvachak created this revision.Jun 8 2022, 6:58 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 8 2022, 6:58 PM
Herald added subscribers: kosarev, tpr. · View Herald Transcript
dhruvachak requested review of this revision.Jun 8 2022, 6:58 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 8 2022, 6:58 PM
dhruvachak retitled this revision from [OMPT] Added lit tests for OMPT target callbacks to [OpenMP] [OMPT] [8/8] Added lit tests for OMPT target callbacks.Jun 20 2022, 11:06 PM
jplehr added a subscriber: jplehr.Dec 1 2022, 8:04 AM
mhalk added a subscriber: mhalk.Feb 10 2023, 1:22 AM
mhalk updated this revision to Diff 499494.Feb 22 2023, 7:03 AM

Rebased

@mhalk Can this be landed on top of trunk today? I would be interested in landing this one on top of what we have today so that we get some test coverage as part of "make check". You will need to update the tests to match the current code and update the tests as patches 4-7 land.

I see this patch still depends on D127367. Can you remove this dependency, update the CHECK lines if not done already, and verify?

Thanks.

mhalk updated this revision to Diff 501245.Feb 28 2023, 11:40 AM

Removed dependency on previous patches.
Note: Tests will now check for failed callback registrations.

mhalk edited the summary of this revision. (Show Details)Feb 28 2023, 11:41 AM
mhalk updated this revision to Diff 501249.Feb 28 2023, 11:49 AM

Fixed update.

@jhuber6 Is this patch ok to land? This will provide some test coverage to the existing OMPT target functionality, the device-independent part. As plugin-specific changes land, these tests will have to be updated.

Just to reiterate what I think this does is: It adds tests which currently ensure that the call backs can not be registered (as it should).
The tests would then be updated for the other patches, when these call backs can be registered, right?

Just to reiterate what I think this does is: It adds tests which currently ensure that the call backs can not be registered (as it should).
The tests would then be updated for the other patches, when these call backs can be registered, right?

That's right. So landing this would test existing OMPT target functionality.

jplehr accepted this revision.Mar 9 2023, 2:11 AM

LGTM

This revision is now accepted and ready to land.Mar 9 2023, 2:11 AM