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
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
