This is an archive of the discontinued LLVM Phabricator instance.

[OpenMP] Add NVIDIA annotations for static grid thread limit
ClosedPublic

Authored by jdoerfert on Aug 20 2023, 8:29 PM.

Details

Summary

We already add AMD GPU annotations, the NVIDIA ones are just a little
more convoluted to add/update but otherwise the same.
We see again that the interplay of ompx_attribute and deduced value
needs to be improved, see the TODO.

Diff Detail

Event Timeline

jdoerfert created this revision.Aug 20 2023, 8:29 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 20 2023, 8:29 PM
jdoerfert requested review of this revision.Aug 20 2023, 8:29 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 20 2023, 8:29 PM
jdoerfert updated this revision to Diff 551889.Aug 20 2023, 8:50 PM

I'm assuming merging these attributes is more complicated as we have code in clang and LLVM that handle it separately? It's unfortunate that the Nvidia backend uses module metadata for this.

I'm assuming merging these attributes is more complicated as we have code in clang and LLVM that handle it separately? It's unfortunate that the Nvidia backend uses module metadata for this.

Yes and no. That complication exists for AMD as well and causes the todo in https://reviews.llvm.org/D158382. The solution to both problems is somewhat clear and somewhat murky. If we would do the clang handling first, we could remember the upper bounds and do the llvm handling after. Right now it is the other way around and the clang handling would need to be 1) made aware of existing upper bounds, and 2) made aware of existing annotations (NVIDIA only).

I'm thinking about how to solve this best. I believe for now this is not a big problem and the progress is worth it.

jhuber6 accepted this revision.Aug 21 2023, 8:48 AM

Fair enough, we probably need to put a lot of this stuff in a utility somewhere.

This revision is now accepted and ready to land.Aug 21 2023, 8:48 AM
This revision was landed with ongoing or failed builds.Aug 23 2023, 11:14 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptAug 23 2023, 11:14 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript