Page MenuHomePhabricator

[OpenMP] Add lit test for metadirective device arch inspired from sollve
ClosedPublic

Authored by animeshk-amd on Aug 12 2022, 5:29 AM.

Details

Summary

This lit test is added based upon the tests present in the
tests/5.0/metadirective directory of the SOLLVE repo
https://github.com/SOLLVE/sollve_vv

Diff Detail

Event Timeline

animeshk-amd created this revision.Aug 12 2022, 5:29 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 12 2022, 5:29 AM
animeshk-amd requested review of this revision.Aug 12 2022, 5:29 AM
Herald added a project: Restricted Project. · View Herald Transcript

This doesn't actually test much, only once case/compilation is covered. In the second function nothing specific to LLVM as impl is checked.

This doesn't actually test much, only once case/compilation is covered. In the second function nothing specific to LLVM as impl is checked.

The second function, is the only place in llvm-project where vendor(llvm) is being tested for a non-error test.

This doesn't actually test much, only once case/compilation is covered. In the second function nothing specific to LLVM as impl is checked.

The second function, is the only place in llvm-project where vendor(llvm) is being tested for a non-error test.

Really?

ag 'vendor\(llvm\)' clang/test/OpenMP --files-with-matches                                                                                                                                                                

clang/test/OpenMP/begin_declare_variant_messages.c
clang/test/OpenMP/begin_declare_variant_using_messages.cpp
clang/test/OpenMP/declare_variant_ast_print.c
clang/test/OpenMP/declare_variant_ast_print.cpp
clang/test/OpenMP/declare_variant_implementation_vendor_codegen.cpp
clang/test/OpenMP/declare_variant_messages.cpp
clang/test/OpenMP/declare_variant_mixed_codegen.cpp
clang/test/OpenMP/metadirective_ast_print.c
clang/test/OpenMP/metadirective_implementation_codegen.c
clang/test/OpenMP/nvptx_declare_variant_implementation_vendor_codegen.cpp
clang/test/OpenMP/declare_variant_messages.c
clang/test/OpenMP/metadirective_empty.cpp
clang/test/OpenMP/metadirective_implementation_codegen.cpp

That said, the above function still doesn't test anything wrt. llvm as impl anyway. We could just as well match amd or nvidia and the check lines still match just fine.

This doesn't actually test much, only once case/compilation is covered. In the second function nothing specific to LLVM as impl is checked.

The second function, is the only place in llvm-project where vendor(llvm) is being tested for a non-error test.

Really?

ag 'vendor\(llvm\)' clang/test/OpenMP --files-with-matches                                                                                                                                                                

clang/test/OpenMP/begin_declare_variant_messages.c
clang/test/OpenMP/begin_declare_variant_using_messages.cpp
clang/test/OpenMP/declare_variant_ast_print.c
clang/test/OpenMP/declare_variant_ast_print.cpp
clang/test/OpenMP/declare_variant_implementation_vendor_codegen.cpp
clang/test/OpenMP/declare_variant_messages.cpp
clang/test/OpenMP/declare_variant_mixed_codegen.cpp
clang/test/OpenMP/metadirective_ast_print.c
clang/test/OpenMP/metadirective_implementation_codegen.c
clang/test/OpenMP/nvptx_declare_variant_implementation_vendor_codegen.cpp
clang/test/OpenMP/declare_variant_messages.c
clang/test/OpenMP/metadirective_empty.cpp
clang/test/OpenMP/metadirective_implementation_codegen.cpp

That said, the above function still doesn't test anything wrt. llvm as impl anyway. We could just as well match amd or nvidia and the check lines still match just fine.

My mistake. You are right about more tests being present. I searched in the wrong branch locally :-)

The second function was testing the implementation trait set for
the vendor trait with respect to the metadirective directive.
After Johannes's comment, on further searching it turns out that
this trait and trait set are already being tested in other tests.
Therfore second function need not be there. Also, the ast_print
test for the first function is added.

Updating D131763: [OpenMP] Add lit test for metadirective device arch inspired

from sollve

saiislam accepted this revision.Sep 5 2022, 3:13 AM

LGTM. Thanks!

This revision is now accepted and ready to land.Sep 5 2022, 3:13 AM

Thanks @saiislam. Since I don't have the commit access yet, I request you to kindly commit this on my behalf.

This revision was automatically updated to reflect the committed changes.