[PATCH] D133705: [HIP] Fix unbundling archive

yaxunl closed this revision. yaxunl added a comment. committed by 1172bdecfab364579d90e6aa5ba7fc64a5b96786 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133705/new/ https://reviews.llvm.org/D133705

[PATCH] D133705: [HIP] Fix unbundling archive

tra accepted this revision. tra added a comment. LGTM. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133705/new/ https://reviews.llvm.org/D133705 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D133705: [HIP] Fix unbundling archive

yaxunl updated this revision to Diff 463016. yaxunl marked an inline comment as done. yaxunl added a comment. check file magic and only unbundle real archives CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133705/new/ https://reviews.llvm.org/D133705 Files:

[PATCH] D133705: [HIP] Fix unbundling archive

yaxunl marked an inline comment as done. yaxunl added inline comments. Comment at: clang/test/Driver/clang-offload-bundler.c:410-412 +// Check clang-offload-bundler extracts empty archives if the input file +// is not an archive when --allow-missing-bundles is specified,

[PATCH] D133705: [HIP] Fix unbundling archive

tra added inline comments. Comment at: clang/test/Driver/clang-offload-bundler.c:410-412 +// Check clang-offload-bundler extracts empty archives if the input file +// is not an archive when --allow-missing-bundles is specified, otherwise +// report an error. Is

[PATCH] D133705: [HIP] Fix unbundling archive

yaxunl updated this revision to Diff 462673. yaxunl added a comment. I just found clang-offload-bundler reports an error when trying to unbundle an archive but the input file is not an archive. This update let clang-offload-bundler to extract empty archives when the input file is not an

[PATCH] D133705: [HIP] Fix unbundling archive

tra accepted this revision. tra added inline comments. Comment at: clang/lib/Driver/Driver.cpp:2907-2908 +// which are not object files. Files with extension ".lib" is classified +// as TY_Object but they are actually archives, therefore should not be +

[PATCH] D133705: [HIP] Fix unbundling archive

yaxunl updated this revision to Diff 462376. yaxunl marked 4 inline comments as done. yaxunl added a comment. revised by Artem's comments CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133705/new/ https://reviews.llvm.org/D133705 Files: clang/lib/Driver/Driver.cpp

[PATCH] D133705: [HIP] Fix unbundling archive

yaxunl marked 4 inline comments as done. yaxunl added inline comments. Comment at: clang/lib/Driver/Driver.cpp:2907-2908 +// which are not object files. Files with extension ".lib" is classified +// as TY_Object but they are actually archives, therefore should

[PATCH] D133705: [HIP] Fix unbundling archive

tra added a comment. It's still not quite clear to me what the patch is intended to do. The description describes the existing issues (we don't unbundle some libraries), but is not clear what we do want to have in the end. Are all library arguments expected to be unbundled? If we do not want

[PATCH] D133705: [HIP] Fix unbundling archive

yaxunl updated this revision to Diff 462189. yaxunl added a comment. allow archive files to have unknown extension CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133705/new/ https://reviews.llvm.org/D133705 Files: clang/lib/Driver/Driver.cpp

[PATCH] D133705: [HIP] Fix unbundling archive

yaxunl added a comment. In D133705#3793931 , @MaskRay wrote: > I know very little about HIP, but I am concerned with relying on extensions > as well. For example, I've seen `libc++.a.1` (we use this for the real > archive while `libc++.a` is a linker

[PATCH] D133705: [HIP] Fix unbundling archive

yaxunl added a comment. In D133705#3793702 , @tra wrote: > In D133705#3785470 , @yaxunl wrote: > >>> Also, using `lib*.a` as pattern to tell device libraries from the host-ony >>> one will be insufficient. There

[PATCH] D133705: [HIP] Fix unbundling archive

MaskRay added a comment. I know very little about HIP, but I am concerned with relying on extensions as well. For example, I've seen `libc++.a.1` (we use this for the real archive while `libc++.a` is a linker script) and `.la` (libtool). A `.so` file is sometimes a linker script and it may

[PATCH] D133705: [HIP] Fix unbundling archive

tra added a comment. In 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

[PATCH] D133705: [HIP] Fix unbundling archive

yaxunl added a comment. In D133705#3784605 , @tra wrote: >> Archives passed by -l: should not be prefixed with >> prefix lib and appended with '.a', but still need to be prefixed with >> paths in -L options. >> Archives passed as input files should

[PATCH] D133705: [HIP] Fix unbundling archive

tra added a comment. > Archives passed by -l: should not be prefixed with > prefix lib and appended with '.a', but still need to be prefixed with > paths in -L options. > Archives passed as input files should not be prefixed > or appended with anything. I'm not sure I understand the

[PATCH] D133705: [HIP] Fix unbundling archive

scchan accepted this revision. scchan added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133705/new/ https://reviews.llvm.org/D133705 ___ cfe-commits mailing list

[PATCH] D133705: [HIP] Fix unbundling archive

yaxunl updated this revision to Diff 459502. yaxunl marked an inline comment as done. yaxunl added a comment. revised by Siu Chi's comments CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133705/new/ https://reviews.llvm.org/D133705 Files: clang/lib/Driver/Driver.cpp

[PATCH] D133705: [HIP] Fix unbundling archive

yaxunl marked an inline comment as done. yaxunl added inline comments. Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:1959 + if (FoundAOB) +break; } scchan wrote: > The AOBFileNames small vector needs to be cleared if !FoundAOB or just

[PATCH] D133705: [HIP] Fix unbundling archive

scchan requested changes to this revision. scchan added inline comments. This revision now requires changes to proceed. Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:1959 + if (FoundAOB) +break; } The AOBFileNames small vector needs to

[PATCH] D133705: [HIP] Fix unbundling archive

yaxunl updated this revision to Diff 459484. yaxunl added a comment. remove debug output CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133705/new/ https://reviews.llvm.org/D133705 Files: clang/lib/Driver/Driver.cpp clang/lib/Driver/ToolChains/CommonArgs.cpp

[PATCH] D133705: [HIP] Fix unbundling archive

yaxunl updated this revision to Diff 459482. yaxunl added a comment. Herald added subscribers: sstefan1, MaskRay. Herald added a reviewer: jdoerfert. sorry. update with the correct patch. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133705/new/ https://reviews.llvm.org/D133705 Files:

[PATCH] D133705: [HIP] Fix unbundling archive

yaxunl created this revision. yaxunl added a reviewer: tra. Herald added a project: All. yaxunl requested review of this revision. HIP is able to unbundle archive of bundled bitcode. However currently there are two bugs: 1. archives passed by -l: are not unbundled. 2. archives passed as input