[PATCH] D93525: [OpenMP] Add unbundling of archives containing bundled object files into device specific archives

Wed Jan 20 08:49:05 PST 2021

jdoerfert added a comment.

In D93525#2509836 <https://reviews.llvm.org/D93525#2509836>, @t-tye wrote:

> In D93525#2509796 <https://reviews.llvm.org/D93525#2509796>, @jdoerfert wrote:
>
>>> At the moment this patch defines compatibility as exact string match of bundler entry ID.
>>> [...]
>>> Supporting target ID requires little more work and discussion.
>>
>> Let's get this in first, then revisit target ID support as we need it.
>
> I do not think this patch should ignore target ID as that is now upstreamed and documented. What is involved in correcting the compatibility test to be correct by the target ID rules? There are examples of doing this in all the runtimes and I can help if that is useful.

First, there is no reason not to have multiple patches as long as they are self contained and testable. Arguably, smaller patches are better.

That said, target ID is a new feature and, as discussed in the OpenMP call today, there is a chance we have to revisit this to support more involved information. As this discussion is open ended (and hasn't started yet), it seems absolutely sensible to continue with a tested and working patch that provides features we need for sure instead of forcing some support of a feature we don't use right now anyway.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93525