This is an archive of the discontinued LLVM Phabricator instance.

[OpenMP][libomptarget][NFC] Add documentation regarding NextGen plugins
ClosedPublic

Authored by kevinsala on Feb 28 2023, 8:23 AM.

Details

Summary

Add a doc section regarding NextGen plugins and the available environment variables

Diff Detail

Event Timeline

kevinsala created this revision.Feb 28 2023, 8:23 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 28 2023, 8:23 AM
kevinsala requested review of this revision.Feb 28 2023, 8:23 AM
jdoerfert accepted this revision.Mar 1 2023, 9:16 AM

Nice! Thanks a lot!

openmp/docs/design/Runtimes.rst
1209

Nit: Format the paragraph to 80 columns.

1244

Is this separate from the other SIGNALS env var on purpose? We can discuss this in our meeting.

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

Should we bother calling them nextgen considering that they will be the only gen in two weeks?

Should we bother calling them nextgen considering that they will be the only gen in two weeks?

You're right. I'll update it be consistent with D142820.

openmp/docs/design/Runtimes.rst
1244

Which SIGNALS envars are you referring to?

There are the other two LIBOMPTARGET_NUM_INITIAL_STREAMS and LIBOMPTARGET_NUM_INITIAL_EVENTS envars. But these two are different since these two apply to all plugins and LIBOMPTARGET_AMDGPU_NUM_INITIAL_HSA_SIGNALS only applies to AMDGPU.

jplehr added a subscriber: jplehr.Mar 13 2023, 6:11 AM
jplehr added inline comments.
openmp/docs/design/Runtimes.rst
1244

Is there a specific reason why more HSA_SIGNALS are created initially than EVENTS?

kevinsala added inline comments.Mar 13 2023, 8:43 AM
openmp/docs/design/Runtimes.rst
1244

There is no specific reason. I did set that value at the beginning and didn't try if there are better ones. Maybe someone else tried other values, but I'm not aware of that.

Anyhow, the use of events/streams is slightly different than the HSA signals. The first two are exposed to libomptarget, whereas the HSA signals are resources used by the AMDGPU plugin internally.

kevinsala updated this revision to Diff 505085.Mar 14 2023, 7:22 AM

Removing most references to NextGen plugins and assuming this is the only plugin implementation available. Now it should be consistent with D142820. Fixing format too.

jhuber6 accepted this revision.Mar 14 2023, 7:23 AM