r239337 - Remove unused variables '__kmp_build_check_*'

Jonathan,
     The commit...

http://lists.cs.uiuc.edu/pipermail/openmp-commits/2015-June/000339.html

only converts the unused-variable warnings for __kmp_build_check_*
into unused typedef warnings. My initial inclination was to test
adding...

#ifdef KMP_USE_ASSERT
...
#endif

preprocessor wrappers around all of the calls to the KMP_BUILD_ASSERT
macro. However this doesn't work since the openmp build passes -D
KMP_USE_ASSERT to the compiler flags in the cmake build of openmp in
the llvm tree. This occurs for a cmake build which pass passed
-DLLVM_ENABLE_ASSERTIONS=OFF for the top-level cmake options.
Shouldn't KMP_USE_ASSERT be disabled in that case or are those two
usages of asserts unrelated?
              Jack

Jonathan,
      Are you sure that the __kmp_build_check_* symbols generated from
the KMP_BUILD_ASSERT macros aren't dead code in openmp? I find that
the following patch eliminates the warnings....

warning: unused typedef '__kmp_build_check_492' [-Wunused-local-typedef]

when the toplevel llvm build is passed-DLLVM_ENABLE_ASSERTIONS=OFF but
they still remain for -DLLVM_ENABLE_ASSERTIONS=ON.
                 Jack

KMP_USE_ASSERT.patch (5.18 KB)

Jonathan,
     I should have also noted that with KMP_USE_ASSERT.patch applied,
the build with -DLLVM_ENABLE_ASSERTIONS=OFF produces no regressions in
the OpenMP3.1_Validation test suite at -m32/-m64 on
x86_64-apple-darwin14.
           Jack

They produce dummy dead code to detect compile time errors. A contrived example is something like:

KMP_BUILD_ASSERT(sizeof(kmp_int32) == sizeof(kmp_int64)); // This should produce a build failure.

It used to be that they produce one-element, unused character arrays (when the assert is successful), or try to produce a "-1" element character array if the assert condition is false (which prompted a build failure because you can't create arrays with -1 elements). I thought my change would get rid of these unused-variable warnings because now it creates an unused type. I thought for sure that there were unused types in kmp.h, and I had not seen any warnings for unused types come up yet. So, I made the assumption that unused types were ok, but they are unfortunately just a different warning as you have pointed out.

I do think we should use LLVM_ENABLE_ASSERTIONS like you have it, but instead of guarding each KMP_BUILD_ASSERT() statement inside the source files, we should change the definition of KMP_BUILD_ASSERT() based on KMP_USE_ASSERT (see how KMP_ASSERT() and KMP_DEBUG_ASSERT() are defined in kmp_debug.h).

-- Johnny

Jonathan,
      I'm not quite sure how I should implement this change to the
definition of KMP_BUILD_ASSERT(). My initial attempt. attached, fails
to compile with the error...

cd /sw/src/fink.build/llvm37-3.7.0-100/llvm-3.7.0.src/build/projects/openmp/runtime
&& /sw/bin/clang-3.6 -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS
-D__STDC_LIMIT_MACROS -Domp_EXPORTS -fPIC -Wall -W
-Wno-unused-parameter -Wwrite-strings -Wmissing-field-initializers
-pedantic -Wno-long-long -Wcovered-switch-default -O3 -DNDEBUG -arch
x86_64 -arch i386 -fPIC
-I/sw/src/fink.build/llvm37-3.7.0-100/llvm-3.7.0.src/build/projects/openmp/runtime
-I/sw/src/fink.build/llvm37-3.7.0-100/llvm-3.7.0.src/projects/openmp/runtime
-I/sw/src/fink.build/llvm37-3.7.0-100/llvm-3.7.0.src/build/include
-I/sw/src/fink.build/llvm37-3.7.0-100/llvm-3.7.0.src/include
-I/sw/src/fink.build/llvm37-3.7.0-100/llvm-3.7.0.src/projects/openmp/runtime/src
-I/sw/src/fink.build/llvm37-3.7.0-100/llvm-3.7.0.src/projects/openmp/runtime/src/i18n
-I/sw/src/fink.build/llvm37-3.7.0-100/llvm-3.7.0.src/projects/openmp/runtime/src/include/41
-I/sw/src/fink.build/llvm37-3.7.0-100/llvm-3.7.0.src/projects/openmp/runtime/src/thirdparty/ittnotify
   -D USE_ITT_BUILD -D KMP_ARCH_STR="\"Intel(R) 64\"" -D BUILD_I8 -D
KMP_LIBRARY_FILE=\"libomp.dylib\" -D KMP_VERSION_MAJOR=5 -D
KMP_NESTED_HOT_TEAMS -D CACHE_LINE=64 -D KMP_ADJUST_BLOCKTIME=1 -D
BUILD_PARALLEL_ORDERED -D KMP_ASM_INTRINS -D USE_ITT_NOTIFY=1 -D
INTEL_ITTNOTIFY_PREFIX=__kmp_itt_ -D _GNU_SOURCE -D _REENTRANT -D
BUILD_TV -D USE_CBLKDATA -D KMP_GOMP_COMPAT -D USE_LOAD_BALANCE -D
KMP_DYNAMIC_LIB -D KMP_STATS_ENABLED=0 -D OMPT_SUPPORT=0 -D
OMPT_BLAME=1 -D OMPT_TRACE=1 -D OMP_50_ENABLED=0 -D OMP_41_ENABLED=1
-D OMP_40_ENABLED=1 -D OMP_30_ENABLED=1 -D KMP_USE_ADAPTIVE_LOCKS=1 -D
KMP_DEBUG_ADAPTIVE_LOCKS=0 -D KMP_USE_INTERNODE_ALIGNMENT=0
-I/sw/include -std=c++0x -fno-exceptions -x c++ -Wno-unused-value
-Wno-switch -Wno-deprecated-register -o
CMakeFiles/omp.dir/src/kmp_ftn_cdecl.c.o -c
/sw/src/fink.build/llvm37-3.7.0-100/llvm-3.7.0.src/projects/openmp/runtime/src/kmp_ftn_cdecl.c
In file included from
/sw/src/fink.build/llvm37-3.7.0-100/llvm-3.7.0.src/projects/openmp/runtime/src/kmp_ftn_cdecl.c:16:
In file included from
/sw/src/fink.build/llvm37-3.7.0-100/llvm-3.7.0.src/projects/openmp/runtime/src/kmp.h:88:
/sw/src/fink.build/llvm37-3.7.0-100/llvm-3.7.0.src/projects/openmp/runtime/src/kmp_lock.h:356:1:
error: expected unqualified-id
KMP_BUILD_ASSERT( offsetof( kmp_base_queuing_lock_t, tail_id ) % 8 == 0 );
^
/sw/src/fink.build/llvm37-3.7.0-100/llvm-3.7.0.src/projects/openmp/runtime/src/kmp_debug.h:48:45:
note: expanded from macro 'KMP_BUILD_ASSERT'
#define KMP_BUILD_ASSERT( expr ) 0
                                            ^
In file included from
/sw/src/fink.build/llvm37-3.7.0-100/llvm-3.7.0.src/projects/openmp/runtime/src/kmp_ftn_cdecl.c:16:
/sw/src/fink.build/llvm37-3.7.0-100/llvm-3.7.0.src/projects/openmp/runtime/src/kmp.h:1415:5:
warning: anonymous structs are a GNU extension
[-Wgnu-anonymous-struct]
    struct KMP_ALIGN( 32 ) { // AC: changed 16 to 32 in order to
simplify template
    ^
/sw/src/fink.build/llvm37-3.7.0-100/llvm-3.7.0.src/projects/openmp/runtime/src/kmp.h:1449:5:
warning: anonymous structs are a GNU extension
[-Wgnu-anonymous-struct]
    struct KMP_ALIGN( 32 ) {
    ^
/sw/src/fink.build/llvm37-3.7.0-100/llvm-3.7.0.src/projects/openmp/runtime/src/kmp.h:1680:5:
warning: anonymous structs are a GNU extension
[-Wgnu-anonymous-struct]
    struct {
    ^
/sw/src/fink.build/llvm37-3.7.0-100/llvm-3.7.0.src/projects/openmp/runtime/src/kmp.h:2043:1:
error: expected unqualified-id
KMP_BUILD_ASSERT( sizeof(kmp_taskdata_t) % sizeof(void *) == 0 );
^
/sw/src/fink.build/llvm37-3.7.0-100/llvm-3.7.0.src/projects/openmp/runtime/src/kmp_debug.h:48:45:
note: expanded from macro 'KMP_BUILD_ASSERT'
#define KMP_BUILD_ASSERT( expr ) 0
                                            ^
3 warnings and 2 errors generated.
projects/openmp/runtime/CMakeFiles/omp.dir/build.make:54: recipe for
target 'projects/openmp/runtime/CMakeFiles/omp.dir/src/kmp_ftn_cdecl.c.o'
failed

Or am I misunderstanding your proposed fix to this problem?
                  Jack

KMP_USE_ASSERT_v2.patch (1.27 KB)

Jonathan,
       The attached patch implements the requested change. We needed
to mimic the behavior of the KA_TRACE() macros rather than the
KMP_ASSERT() and KMP_DEBUG_ASSERT() macros. With this change, the
build of openmp in the llvm tree completes on x86_64-apple-darwin14
using clang 3.6.1 without unused-typedef warnings for
__kmp_build_check_* when cmake is passed -DLLVM_ENABLE_ASSERTIONS=OFF.
                     Jack

KMP_USE_ASSERT_v3.patch (1.38 KB)

Committed revision 239546.

I changed it a little bit so that there is a new CACHED CMake variable LIBOMP_ENABLE_ASSERTIONS which can be set in standalone builds or take on the value of LLVM_ENABLE_ASSERTIONS in LLVM builds.

-- Johnny