tra accepted this revision.
tra added a comment.
This revision is now accepted and ready to land.
LGTM.
The patch could use an OK with OMP folks, considering that we've changed the
way offload bunder is invoked for OMP.
================
Comment at: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp:1126
for (StringRef Target : TargetNames) {
+ if (ParsedTargets.count(Target)) {
+ reportError(createStringError(errc::invalid_argument,
----------------
Nit: `.contains(Target)` ?
================
Comment at: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp:985
+ unsigned I = 0;
+ unsigned Last = Worklist.size() - 1;
+ for (auto &E : Sorted) {
----------------
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.
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
