This is an archive of the discontinued LLVM Phabricator instance.

[OpenMP] Add option to assert no nested OpenMP parallelism on the GPU
ClosedPublic

Authored by jhuber6 on Aug 17 2022, 2:47 PM.

Details

Summary

The OpenMP device runtime needs to support the OpenMP standard. However
constructs like nested parallelism are very uncommon in real application
yet lead to complexity in the runtime that is sometimes difficult to
optimize out. As a stop-gap for performance we should supply an argument
that selectively disables this feature. This patch adds the
-fopenmp-assume-no-nested-parallelism argument which explicitly
disables the usee of nested parallelism in OpenMP.

Diff Detail

Event Timeline

jhuber6 created this revision.Aug 17 2022, 2:47 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 17 2022, 2:47 PM
jhuber6 requested review of this revision.Aug 17 2022, 2:47 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptAug 17 2022, 2:47 PM
jhuber6 updated this revision to Diff 453474.Aug 17 2022, 4:33 PM

Tweak ordering of comparisons.

This looks good, but what happens when the user accidentally adds a nested parallel when this option is turned on? Do we get serial (correct) execution?

This looks good, but what happens when the user accidentally adds a nested parallel when this option is turned on? Do we get serial (correct) execution?

With the code as it is, it will simply ignore the level and continue executing. This will probably be broken as we don't adjust some of the other state variables for this. The flag more-so asserts that nested parallelism will not work in any capacity than we reduce them to a single thread. We could potentially make it work, but it would be more complicated. There's an assertion if nested parallelism is attempted while being disabled, but this requires the user checking the assertions via -fopenmp-debug.

carlo.bertolli accepted this revision.Aug 22 2022, 7:27 AM

Thanks for explaining. LGTM.

This revision is now accepted and ready to land.Aug 22 2022, 7:27 AM
jhuber6 updated this revision to Diff 454845.Aug 23 2022, 7:55 AM

Fix missing () in assertion and accidentally deleting device libs addition.

This revision was landed with ongoing or failed builds.Aug 23 2022, 12:10 PM
This revision was automatically updated to reflect the committed changes.