[OpenMP][Fix] libomptarget Fortran tests #74543
Conversation
This patch fixes the erroneous multiple-target requirement in Fortran offloading tests. Additionally, it adds two new variables (`test_flags_clang`, `test_flags_flang`) to `lt.cfg` so that compiler-specific flags for Clang and Flang can be specified.
| @@ -2,8 +2,7 @@ | |||
| ! that checks constant indexing on device | |||
| ! correctly works (regression test for prior | |||
| ! bug). | |||
| ! REQUIRES: flang, amdgcn-amd-amdhsa | |||
| ! UNSUPPORTED: nvptx64-nvidia-cuda | |||
| ! REQUIRES: flang | |||
| ! UNSUPPORTED: nvptx64-nvidia-cuda-LTO | |||
There was a problem hiding this comment.
why is nvptx still unsupported?
There was a problem hiding this comment.
I removed the restriction that test is supported.
However, there are a couple of tests that are still unsupported like target_map_common_block2.f90. The reason is that nvlink fails to resolve some external symbols (bc of the inclusion of omp_lib), but those symbols shouldn't have been inserted into the module in the first place.
| @@ -136,7 +140,7 @@ else: # Unices | |||
| if config.cuda_libdir: | |||
| config.test_flags += " -Wl,-rpath," + config.cuda_libdir | |||
| if config.libomptarget_current_target.startswith('nvptx'): | |||
| config.test_flags += " --libomptarget-nvptx-bc-path=" + config.library_dir + '/DeviceRTL' | |||
There was a problem hiding this comment.
I think the leading whitespace needs to stay in case config.test_flags_clang is not empty.
|
This patch seems to have made the AMDGPU OpenMP Offload buildbot unhappy (https://lab.llvm.org/buildbot/#/builders/193/builds/44036). If you need help in recreating the issue, let me know. |
This reverts commit 49efb08.
|
I've just reverted, I'll investigate the issue offline and try to replicate. |
This patch fixes the erroneous multiple-target requirement in Fortran offloading tests. Additionally, it adds two new variables (test_flags_clang, test_flags_flang) to lit.cfg so that compiler-specific flags for Clang and Flang can be specified. This patch re-lands: #74543. The error was caused by having: ``` config.substitutions.append(("%flags", config.test_flags)) config.substitutions.append(("%flags_clang", config.test_flags_clang)) config.substitutions.append(("%flags_flang", config.test_flags_flang)) ``` when instead it has to be: ``` config.substitutions.append(("%flags_clang", config.test_flags_clang)) config.substitutions.append(("%flags_flang", config.test_flags_flang)) config.substitutions.append(("%flags", config.test_flags)) ``` because LIT replaces with the first longest sub-string match.
This patch fixes the erroneous multiple-target requirement in Fortran offloading tests. Additionally, it adds two new variables (
test_flags_clang,test_flags_flang) tolit.cfgso that compiler-specific flags for Clang and Flang can be specified.