[PATCH] D133705: [HIP] Fix unbundling archive

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D133705#3793702 <https://reviews.llvm.org/D133705#3793702>, @tra wrote:

> In D133705#3785470 <https://reviews.llvm.org/D133705#3785470>, @yaxunl wrote:
>
>>> Also, using `lib*.a` as pattern to tell device libraries from the host-ony one will be insufficient. There will be libraries with other extensions (e.g. `.lo` is fairly common) and there will be libraries that do not have `lib` prefix. I.e. nothing stops me from building `mylib.a` and linking with it as in the example above.
>>
>> For archives passed as input files, we use the extension ('.a' on Linux and '.lib' on MSVC) to identify them. We do not restrict the file names to 'lib*.a', but they need to be '*.a' or '*.lib' to be identified as archives.
>
> As I've pointed above that will create issues.
>
>> For arbitrary file names to be identified as archives, users can pass them by '-l:', e.g. '-l:file.bin', then clang will treat 'file.bin' as an archive.
>
> It's not always possible. Linking with dependencies is often handled by the build tools that the end user may not control. E.g. TF is built w/ `bazel` which handles virtually all linking under the hood with almost no user controls over the names/extensions it assigns to files or how they are passed to the linker.
>
> @MaskRay - WDYT?

A side note: this patch does not change clang's original behavior about how to link archives in host compilation. It only changes how clang determines what files to be unbundled as archives. The current patch only treats '*.a' or '*.lib' input files as archives and tries to unbundle them.

I think the root issue is that clang lacks a way to mark an arbitrary file name as an archive file. One solution might be to introduce a new file type 'Archive' and a '-x' value 'archive' to represent that. However, there are some complications with how currently clang treated '.lib' files. This makes me hesitate to introduce such changes. Since such unavoidably will affect how clang links archives for host compilation. My current patch does not change that. It has a limitation, but that limitation only affects HIP and I think it is acceptable.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133705/new/

https://reviews.llvm.org/D133705