Hahnfeld added a comment. In D65819#1627036 <https://reviews.llvm.org/D65819#1627036>, @ABataev wrote:
> In D65819#1627032 <https://reviews.llvm.org/D65819#1627032>, @Hahnfeld wrote: > > > In D65819#1617736 <https://reviews.llvm.org/D65819#1617736>, @Hahnfeld > > wrote: > > > > > Will this patch change the ability to consume a bundled object file > > > without calling the unbundler? Using known ELF tools on the produced > > > object files was an important design decision and IIRC was somewhat > > > important for using build systems that are unaware of the bundled format. > > > > > > Ping. > > > Missed this. We still produce correct object files, so all the tools will > work with this. I agree on a technical level that it's still a "correct" object, but not what I was looking for: The host object file will only be in the bundled section, so you cannot examine it without unbundling. For example, with a small test file and `clang -fopenmp -fopenmp-targets=x86_64 -c test.c` I saw the following: $ nm test.o 0000000000000000 t .omp_offloading.requires_reg 0000000000000000 T test U __tgt_register_requires After applying this patch, the output is empty which might be a problem in certain cases. ================ Comment at: tools/clang-offload-bundler/ClangOffloadBundler.cpp:373 /// In order to bundle we create an IR file with the content of each section and /// 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 ---------------- Please update if needed. ================ Comment at: tools/clang-offload-bundler/ClangOffloadBundler.cpp:534 // Do the incremental linking. We write to the output file directly. So, we // close it and use the name to pass down to clang. ---------------- Please update if needed. ================ Comment at: tools/clang-offload-bundler/ClangOffloadBundler.cpp:541-549 std::vector<StringRef> ClangArgs = {"clang", - "-r", + "-c", "-target", TargetName.c_str(), "-o", OutputFileNames.front().c_str(), - InputFileNames[HostInputIndex].c_str(), ---------------- I think we should revert this change and just bundle the host object file as we do for all other targets. ================ Comment at: tools/clang-offload-bundler/ClangOffloadBundler.cpp:572 if (Failed) { errs() << "error: incremental linking by external tool failed.\n"; return true; ---------------- Please update if needed. 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
