This is an archive of the discontinued LLVM Phabricator instance.

[HIP] Support offloading by linker script

Authored by yaxunl on May 4 2018, 2:09 PM.



To support linking device code in different source files, it is necessary to
embed fat binary at host linking stage.

This patch emits an external symbol for fat binary in host codegen, then
embed the fat binary by lld through a linker script.

Diff Detail


Event Timeline

yaxunl created this revision.May 4 2018, 2:09 PM

The IRGen aspects of this seem reasonable, modulo a comment change. I don't feel qualified to judge the driver change.

317 ↗(On Diff #145291)

Is this filename string only used for CUDA? If so, please rename it to make that clear, or at least add a comment.

yaxunl marked an inline comment as done.May 7 2018, 8:57 AM
yaxunl added inline comments.
317 ↗(On Diff #145291)

Yes. Will do.

yaxunl updated this revision to Diff 145480.May 7 2018, 9:25 AM
yaxunl marked 2 inline comments as done.

Rename variables used for CUDA only.

yaxunl retitled this revision from [HIP] Support offloading by linkscript to [HIP] Support offloading by linker script.May 15 2018, 9:12 AM
yaxunl edited the summary of this revision. (Show Details)
yaxunl added reviewers: Hahnfeld, hfinkel.
t-tye accepted this revision.May 17 2018, 1:17 PM

LGTM except for minor suggestions.

361–373 ↗(On Diff #145480)

Should this all be inside an if (IsHip) so the names can be different for HIP and NV CUDA? Seems HIP should not be using names with VV_CUDA in them. In fact maybe the following IF should be included as well to consilidate the NV CUDA code and HIP code together.

394–398 ↗(On Diff #145480)

Should HIP use the same magic value and version number? Perhaps this should also be moved into the IsHip IF.

438 ↗(On Diff #145480)

Update comment for HIP

150 ↗(On Diff #145480)

OpenMP -> OpenMP or HIP

1321–1322 ↗(On Diff #145480)

for for -> for

This revision is now accepted and ready to land.May 17 2018, 1:17 PM

LGTM except for minor suggestions.

Thanks. Will make changes when committing.

This revision was automatically updated to reflect the committed changes.
tra added inline comments.May 18 2018, 4:26 PM

Using this linker script may present a problem.

INSERT BEFORE is not going to work with

LLD also does not handle it particularly well -- INSERT BEFORE can only be used to override explicitly specified external linker script and virtually nobody uses linker scripts with LLD.
See tests in

pcc added a subscriber: pcc.May 18 2018, 4:38 PM
pcc added inline comments.

If you're just trying to embed a file it may be better to use MC to write an ELF with something like:

.section .hip_fatbin
.align 16
.globl __hip_fatbin
.incbin "BundleFileName"

and add that to the link.

yaxunl added inline comments.May 21 2018, 6:57 AM

In we specified to use lld for linking. Since this is an explicit linker script, it works. We have tested this approach with many HIP applications that it works well.

Similar approach has also been used by OpenMP for embedding device binary.


Thanks for your suggestion. We will consider using MC if we see issues arise due to linker script.

JonChesterfield added inline comments.

Should this be toolchains::HipToolChain?

yaxunl marked 2 inline comments as done.Apr 23 2020, 8:27 AM
yaxunl added inline comments.

will fix. thanks