Page MenuHomePhabricator

Make tests unsupported if libatomic cannot be found.
Needs ReviewPublic

Authored by twoh on Jul 16 2018, 9:51 AM.

Details

Summary

As https://reviews.llvm.org/rL336467 appends -latomic when -fopenmp is given, openmp unit tests are failing if libatomic cannot be found. This diff makes unit tests unsupported if there's no libatomic.

  • Test

From the build dir

[~/llvms/upstream-git/build-Release] grep -r LIBOMP_HAVE_LIBATOMIC .
./CMakeCache.txt:LIBOMP_HAVE_LIBATOMIC:INTERNAL=

suggests that libatomic cannot be found. Without the diff,

[~/llvms/upstream-git/build-Release] ninja check-openmp
...
    libomp :: worksharing/single/omp_single.c
    libomp :: worksharing/single/omp_single_copyprivate.c
    libomp :: worksharing/single/omp_single_nowait.c
    libomp :: worksharing/single/omp_single_private.c

  Expected Passes    : 2
  Expected Failures  : 1
  Unsupported Tests  : 2
  Unexpected Failures: 166

With the diff,

[~/llvms/upstream-git/build-Release] ninja check-openmp
[0/1] Running OpenMP tests
Testing Time: 0.16s
  Expected Passes    : 2
  Unsupported Tests  : 169

Event Timeline

twoh created this revision.Jul 16 2018, 9:51 AM

@ABataev does this mean that Clang will now require libatomic even if there is no OpenMP atomic construct? I remember documentation saying that the user has to link atomic libraries when needed...

runtime/test/lit.cfg
56–57

I think what you want to test here is whether you are compiling with Clang? If so, please use config.test_c_compiler or config.test_compiler_features.

twoh added inline comments.Jul 16 2018, 11:12 AM
runtime/test/lit.cfg
56–57

Thanks for the comment! I couldn't quite get it why are you thinking that I want to check if the compiler is clang. Following https://reviews.llvm.org/rL336467, -latomic is added if -fopenmp is given, so that's what I'm checking here. Don't all test cases here assume that they are compiling with clang?

@ABataev does this mean that Clang will now require libatomic even if there is no OpenMP atomic construct? I remember documentation saying that the user has to link atomic libraries when needed...

(I've replied on cfe-commits to the commit that introduced linking of -latomic because I don't think it's a good idea.)

runtime/test/lit.cfg
56–57

No, the tests can be run with GCC, Intel Compiler and maybe others. The test in its current form is pointless because all OpenMP tests get -fopenmp.

twoh updated this revision to Diff 155730.Jul 16 2018, 11:42 AM

Addressing comments from @Hahnfeld. Thanks!

This shouldn't be needed anymore, I reverted the relevant commit in rC337722 after discussion on cfe-commits.

@twoh Which distribution are you using that doesn't have libatomic? If possible you might want to explain your situation in https://bugs.llvm.org/show_bug.cgi?id=38026, thanks.