Build failure with OpenMP support enabled

[npmiller]

Describe the bug

The build fails whenever OpenMP support is enabled.

To Reproduce

From a clean build directory enable the OpenMP project then try to build:

python ../buildbot/configure.py -o . --llvm-external-projects=openmp
ninja

As part of the main build it will then try to build DeviceRTL for a number of targets and fail to link the IR with the following error:

cd /home/npmiller/llvm/build2/projects/openmp/libomptarget/DeviceRTL && /home/npmiller/llvm/build2/bin/llvm-link -o /home/npmiller/llvm/build2/projects/openmp/libomptarget/DeviceRTL/linked_libomptarget-nvptx-sm_60.bc Configuration.cpp-sm_60.bc Debug.cpp-sm_60.bc Kernel.cpp-sm_60.bc LibC.cpp-sm_60.bc Mapping.cpp-sm_60.bc Misc.cpp-sm_60.bc Parallelism.cpp-sm_60.bc Reduction.cpp-sm_60.bc State.cpp-sm_60.bc Synchronization.cpp-sm_60.bc Tasking.cpp-sm_60.bc Utils.cpp-sm_60.bc Workshare.cpp-sm_60.bc
invalid uses of intrinsic global variable
[2 x i8*]* @llvm.used
/home/npmiller/llvm/build2/bin/llvm-link: error: linked module is broken!

[jle-quel]

Hi Nicolas 👋

Introdution

I've been looking at this issue, and I've been able to reproduce the error.

The WW03 pulldown has introduced this error; here's the pulldown commit: #8053
and here's the relevant commit which introduced the error b67c16f

Explanation

When building DeviceRTL for the different targets, it will generate bitcode files, and then, these will be linked.

The two bitcode files we are interested in for this failure are the configuration.cpp-TargetsN.bc and utils.cpp-TargetsN.bc.

These two files contain intrinsic global variables, specifically the llvm.used global variable.

> Configuration.cpp-TargetsN.bc
@llvm.used = appending global [1 x i8*] [i8* addrspacecast (i8 addrspace(4)* bitcast (%struct.DeviceEnvironmentTy addrspace(4)* @__omp_rtl_device_environment to i8 addrspace(4)*) to i8*)], section "llvm.metadata"

Utils.cpp-TargetsN.bc
@llvm.used = appending global [1 x i8*] [i8* bitcast (void ()* @__keep_alive to i8*)], section "llvm.metadata"

When linking those two files, the verifier will fail and throw the error you posted.

After the WW03 pulldown, new conditions have been added to the verifier, which tests if any of the GlobalVariable has users that require materialization.
These new conditions are the ones that introduced the build failure (specifically the one regarding llvm.used)

This also impacts the llvm.global_ctors; See: llvm/llvm-project#60287

Solutions

For the llvm-16 release, the way they solved it was to revert the commit; See llvm/llvm-project#60298.

So, one way we could solve that problem (if a fix is needed ASAP) will be to do the same as upstream llvm.

After some debugging, I realized that when I was using DPC++'s llvm-link the verifier was failing, and when I was using upstream's llvm-link, the verifier did not fail.

The reason is that upstream llvm-link (and more generally upstream llvm have opaque pointers enabled by default) uses opaque pointers instead of type pointers, which DPC++'s llvm-link does not. As mentioned in https://llvm.org/docs/OpaquePointers.html, since llvm-15, opaque pointers are enabled by default, and the support for type pointers has been dropped out since llvm-17.

When generating the bitcode files from the Utils.cpp and Configuration.cpp file, using the upstream's clang, we end up with the global variable being defined with opaque pointers.

> Configuration.cpp-TargetsN.bc
@llvm.used = appending global [1 x ptr] [ptr addrspacecast (ptr addrspace(4) @__omp_rtl_device_environment to ptr)], section "llvm.metadata"

> Utils.cpp-TargetsN.bc
@llvm.used = appending global [1 x ptr] [ptr @__keep_alive], section "llvm.metadata"

Because, DPC++'s clang does invoke cc1 with -no-opaque-pointers, we end up with a valid IR, but when invoking llvm-link, the verifier will expect the IR to be using opaque pointers and will fail.

> Upstream cc1 invocation for a dummy main.cpp
 "/home/llvm-project/build/bin/clang-17" "-cc1" "-triple" "x86_64-unknown-linux-gnu" "-emit-llvm" "-disable-free" "-clear-ast-before-backend" "-main-file-name" "main.cpp" "-mrelocation-model" "pic" "-pic-level" "2" "-pic-is-pie" "-mframe-pointer=all" "-fmath-errno" "-ffp-contract=on" "-fno-rounding-math" "-mconstructor-aliases" "-funwind-tables=2" "-target-cpu" "x86-64" "-tune-cpu" "generic" "-debugger-tuning=gdb" "-fcoverage-compilation-dir=/tmp/baz" "-resource-dir" "/home/llvm-project/build/lib/clang/17" "-I/opt/intel/oneapi-tbb-2021.6.0/env/../include" "-internal-isystem" "/usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11" "-internal-isystem" "/usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/x86_64-linux-gnu/c++/11" "-internal-isystem" "/usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/backward" "-internal-isystem" "/home//llvm-project/build/lib/clang/17/include" "-internal-isystem" "/usr/local/include" "-internal-isystem" "/usr/lib/gcc/x86_64-linux-gnu/11/../../../../x86_64-linux-gnu/include" "-internal-externc-isystem" "/usr/include/x86_64-linux-gnu" "-internal-externc-isystem" "/include" "-internal-externc-isystem" "/usr/include" "-fdeprecated-macro" "-fdebug-compilation-dir=/tmp/baz" "-ferror-limit" "19" "-fgnuc-version=4.2.1" "-fcxx-exceptions" "-fexceptions" "-fcolor-diagnostics" "-faddrsig" "-D__GCC_HAVE_DWARF2_CFI_ASM=1" "-o" "main-llvm.ll" "-x" "c++" "main.cpp"

> DPC++ cc1 invocation for a dummy main.cpp
 "/home/oneAPI/llvm/build/bin/clang-17" "-cc1" "-triple" "x86_64-unknown-linux-gnu" "-emit-llvm" "-disable-free" "-clear-ast-before-backend" "-main-file-name" "main.cpp" "-mrelocation-model" "static" "-mframe-pointer=all" "-fmath-errno" "-ffp-contract=on" "-fno-rounding-math" "-mconstructor-aliases" "-funwind-tables=2" "-target-cpu" "x86-64" "-tune-cpu" "generic" "-mllvm" "-treat-scalable-fixed-error-as-warning" "-debugger-tuning=gdb" "-fcoverage-compilation-dir=/tmp/baz" "-resource-dir" "/home/oneAPI/git/llvm/build/lib/clang/17" "-I/opt/intel/oneapi-tbb-2021.6.0/env/../include" "-internal-isystem" "/usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11" "-internal-isystem" "/usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/x86_64-linux-gnu/c++/11" "-internal-isystem" "/usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/backward" "-internal-isystem" "/home/oneAPI/git/llvm/build/lib/clang/17/include" "-internal-isystem" "/usr/local/include" "-internal-isystem" "/usr/lib/gcc/x86_64-linux-gnu/11/../../../../x86_64-linux-gnu/include" "-internal-externc-isystem" "/usr/include/x86_64-linux-gnu" "-internal-externc-isystem" "/include" "-internal-externc-isystem" "/usr/include" "-fdeprecated-macro" "-fdebug-compilation-dir=/tmp/baz" "-ferror-limit" "19" "-fgnuc-version=4.2.1" "-no-opaque-pointers" "-fcxx-exceptions" "-fexceptions" "-fcolor-diagnostics" "-faddrsig" "-D__GCC_HAVE_DWARF2_CFI_ASM=1" "-o" "main-sycl.ll" "-x" "c++" "main.cpp"

One way I was able to successfully build the libomptarget archive library libomptarget.devicertl.a was to either enable opaque pointers when invoking the compiler using -Xclang -no-opaque-pointers or invoking the linker with the --opaque-pointers argument.

One possible patch could be to modify the DeviceRTL's CMake, and pass the -Xclang -no-opaque-pointers arguments when compiling the target.

The former will end up with the bitcode files generated using opaque pointers. The latter will link the two bitcode files together using opaque pointers, both satisfying the linker's verifier.

However, the way to fully support the opaque pointers would be to have them enabled by default, but I'm not an expert regarding the support of the opaque pointers and their integration with DPC++.

Conclusion

After more investigation, I will reply to this issue in the future and either come up with a patch or a follow-up stating the possible ongoing work on supporting opaque pointers.

[jle-quel]

This issue has been solved by the following PR #8286.

Now that the opaque pointers are enabled by default, the verifier is satisfied, and we no longer have the linked module is broken!.