[PATCH] D65819: [Driver][Bundler] Improve bundling of object files.

Jonas Hahnfeld via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Aug 15 06:54:57 PDT 2019

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`?

  rC Clang



More information about the cfe-commits mailing list