[PATCH] D93525: [clang-offload-bundler] Add unbundling of archives containing bundled object files into device specific archives

Wed Aug 18 05:46:46 PDT 2021

saiislam added a comment.

In D93525#2952012 <https://reviews.llvm.org/D93525#2952012>, @JonChesterfield wrote:

> This is mentioned as broken in the referenced patch and landed with @t-tye still marked as requires changes. Revert warranted?
>
> Testing looks very sparse for something handling archives. Presumably it has the same set of bugs as D108291 <https://reviews.llvm.org/D108291> e.g. implicit whole archive semantics

Tony's earlier objection/ask was to include TargetID support along with archive unbundling support in this. I had verbal consent from him to split the patch and propose TargetID support for OpenMP in a separate patch. The same was agreed upon in the multi-company meeting as well.
Also, D106870 <https://reviews.llvm.org/D106870> places necessary infrastructure to support TargetID in a follow up patch. Once it lands, TargetID patch is fairly straightforward.
Whole archive semantics didn't introduce any bug anywhere, though performance is a separate issue. D108291 <https://reviews.llvm.org/D108291> is required because nvlink silently fails to link cubin files inside an archive.

Any suggestions on how can I improve testing for archives?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93525