This is an archive of the discontinued LLVM Phabricator instance.

[HIP] Fix unbundling
ClosedPublic

Authored by yaxunl on May 30 2018, 1:51 PM.

Details

Summary

HIP uses clang-offload-bundler to bundle intermediate files for host
and different gpu archs together. When a file is unbundled,
clang-offload-bundler should be called only once, and the objects
for host and different gpu archs should be passed to the next
jobs. This is because Driver maintains CachedResults which maps
triple-arch string to output files for each job.

This patch fixes a bug in Driver::BuildJobsForActionNoCache which
uses incorrect key for CachedResults for HIP which causes
clang-offload-bundler being called mutiple times and incorrect
output files being used.

It only affects HIP.

Diff Detail

Event Timeline

yaxunl created this revision.May 30 2018, 1:51 PM
yaxunl added a reviewer: tra.Jun 3 2018, 4:44 PM
tra accepted this revision.Jun 6 2018, 10:48 AM

Few minor nits/suggestions. LGTM otherwise.

lib/Driver/Driver.cpp
3895

Should it be something more descriptive? E.g. "all" or "combined".

test/Driver/hip-binding.hip
5–7

Nit: you could probably just use /dev/null instead of a real temp file.

11–12

There's currently a bundler invocation in-between these two linker commands.
I think you need a negative check for bundler here, too.

This revision is now accepted and ready to land.Jun 6 2018, 10:48 AM
yaxunl marked 3 inline comments as done.Jun 6 2018, 12:11 PM
yaxunl added inline comments.
lib/Driver/Driver.cpp
3895

will use "all"

test/Driver/hip-binding.hip
5–7

I tried /dev/null but I got

error: no input files

I think we need a real input file here, even though an empty one.

11–12

will do

This revision was automatically updated to reflect the committed changes.
yaxunl marked 3 inline comments as done.