yaxunl marked 3 inline comments as done.
yaxunl added inline comments.
================
Comment at: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp:980
+ if (FailOnMissingBundles && !Worklist.empty()) {
+ std::string ErrMsg = "Can't find bundles for";
+ std::set<StringRef> Sorted;
----------------
tra wrote:
> Do we need to report complete list of missing bundles?
> Can we just report the first one we've encountered and abort the command?
>
Better to report all missing bundles since the info is available here and
useful.
================
Comment at: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp:985
+ unsigned I = 0;
+ unsigned Last = Worklist.size() - 1;
+ for (auto &E : Sorted) {
----------------
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.
================
Comment at: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp:998-999
+
// If no bundles were found, assume the input file is the host bundle and
// create empty files for the remaining targets.
if (Worklist.size() == TargetNames.size()) {
----------------
tra wrote:
> Can we, instead use an explicit option to enable this 'make empty files for
> missing targets' and report missing targets as an error otherwise?
> I would assume that missing a target explicitly specified on command line
> would be an error more often than not. After all we've asked to extract
> something which implies that that 'something' exists. I.e. if I tell unzip to
> unpack a non-existing file from an archive, I do not expect it to give me an
> empty file.
will do
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
