This is an archive of the discontinued LLVM Phabricator instance.

[NFC][OpenMP] Update description of OMPGridValues enums
ClosedPublic

Authored by saiislam on Aug 19 2020, 10:39 AM.

Details

Summary

Update comments describing how OMPGridValues enums will be used in
clang, deviceRTLs, and hsa and cuda plugins.

Diff Detail

Event Timeline

saiislam created this revision.Aug 19 2020, 10:39 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 19 2020, 10:39 AM
saiislam requested review of this revision.Aug 19 2020, 10:39 AM
JonChesterfield added a comment.EditedAug 19 2020, 10:46 AM

This can't be used in the libomptarget plugins or in the deviceRTL, as both are required to compile without the llvm sources available.

Current preferences appears to be to copy and paste the numbers between the two files. At the least, we should have a test outside of llvm & openmp trees that verifies the numbers are the same in each.

@jdoerfert can you remember who was strongly against #include between openmp and llvm? I'm failing to find the discussion, and the mail "Sharing enums between llvm and openmp" didn't reach a conclusion

This can't be used in the libomptarget plugins or in the deviceRTL, as both are required to compile without the llvm sources available.

Current preferences appears to be to copy and paste the numbers between the two files. At the least, we should have a test outside of llvm & openmp trees that verifies the numbers are the same in each.

@jdoerfert can you remember who was strongly against #include between openmp and llvm? I'm failing to find the discussion, and the mail "Sharing enums between llvm and openmp" didn't reach a conclusion

What about if we put these gridvalues in OMPKinds.def (or OMP.td)?
Ultimately we need a common place to share content with clang, RTLs, and plugins.

Ultimately we need a common place to share content with clang, RTLs, and plugins.

That was my hope.

Ping.
Merged patch D87841 uses the Gridvalue as described here.

This revision is now accepted and ready to land.Oct 15 2020, 1:36 PM
This revision was automatically updated to reflect the committed changes.