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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
