saiislam added a comment.
The patch looks fine to me.
Please wait for @tra 's final review.
On a different note, can this compatibility testing logic be moved to a llvm
library instead of clang's?
I want to use it in OpenMP's AMDGPU plugin, which now links llvm libraries by
default.
================
Comment at: clang/lib/Driver/OffloadBundler.cpp:1008
+ auto Output = Worklist.begin();
+ for (auto E = Worklist.end(); Output != E; Output++) {
+ if (isCodeObjectCompatible(
----------------
tra wrote:
> The patch description implies that there are at least two classes of
> compatible objects -- the ones that match exactly and the ones that are not
> exact match, but are still compatible.
>
>
> Here we're iterating until we find the first compatible object. What if we
> also have the object that matches exactly, but it's further down the list. Is
> that a problem that we may pick one or the other, depending on the order they
> happen to appear in the worklist? It would be good to add a test case for
> this scenario.
Though it looks plausible, such a case is not possible.
From [[ https://clang.llvm.org/docs/ClangOffloadBundler.html#bundle-entry-id |
Clang Offload Bundler's Documentation]]
> If there is an entry with a target feature specified as Any, then all entries
> must specify that target feature as Any for the same processor.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D134546/new/
https://reviews.llvm.org/D134546
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
