This is an archive of the discontinued LLVM Phabricator instance.

[OpenMP][Tests] Fix/Mark compatibilty for GCC
ClosedPublic

Authored by protze.joachim on Jun 20 2020, 3:05 PM.

Details

Summary

Some of the newly added tests are not compatible with GCC as test compiler.

runtime/test/ompt/tasks/dependences.c

is reverted to the old pre 63a3c5925 test (without mutexinoutset)

runtime/test/ompt/tasks/dependences_mtxinoutset.c

This test is the 63a3c5925 version of dependences.c, is not compatible with gcc and needs clang-9+

runtime/test/ompt/worksharing/for/runtime*.c

These tests use GOMP_parallel_loop_maybe_nonmonotonic_runtime starting with gcc-9

tools/multiplex/tests/*

These tests use the master construct. GCC does not generate events for that -> expect to fail

Diff Detail

Event Timeline

protze.joachim created this revision.Jun 20 2020, 3:05 PM

LGTM, would like Saiyed and Johannes to review and approve

I found another issue with this test (addressing Saiyedul's error). I'll post an updated patch in a minute

The intention was to check the return address for the first two tasks. Unfortunately, the CHECK did not capture the return address of the second task creation, but instead compared to the return address of the first task.

saiislam accepted this revision.Jul 2 2020, 7:44 AM

LGTM. Thank you!
Please link any ongoing/future effort to enable support for gcc-9/10 for the failing tests, as and when it materializes.

Please wait for @jdoerfert 's review.

This revision is now accepted and ready to land.Jul 2 2020, 7:44 AM

Sorry, in my previous update I missed to include the other changes (diff to the wrong commit)

Now, I also added compatibility flags for gcc-10

The summary also talks about tools/multiplex/tests/*, is that information obsolete?

openmp/runtime/test/ompt/tasks/dependences_mtxinoutset.c
3–6 ↗(On Diff #275198)

I would put the comment in front of the UNSUPPORTED and possibly split into gcc and clang-4, .... Also can we rename the test file to the full mutexinoutset? That's two additional characters, but easier to search for.

Addressed @Hahnfeld 's comments

This revision was automatically updated to reflect the committed changes.