sdmitriev marked an inline comment as done. sdmitriev added inline comments.
================ Comment at: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp:888 + // treat missing host triple as error if we do unbundling. + if ((Unbundle && HostTargetNum > 1) || (!Unbundle && HostTargetNum != 1)) { Error = true; ---------------- ABataev wrote: > ABataev wrote: > > sdmitriev wrote: > > > sdmitriev wrote: > > > > sdmitriev wrote: > > > > > ABataev wrote: > > > > > > sdmitriev wrote: > > > > > > > ABataev wrote: > > > > > > > > I believe, for unbundling we also must check for `!= 1` rather > > > > > > > > than `> 1`. Zero host targets also is not allowed. > > > > > > > But the whole idea of this change is to remove requirement to > > > > > > > provide host triple for unbundling operation. Target bundle(s) > > > > > > > can always be extracted without extracting host, so host bundle > > > > > > > is optional. Therefore zero host targets should not be considered > > > > > > > as error for unbundling. > > > > > > And why do we need this? I think it would be better to check that > > > > > > the requested host triple matches the bundled one using this > > > > > > parameter rather than removing it. > > > > > > And why do we need this? > > > > > > > > > > As I wrote in the summary it is a usability issue. You may for > > > > > example want to extract device object for a particular offload target > > > > > to examine its contents (symbols, sections, etc..), but currently you > > > > > also have to extract host bundle as well even if you do not need it. > > > > > > > > > > > I think it would be better to check that the requested host triple > > > > > > matches the bundled one using this parameter rather than removing > > > > > > it. > > > > > > > > > > So you suggest to check that host bundle name that exists in the fat > > > > > image matches the host bundle name provided it command line if it was > > > > > provided? Should it be an error if names do not match? > > > > > > > > > I have updated patch to do error checking if host bundle name was > > > > provided in command line. > > > @ABataev I believe the host bundle name is now being checked as you > > > suggested. Can you please confirm that it matches your expectations? > > Ok, I got the idea of the patch. BTW, will happen if I request the device > > code for the triple, which is correct, but bundled container does not have > > the device code for this triple? > What about this? Based on the comments on lines 775-776 and 801 in ClangOffloadBundler.cpp bundler will create empty files for missing device bundles. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66601/new/ https://reviews.llvm.org/D66601 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits