Hahnfeld requested changes to this revision. Hahnfeld added a comment. This revision now requires changes to proceed.
The code changes look good to me, but the test doesn't pass on x86. We've faced the same problem when `clang-offload-bundler` was initially committed and the current testing is the best we were able to do. ================ Comment at: test/Driver/clang-offload-bundler.c:223-227 // Check object bundle/unbundle. The content should be bundled into an ELF // section (we are using a PowerPC little-endian host which uses ELF). We // have an already bundled file to check the unbundle and do a dry run on the // bundling as it cannot be tested in all host platforms that will run these // tests. ---------------- This still holds: I can't partially link PPC object files on x86_64. Please revert the test changes to not actually perform the bundling. ================ Comment at: tools/clang-offload-bundler/ClangOffloadBundler.cpp:373-375 /// use incremental linking to produce the resulting object. We also add section /// with a single byte to state the name of the component the main object file /// (the one we are bundling into) refers to. ---------------- This isn't true anymore. ================ Comment at: tools/clang-offload-bundler/ClangOffloadBundler.cpp:377 /// -/// To unbundle, we use just copy the contents of the designated section. If the -/// requested bundle refer to the main object file, we just copy it with no -/// changes. +/// To unbundle, we use just copy the contents of the designated section. class ObjectFileHandler final : public FileHandler { ---------------- I know this has been wrong before, but can you please fix to `we just copy` without `use`? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65819/new/ https://reviews.llvm.org/D65819 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits