Page MenuHomePhabricator

[libomptarget] Add device RTL to regression test dependencies.
ClosedPublic

Authored by Meinersbur on Apr 5 2022, 8:47 PM.

Details

Summary

In a clean build directory, check-openmp or check-libomptarget will fail because of missing device RTL .bc files. Ensure that the new targets new custom targets omptarget.devicertl.nvptx and omptarget.devicertl.amdgpu (corresponding to the plugin rtl targets omptarget.rtl.cuda, respectively omptarget.rlt.amdgpu ) are dependencies of the regression tests.

Diff Detail

Event Timeline

Meinersbur created this revision.Apr 5 2022, 8:47 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 5 2022, 8:47 PM
Meinersbur requested review of this revision.Apr 5 2022, 8:47 PM

Extra dependency edge is great, thanks! Is LIBOMPTARGET_TESTED_PLUGINS new? It looks unused in this patch

LIBOMPTARGET_TESTED_PLUGINS already exists and is already added to check-openmp and check-libomptarget dependencies in openmp\libomptarget\test\CMakeLists.txt:

  add_openmp_testsuite(check-libomptarget-${CURRENT_TARGET}
    "Running libomptarget tests"
    ${CMAKE_CURRENT_BINARY_DIR}/${CURRENT_TARGET}
    DEPENDS omptarget omp ${LIBOMPTARGET_TESTED_PLUGINS}
    ARGS ${LIBOMPTARGET_LIT_ARG_LIST})
[... ]
add_openmp_testsuite(check-libomptarget
  "Running libomptarget tests"
  ${LIBOMPTARGET_LIT_TESTSUITES}
  EXCLUDE_FROM_CHECK_ALL
  DEPENDS omptarget omp ${LIBOMPTARGET_TESTED_PLUGINS}
  ARGS ${LIBOMPTARGET_LIT_ARG_LIST})

Elements are only added by plugins that use the build_generic_elf64 macro, so I guess the dependencies have been accidentally dropped at some point.

JonChesterfield accepted this revision.Apr 6 2022, 7:54 AM

Nice, thanks!

This revision is now accepted and ready to land.Apr 6 2022, 7:54 AM
kkwli0 added a subscriber: kkwli0.Apr 6 2022, 9:20 AM

Not sure if it is related. If I do a clean make check-openmp, I see many failures due to missing FileCheck etc.

...
$ "build-llvm/./bin/FileCheck" "/llvm-project/openmp/runtime/test/ompt/parallel/nested_threadnum.c"
# command stderr:
'build-llvm/./bin/FileCheck': command not found
error: command failed with exit status: 127

Not sure if it is related. If I do a clean make check-openmp, I see many failures due to missing FileCheck etc.

...
$ "build-llvm/./bin/FileCheck" "/llvm-project/openmp/runtime/test/ompt/parallel/nested_threadnum.c"
# command stderr:
'build-llvm/./bin/FileCheck': command not found
error: command failed with exit status: 127

I cannot reproduce, FileCheck is a correct dependency:

add_lit_testsuite(${target}
  ${comment}
  ${ARG_UNPARSED_ARGUMENTS}
  EXCLUDE_FROM_CHECK_ALL
  DEPENDS clang FileCheck not ${ARG_DEPENDS}
  ARGS ${ARG_ARGS}
)

However, for me in the tests FileCheck is resolved to an absolute path. Do you use a standalone build with incorrect (relative) paths to LLVM's build dir?

kkwli0 added a comment.Apr 6 2022, 6:28 PM

Not sure if it is related. If I do a clean make check-openmp, I see many failures due to missing FileCheck etc.

...
$ "build-llvm/./bin/FileCheck" "/llvm-project/openmp/runtime/test/ompt/parallel/nested_threadnum.c"
# command stderr:
'build-llvm/./bin/FileCheck': command not found
error: command failed with exit status: 127

I cannot reproduce, FileCheck is a correct dependency:

add_lit_testsuite(${target}
  ${comment}
  ${ARG_UNPARSED_ARGUMENTS}
  EXCLUDE_FROM_CHECK_ALL
  DEPENDS clang FileCheck not ${ARG_DEPENDS}
  ARGS ${ARG_ARGS}
)

However, for me in the tests FileCheck is resolved to an absolute path. Do you use a standalone build with incorrect (relative) paths to LLVM's build dir?

I don't think it is the path issue. The problem is that FileCheck is not built in make check-openmp for some reason.

[kli@host build-llvm]$ pwd 
/home/kli/wrk/f/build-llvm
[kli@host build-llvm]$ ll bin/FileCheck
/bin/ls: cannot access 'bin/FileCheck': No such file or directory
[kli@host build-llvm]$ find . -type f -name FileCheck
[kli@host build-llvm]$

The patch prevents me from building libomptarget due to linking errors. When building the tools subdirectory there is a clash where the version scripts for both x86 and nvptx are included causing the following error,

FAILED: /home2/3n4/llvm/llvm-project/build/bin/llvm-omp-device-info 
: && /home2/3n4/llvm/llvm-project/build/./bin/clang++ --target=x86_64-unknown-linux-gnu -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wno-comment -Wstring-conversion -Wmisleading-indentation -fdiagnostics-color -ffunction-sections -fdata-sections -Wall -Wcast-qual -Wformat-pedantic -Wimplicit-fallthrough -Wsign-compare -Wno-extra -Wno-pedantic -std=c++14 -O3 -DNDEBUG -fuse-ld=gold    -Wl,--gc-sections openmp/libomptarget/tools/deviceinfo/CMakeFiles/llvm-omp-device-info.dir/llvm-omp-device-info.cpp.o -o /home2/3n4/llvm/llvm-project/build/bin/llvm-omp-device-info  -Wl,-rpath,"\$ORIGIN/../lib:/home2/3n4/llvm/llvm-project/build/./lib"  -lpthread  openmp/runtime/src/libomp.so  openmp/libomptarget/libomptarget.so  openmp/libomptarget/libomptarget.rtl.cuda.so  openmp/libomptarget/libomptarget.rtl.x86_64.so  -lpthread -lrt -lm  /usr/lib64/libcuda.so  -Wl,-z,defs  /home2/3n4/llvm/llvm-project/build/lib/libLLVMObject.so.15git  /home2/3n4/llvm/llvm-project/build/lib/libLLVMBinaryFormat.so.15git  /home2/3n4/llvm/llvm-project/build/lib/libLLVMSupport.so.15git  /usr/lib64/libffi.so  -ldl  /usr/lib64/libelf.so  -lpthread  -Wl,-rpath-link,/home2/3n4/llvm/llvm-project/build/lib && :
-Wl,--version-script=/home2/3n4/llvm/llvm-project/openmp/libomptarget/plugins/x86_64/../exports  
-Wl,--version-script=/home2/3n4/llvm/llvm-project/openmp/libomptarget/plugins/cuda/../exports  
/usr/bin/ld.gold: error: linker defined: multiple definition of 'VERS1.0'
/usr/bin/ld.gold: command line: previous definition here

It seems to be because the tool links against LIBOMPTARGET_TESTED_PLUGINS and this patch adds to that list and creates the linker conflict.

brooks added a subscriber: brooks.Apr 11 2022, 1:49 PM

This fails to configure in cmake when enabled as a project.

$ cmake ../llvm -DLLVM_ENABLE_PROJECTS='llvm;clang;lld;openmp'
...
-- Configuring done
CMake Error at /home/bed22/git/llvm-project/openmp/libomptarget/plugins/amdgpu/CMakeLists.txt:74 (add_dependencies):
  The dependency target "omptarget.devicertl.amdgpu" of target
  "omptarget.rtl.amdgpu" does not exist.


-- Generating done
CMake Generate step failed.  Build files cannot be regenerated correctly.

Reverting this commit allows cmake to succeed. The linkage fix in rG2e0cb61570ec does not fix this issue.