[OpenMP][OMPT] Fix locking testcases for 32 bit architectures

[protze.joachim]

Summary

Fix for the bug reported in:
https://bugs.llvm.org/show_bug.cgi?id=40531

The address is now casted the same way as in the runtime code.

[hbae]

LGTM

[hans]

runtime/test/ompt/synchronization/lock.c

13 ↗

(On Diff #187591)

I don't know this code at all obviously, but will this work with the PRIu64 conversion specifier? If ompt_wait_id_t is 32-bit on 32-bit platforms, this sounds like it wouldn't match?

[protze.joachim]

runtime/test/ompt/synchronization/lock.c

13 ↗

(On Diff #187591)

ompt_wait_id_t is defined to be 64 bit by the OpenMP spec.

The problem is the expansion to 64 bit in the cast.
gcc and clang seem to have a different idea of what should be the result on 32 bit arch for

`int a;
(uint64_t)&a;`

I still can reproduce the issue when using gcc as test compiler after compiling the runtime with clang or vice versa.

[krytarowski]

This broke the NetBSD buildbot: http://lab.llvm.org:8011/builders/lldb-amd64-ninja-netbsd8/builds/19082

[mgorny]

openmp/trunk/runtime/test/CMakeLists.txt

34

Is this really supposed to be part of this fix? NetBSD buildbot got broken between r354552..r354559 (http://lab.llvm.org:8011/builders/lldb-amd64-ninja-netbsd8/builds/19082) and this looks like the only change in openmp in that batch.

[Hahnfeld]

In D58454#1405621, @krytarowski wrote:

This broke the NetBSD buildbot: http://lab.llvm.org:8011/builders/lldb-amd64-ninja-netbsd8/builds/19082

This should be fixed in rOMP354572. As a bot owner, could you please make sure that the bot lists the commits to openmp? Otherwise it's hard to detect broken bots without emails.

[mgorny]

In D58454#1405643, @Hahnfeld wrote:

In D58454#1405621, @krytarowski wrote:

This broke the NetBSD buildbot: http://lab.llvm.org:8011/builders/lldb-amd64-ninja-netbsd8/builds/19082

This should be fixed in rOMP354572. As a bot owner, could you please make sure that the bot lists the commits to openmp? Otherwise it's hard to detect broken bots without emails.

If I only knew how, I'd have fixed that long ago.

[Hahnfeld]

In D58454#1405654, @mgorny wrote:

In D58454#1405643, @Hahnfeld wrote:

In D58454#1405621, @krytarowski wrote:

This broke the NetBSD buildbot: http://lab.llvm.org:8011/builders/lldb-amd64-ninja-netbsd8/builds/19082

This should be fixed in rOMP354572. As a bot owner, could you please make sure that the bot lists the commits to openmp? Otherwise it's hard to detect broken bots without emails.

If I only knew how, I'd have fixed that long ago.

If I had to bet, that's because lldb-amd64-ninja-netbsd8 declares itself to be an lldb builder: https://github.com/llvm/llvm-zorg/blob/master/buildbot/osuosl/master/config/builders.py#L976
As a result it will only be triggered by commits to llvm, cfe and lldb: https://github.com/llvm/llvm-zorg/blob/master/buildbot/osuosl/master/master.cfg#L194
And if buildbot doesn't know that the bot magically updates and tests openmp, it probably doesn't add the relevant commits and so on.

[protze.joachim]

runtime/test/ompt/synchronization/lock.c

13 ↗

(On Diff #187591)

https://reviews.llvm.org/D58506 casts the addresses to uintptr_t before expanding to 64 bit.

openmp/trunk/runtime/test/CMakeLists.txt

 
 pythonize_bool(LIBOMP_USE_HWLOC)
 pythonize_bool(LIBOMP_OMPT_SUPPORT)
 pythonize_bool(LIBOMP_OMPT_OPTIONAL)
 pythonize_bool(LIBOMP_HAVE_LIBM)
 pythonize_bool(LIBOMP_HAVE_LIBATOMIC)
 
 add_openmp_testsuite(check-libomp "Running libomp tests" ${CMAKE_CURRENT_BINARY_DIR} DEPENDS omp)
+add_openmp_testsuite(check-ompt "Running OMPT tests" ${CMAKE_CURRENT_BINARY_DIR}/ompt DEPENDS omp)
 
 # Configure the lit.site.cfg.in file
 set(AUTO_GEN_COMMENT "## Autogenerated by libomp configuration.\n# Do not edit!")
 configure_file(lit.site.cfg.in lit.site.cfg @ONLY)

openmp/trunk/runtime/test/ompt/synchronization/lock.c

 // RUN: %libomp-compile-and-run | FileCheck %s
 // REQUIRES: ompt
 #include "callback.h"
 #include <omp.h>
 
 int main()
 {
   //need to use an OpenMP construct so that OMPT will be initalized
   #pragma omp parallel num_threads(1)
     print_ids(0);
 
   omp_lock_t lock;
-  printf("%" PRIu64 ": &lock: %" PRIu64 "\n", ompt_get_thread_data()->value, (uint64_t) &lock);
+  printf("%" PRIu64 ": &lock: %" PRIu64 "\n", ompt_get_thread_data()->value, (ompt_wait_id_t) &lock);
   omp_init_lock(&lock);
   print_fuzzy_address(1);
   omp_set_lock(&lock);
   print_fuzzy_address(2);
   omp_unset_lock(&lock);
   print_fuzzy_address(3);
   omp_destroy_lock(&lock);
   print_fuzzy_address(4);

openmp/trunk/runtime/test/ompt/synchronization/nest_lock.c

 // RUN: %libomp-compile-and-run | FileCheck %s
 // REQUIRES: ompt
 #include "callback.h"
 #include <omp.h>
 
 int main()
 {
   //need to use an OpenMP construct so that OMPT will be initalized
   #pragma omp parallel num_threads(1)
     print_ids(0);
 
   omp_nest_lock_t nest_lock;
-  printf("%" PRIu64 ": &nest_lock: %lli\n", ompt_get_thread_data()->value, (long long) &nest_lock);
+  printf("%" PRIu64 ": &nest_lock: %lli\n", ompt_get_thread_data()->value, (ompt_wait_id_t) &nest_lock);
   omp_init_nest_lock(&nest_lock);
   print_fuzzy_address(1);
   omp_set_nest_lock(&nest_lock);
   print_fuzzy_address(2);
   omp_set_nest_lock(&nest_lock);
   print_fuzzy_address(3);
   omp_unset_nest_lock(&nest_lock);
   print_fuzzy_address(4);