yaxunl marked 3 inline comments as done. yaxunl added inline comments.
================ Comment at: clang/lib/Driver/OffloadBundler.cpp:1008 + auto Output = Worklist.begin(); + for (auto E = Worklist.end(); Output != E; Output++) { + if (isCodeObjectCompatible( ---------------- tra wrote: > saiislam wrote: > > 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. > > > > > Does it mean that the bundler is supposed to error out if I pass > `-targets=hip-amdgcn-amd-amdhsa--gfx906,hip-amdgcn-amd-amdhsa--gfx906:xnack-` > ? > > I've just tried it with a bundler built in recent LLVM tree and it accepts > such a mis of targets without complaining. > > Bundle entries in the same bundle should follow the constraints specified in the clang-offload-bundler documentation. Currently, clang enforces the rule when specifying `--offload-arch` options, which is the most common approach to generate the clang-offload-bundler bundles. However, if users use clang-offload-bundler directly to generate the bundle, the rule is not enforced. I will add the enforcement to clang-offload-bundler too. 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