sfantao marked 7 inline comments as done. sfantao added a comment. Hi Hal,
Thanks for the review! Comments inlined. ================ Comment at: include/clang/Driver/Action.h:504 + /// unbundling action. + struct DependingActionInfoTy final { + /// \brief The tool chain of the depending action. ---------------- hfinkel wrote: > Don't need 'Ty' in the name of this struct. Ok, using `DependentActionInfo` now. ================ Comment at: lib/Driver/Driver.cpp:2091 + InputArg->getOption().getKind() == llvm::opt::Option::InputClass && + !types::isSrcFile(HostAction->getType())) { + auto UnbundlingHostAction = ---------------- hfinkel wrote: > hfinkel wrote: > > This checks that the file needs to be preprocessed. What does preprocessing > > have to do with this? I don't imagine that providing a preprocessed source > > file as input should invoke the unbundler . > On second thought, this is okay. It does not make sense to have a non-bundled > preprocessed source for the input there, as the host and device compilation > don't share a common preprocessor state. > > We do need to be careful, perhaps, about .s files (which don't need > preprocessing as .S files do) -- we should probably assume that all > non-bundled .s files are host assembly code. Yes, that is what we do. If the bundler tool detects that the input is not a bundle, it assumes it is host code/bits. In either case, we still generate the unbundling tool as the driver doesn't check the contents of the files. ================ Comment at: test/Driver/openmp-offload.c:274 +/// Check separate compilation with offloading - unbundling actions +// RUN: touch %t.i +// RUN: %clang -### -ccc-print-phases -fopenmp -o %t.out -lsomelib -target powerpc64le-linux -fopenmp-targets=powerpc64le-ibm-linux-gnu,x86_64-pc-linux-gnu %t.i 2>&1 \ ---------------- hfinkel wrote: > hfinkel wrote: > > Oh, are you using .i to indicate a bundle instead of a preprocessed file? > > Don't do that. Please use a different suffix -- the bundler has its own > > file format. > Never mind; this is okay too. Ok, there is no particular suffix to indicate a file is a bundle. The (un)bundler, however, has the machinery to detect if a given file is a bundle, it just uses the extension to understand if it is a human readable file, bitcode file, or object file, because the bundle format is different in those three cases. https://reviews.llvm.org/D21853 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits