[PATCH] D93068: [clang-offload-bundler] Add option -allow-missing-bundles

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl marked 2 inline comments as done.
yaxunl added a comment.

@ABataev Is this patch OK for OpenMP? It is NFC for OpenMP toolchain but affects using clang-offload-bundler as a standalone tool. Thanks.



================
Comment at: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp:1126
   for (StringRef Target : TargetNames) {
+    if (ParsedTargets.count(Target)) {
+      reportError(createStringError(errc::invalid_argument,
----------------
tra wrote:
> Nit: `.contains(Target)` ? 
will fix


================
Comment at: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp:985
+    unsigned I = 0;
+    unsigned Last = Worklist.size() - 1;
+    for (auto &E : Sorted) {
----------------
tra wrote:
> yaxunl wrote:
> > tra wrote:
> > > This assumes that all items on the `WorkList` were unique. 
> > > If some of the WorkList items were duplicated, then there will be fewer items in `Sorted` and the ` I == Last` comparison will never be true.
> > > I'd use `Sorted.size()` instead.
> > clang-offload-bundler does not allow duplicated targets. It asserts when duplicated targets in options are found when unbundling. Will add check for duplicate targets in options and emit error.
> I'd still use `Sorted.size()` as it's the `Sorted` you're iterating on.
will fix


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

https://reviews.llvm.org/D93068