[PATCH] D46472: [HIP] Support offloading by linker script

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added inline comments.


================
Comment at: cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp:1371-1388
+  // machines.
+  LksStream << "/*\n";
+  LksStream << "       HIP Offload Linker Script\n";
+  LksStream << " *** Automatically generated by Clang ***\n";
+  LksStream << "*/\n";
+  LksStream << "TARGET(binary)\n";
+  LksStream << "INPUT(" << BundleFileName << ")\n";
----------------
pcc wrote:
> tra wrote:
> > Using this linker script may present a problem.
> > 
> > INSERT BEFORE is not going to work with ld.gold.
> > https://sourceware.org/bugzilla/show_bug.cgi?id=15373
> > 
> > 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 https://reviews.llvm.org/D44380
> > 
> 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
> __hip_fatbin:
> .incbin "BundleFileName"
> ```
> and add that to the link.
In https://reviews.llvm.org/D45212 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.


================
Comment at: cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp:1371-1388
+  // machines.
+  LksStream << "/*\n";
+  LksStream << "       HIP Offload Linker Script\n";
+  LksStream << " *** Automatically generated by Clang ***\n";
+  LksStream << "*/\n";
+  LksStream << "TARGET(binary)\n";
+  LksStream << "INPUT(" << BundleFileName << ")\n";
----------------
yaxunl wrote:
> pcc wrote:
> > tra wrote:
> > > Using this linker script may present a problem.
> > > 
> > > INSERT BEFORE is not going to work with ld.gold.
> > > https://sourceware.org/bugzilla/show_bug.cgi?id=15373
> > > 
> > > 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 https://reviews.llvm.org/D44380
> > > 
> > 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
> > __hip_fatbin:
> > .incbin "BundleFileName"
> > ```
> > and add that to the link.
> In https://reviews.llvm.org/D45212 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.


Repository:
  rL LLVM

https://reviews.llvm.org/D46472