This is an archive of the discontinued LLVM Phabricator instance.

[OpenMP] Drop the static library libomptarget-nvptx
ClosedPublic

Authored by tianshilei1992 on Jan 12 2021, 6:27 PM.

Details

Summary

For NVPTX target, OpenMP provides a static library libomptarget-nvptx
built by NVCC, and another bitcode libomptarget-nvptx-sm_{$sm}.bc generated by
Clang. When compiling an OpenMP program, the .bc file will be fed to clang
in the second run on the program that compiles the target part. Then the generated
PTX file will be fed to ptxas to generate the object file, and finally the driver
invokes nvlink to generate the binary, where the static library will be appened
to nvlink.

One question is, why do we need two libraries? The only difference is, the static
library contains omp_data.cu and the bitcode library doesn't. It's unclear why
they were implemented in this way, but per D94565, there is no issue if we also
include the file into the bitcode library. Therefore, we can safely drop the
static library.

This patch is about the change in OpenMP. The driver will be updated as well if
this patch is accepted.

Diff Detail

Event Timeline

tianshilei1992 created this revision.Jan 12 2021, 6:27 PM
tianshilei1992 requested review of this revision.Jan 12 2021, 6:27 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 12 2021, 6:27 PM

One question is, why do we need two libraries? The only difference is, the static library contains omp_data.cu and the bitcode library doesn't. It's unclear why they were implemented in this way

It's been a very long time since then (6+ years), but if memory serves me well the reason we needed to build the static library in the early days was that a lot of systems lacked a CUDA compiler capable of emitting LLVM bitcode, so the static library was the only choice. Things have changed a lot in the meantime, so I guess the static library fallback is irrelevant nowadays.

JonChesterfield accepted this revision.Jan 12 2021, 7:59 PM

@grokos that sounds right. As far as I know, clang is capable of compiling this correctly as cuda (otherwise we'd have bug reports from people using the bitcode build).

Dropping this is a step towards compiling the deviceRTL as openmp (or c++).

openmp/libomptarget/deviceRTLs/nvptx/CMakeLists.txt
61–62

We should probably rework this logic / failure reporting. Finding a working compiler should be simpler if we build libomptarget via ENABLE_RUNTIMES.

This revision is now accepted and ready to land.Jan 12 2021, 7:59 PM

Also, since D94565 is merged now, this patch needs rebasing :)

Discussed on weekly call, we have consensus to ship this.

Rebased the patch and removed unnecessary part

Optimized some logics

tianshilei1992 marked an inline comment as done.Jan 14 2021, 9:53 AM