This is an archive of the discontinued LLVM Phabricator instance.

Differential D116540

[OpenMP] Add nec and nvidia as compiler vendors for OpenMP
ClosedPublic

Authored by saiislam on Jan 3 2022, 8:59 AM.

Details

Reviewers
jdoerfert
JonChesterfield
ye-luo
Meinersbur
tra
Commits
rG49f23afdc345: [OpenMP] Add nec and nvidia as compiler vendors for OpenMP
Summary

OpenMP Specs 5.0[1] and 5.1[2] recognizes nec and nvidia as known
compiler vendors and their absence is causing compilation error in one
of the vendor based metadirective test of sollve_vv project[3].

[1] https://www.openmp.org/wp-content/uploads/Context-Definitions-5.0-v1.0.pdf
[2] https://www.openmp.org/wp-content/uploads/OpenMP-API-Additional-Definitions-2-0.pdf
[3] https://github.com/SOLLVE/sollve_vv/blob/master/tests/5.0/metadirective/test_metadirective_arch_nvidia_or_amd.c

Diff Detail

Unit TestsFailed

TimeTest
40 msx64 debian > Clang.OpenMP::begin_declare_variant_messages.c
50 msx64 debian > Clang.OpenMP::declare_variant_messages.c
70 msx64 debian > Clang.OpenMP::declare_variant_messages.cpp
80 msx64 debian > LLVM.Bindings/Go::go.test

Event Timeline

saiislam created this revision.Jan 3 2022, 8:59 AM
Herald added subscribers: guansong, yaxunl. · View Herald TranscriptJan 3 2022, 8:59 AM
saiislam requested review of this revision.Jan 3 2022, 8:59 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 3 2022, 8:59 AM
Herald added subscribers: llvm-commits, sstefan1. · View Herald Transcript
saiislam added a subscriber: sandoval.Jan 3 2022, 9:05 AM
  1. I don't know why "pgi" is present here as a vendor. Should it be renamed as nvidia instead? @Meinersbur @tra
  2. Should "cray" be renamed as "hpe" here? @sandoval
  3. Should this vendor list be reordered as per Spec 5.1 [https://www.openmp.org/wp-content/uploads/OpenMP-API-Additional-Definitions-2-0.pdf] ?
jdoerfert accepted this revision.Jan 3 2022, 9:13 AM
In D116540#3217684, @saiislam wrote:
  1. I don't know why "pgi" is present here as a vendor. Should it be renamed as nvidia instead? @Meinersbur @tra

It was in the spec list and I would not remove it now.

  1. Should "cray" be renamed as "hpe" here? @sandoval

No, both are listed in the spec.

  1. Should this vendor list be reordered as per Spec 5.1 [https://www.openmp.org/wp-content/uploads/OpenMP-API-Additional-Definitions-2-0.pdf] ?

It was ordered as per that spec, which is just alphabetical. Can you add nec as well and keep the others please.

LG, see above.

This revision is now accepted and ready to land.Jan 3 2022, 9:13 AM
Harbormaster completed remote builds in B141338: Diff 397077.Jan 3 2022, 10:16 AM
saiislam updated this revision to Diff 397108.Jan 3 2022, 11:02 AM
  1. Added nec as compiler vendor.
  2. Fixed clang test cases.
Herald added a project: Restricted Project. · View Herald TranscriptJan 3 2022, 11:02 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
saiislam retitled this revision from [OpenMP] Add nvidia as a compiler vendor for OpenMP to [OpenMP] Add nec and nvidia as compiler vendors for OpenMP.Jan 3 2022, 11:03 AM
saiislam edited the summary of this revision. (Show Details)
saiislam added a comment.Jan 3 2022, 11:12 AM
In D116540#3217689, @jdoerfert wrote:
In D116540#3217684, @saiislam wrote:
  1. I don't know why "pgi" is present here as a vendor. Should it be renamed as nvidia instead? @Meinersbur @tra

It was in the spec list and I would not remove it now.

Specs 5.0 and 5.1 linked above doesn't contain PGI as a vendor. But, we can leave it as is.

  1. Should "cray" be renamed as "hpe" here? @sandoval

No, both are listed in the spec.

Ok.

  1. Should this vendor list be reordered as per Spec 5.1 [https://www.openmp.org/wp-content/uploads/OpenMP-API-Additional-Definitions-2-0.pdf] ?

It was ordered as per that spec, which is just alphabetical. Can you add nec as well and keep the others please.

Spec 5.1 has moved "unknown" from last position to the first. Some reordering also due to "hpe or cray" thing as well.

LG, see above.

Done.

Harbormaster completed remote builds in B141360: Diff 397108.Jan 3 2022, 12:04 PM
This revision was landed with ongoing or failed builds.Jan 4 2022, 4:31 AM
Closed by commit rG49f23afdc345: [OpenMP] Add nec and nvidia as compiler vendors for OpenMP (authored by saiislam). · Explain Why
This revision was automatically updated to reflect the committed changes.
saiislam added a commit: rG49f23afdc345: [OpenMP] Add nec and nvidia as compiler vendors for OpenMP.

Revision Contents

PathSize
llvm/
include/
llvm/
Frontend/
OpenMP/
1 line

Diff 397077

llvm/include/llvm/Frontend/OpenMP/OMPKinds.def

Show First 20 Lines • Show All 1,124 Lines • ▼ Show 20 Lines
__OMP_TRAIT_PROPERTY(implementation, vendor, arm) __OMP_TRAIT_PROPERTY(implementation, vendor, arm)
__OMP_TRAIT_PROPERTY(implementation, vendor, bsc) __OMP_TRAIT_PROPERTY(implementation, vendor, bsc)
__OMP_TRAIT_PROPERTY(implementation, vendor, cray) __OMP_TRAIT_PROPERTY(implementation, vendor, cray)
__OMP_TRAIT_PROPERTY(implementation, vendor, fujitsu) __OMP_TRAIT_PROPERTY(implementation, vendor, fujitsu)
__OMP_TRAIT_PROPERTY(implementation, vendor, gnu) __OMP_TRAIT_PROPERTY(implementation, vendor, gnu)
__OMP_TRAIT_PROPERTY(implementation, vendor, ibm) __OMP_TRAIT_PROPERTY(implementation, vendor, ibm)
__OMP_TRAIT_PROPERTY(implementation, vendor, intel) __OMP_TRAIT_PROPERTY(implementation, vendor, intel)
__OMP_TRAIT_PROPERTY(implementation, vendor, llvm) __OMP_TRAIT_PROPERTY(implementation, vendor, llvm)
__OMP_TRAIT_PROPERTY(implementation, vendor, nvidia)
__OMP_TRAIT_PROPERTY(implementation, vendor, pgi) __OMP_TRAIT_PROPERTY(implementation, vendor, pgi)
__OMP_TRAIT_PROPERTY(implementation, vendor, ti) __OMP_TRAIT_PROPERTY(implementation, vendor, ti)
__OMP_TRAIT_PROPERTY(implementation, vendor, unknown) __OMP_TRAIT_PROPERTY(implementation, vendor, unknown)
__OMP_TRAIT_SELECTOR(implementation, extension, true) __OMP_TRAIT_SELECTOR(implementation, extension, true)
__OMP_TRAIT_PROPERTY(implementation, extension, match_all) __OMP_TRAIT_PROPERTY(implementation, extension, match_all)
__OMP_TRAIT_PROPERTY(implementation, extension, match_any) __OMP_TRAIT_PROPERTY(implementation, extension, match_any)
__OMP_TRAIT_PROPERTY(implementation, extension, match_none) __OMP_TRAIT_PROPERTY(implementation, extension, match_none)
▲ Show 20 Lines • Show All 85 Lines • Show Last 20 Lines