This is an archive of the discontinued LLVM Phabricator instance.

[OpenMP][Tests] Enable nvptx64 testing for most libomptarget tests
ClosedPublic

Authored by protze.joachim on Jul 24 2020, 2:27 PM.

Details

Summary

This patch adds a run line to most libomptarget tests to also test the nvptx64 target, if available.

Adding $BUILD/lib to the LIBRARY_PATH fixes https://bugs.llvm.org/show_bug.cgi?id=46836.

I moved the existing runline for nvptx64 to the end. Lit will stop for the first failing runline and mark the test as failed. If the GPU is configured for exclusive mode, this line tends to fail quite often.

None of the newly enabled tests fail on my system.

Diff Detail

Event Timeline

protze.joachim created this revision.Jul 24 2020, 2:27 PM
JonChesterfield accepted this revision.Jul 27 2020, 6:03 AM
JonChesterfield added a subscriber: JonChesterfield.

Not Johannes, but LGTM. I didn't realise our tests could pick up a different runtime to the one just built, that was a nasty failure mode. Thanks for the patch

This revision is now accepted and ready to land.Jul 27 2020, 6:03 AM

Thanks for reviewing.

At the time of the commit 9 tests are failing during offloading to V100:

Broken because of D83963, @jdenny proposed a fix, which works for me:

libomptarget :: mapping/alloc_fail.c
libomptarget :: mapping/present/target.c
libomptarget :: mapping/present/target_data.c
libomptarget :: mapping/present/target_enter_data.c
libomptarget :: mapping/present/target_exit_data.c
libomptarget :: mapping/present/zero_length_array_section.c
libomptarget :: mapping/present/zero_length_array_section_exit.c

Broken since D68100, solution seems to be unclear:

libomptarget :: mapping/declare_mapper_target.cpp

Fix is discussed in D84470, patch works for me:

libomptarget :: offloading/target_depend_nowait.cpp
jdoerfert added a subscriber: hans.Jul 28 2020, 5:13 AM

Thanks for reviewing.

At the time of the commit 9 tests are failing during offloading to V100:

Broken because of D83963, @jdenny proposed a fix, which works for me:

libomptarget :: mapping/alloc_fail.c
libomptarget :: mapping/present/target.c
libomptarget :: mapping/present/target_data.c
libomptarget :: mapping/present/target_enter_data.c
libomptarget :: mapping/present/target_exit_data.c
libomptarget :: mapping/present/zero_length_array_section.c
libomptarget :: mapping/present/zero_length_array_section_exit.c

Broken since D68100, solution seems to be unclear:

libomptarget :: mapping/declare_mapper_target.cpp

Fix is discussed in D84470, patch works for me:

libomptarget :: offloading/target_depend_nowait.cpp

Thanks. Could you ask @hans to include this in the release (via your bug) if you haven't already?

Broken because of D83963, @jdenny proposed a fix, which works for me:

libomptarget :: mapping/alloc_fail.c
libomptarget :: mapping/present/target.c
libomptarget :: mapping/present/target_data.c
libomptarget :: mapping/present/target_enter_data.c
libomptarget :: mapping/present/target_exit_data.c
libomptarget :: mapping/present/zero_length_array_section.c
libomptarget :: mapping/present/zero_length_array_section_exit.c

I've pushed it to master as 9b4826d18b5f.

Thanks. Could you ask @hans to include this in the release (via your bug) if you haven't already?

The release branch has no libomptarget tests with run lines for nvptx64. Therefore the release branch is not affected.

I'm having second thoughts about this.

It works because clang searches LIBRARY_PATH first when looking for the deviceRTL. That means a system that has LIBRARY_PATH accidentally/incidentally pointing to a different toolchain to the one that contains the running clang will pick up the wrong library. That's relatively likely to fail in opaque fashion.

Would you accept changing the tests to pass the exact location of the deviceRTL to use instead? The command line flag is already there, and wins out over LIBRARY_PATH. Closely related, I'd like to delete the behaviour of checking the LIBRARY_PATH to find the deviceRTL to use with a particular clang, so that only the people who consciously choose to point it at a different device runtime experience the result. LIBRARY_PATH is too easy to set accidentally and/or forget about.