This is an archive of the discontinued LLVM Phabricator instance.

[OpenMP] Make bc file compilation sensitive to LIBOMPTARGET_NVPTX_DEBUG flag
ClosedPublic

Authored by guansong on Apr 11 2018, 10:26 AM.

Details

Summary

The LIBOMPTARGET_NVPTX_DEBUG flag is inconsistent between using nvcc to generate .a file and clang to generate .bc file. Sync the two setting so we can get debug messages from the bc file path as well.

Diff Detail

Repository
rL LLVM

Event Timeline

guansong created this revision.Apr 11 2018, 10:26 AM
guansong retitled this revision from Make bc file compilation sensitive to LIBOMPTARGET_NVPTX_DEBUG flag to [OpenMP] Make bc file compilation sensitive to LIBOMPTARGET_NVPTX_DEBUG flag.Apr 11 2018, 10:27 AM
Hahnfeld added inline comments.
libomptarget/deviceRTLs/nvptx/CMakeLists.txt
146 ↗(On Diff #142040)

Hmm, why does Sync mean a new variable? If that's what you want, it should start with LIBOMPTARGET_NVPTX and be properly documented

guansong added inline comments.Apr 11 2018, 11:00 AM
libomptarget/deviceRTLs/nvptx/CMakeLists.txt
146 ↗(On Diff #142040)

Thanks. I put a typo there, it should be

set(BC_DEBUG FALSE CACHE BOOL

As I used in the next few lines. BC_DEBUG is an internal var. The behavior to sync is the Cmake command line def LIBOMPTARGET_NVPTX_DEBUG

guansong added inline comments.Apr 11 2018, 11:05 AM
libomptarget/deviceRTLs/nvptx/CMakeLists.txt
146 ↗(On Diff #142040)

Look at this again, I am thinking I don't need these few lines. The BC_DEBUG will be defined either way in the following if block.

Hahnfeld added inline comments.Apr 11 2018, 12:16 PM
libomptarget/deviceRTLs/nvptx/CMakeLists.txt
146 ↗(On Diff #142040)

In that case it must not be CACHE. You can define BC_DEBUG to -DOMPTARGET_NVPTX_DEBUG=0 and overwrite it if(${LIBOMPTARGET_NVPTX_DEBUG})

guansong updated this revision to Diff 142069.Apr 11 2018, 1:55 PM

update according to suggestion, thanks.

grokos accepted this revision.Apr 12 2018, 1:14 PM

I think this is now good to go.

This revision is now accepted and ready to land.Apr 12 2018, 1:14 PM
This revision was automatically updated to reflect the committed changes.