Hahnfeld added a comment. In D65819#1627620 <https://reviews.llvm.org/D65819#1627620>, @ABataev wrote:
> In D65819#1627600 <https://reviews.llvm.org/D65819#1627600>, @Hahnfeld wrote: > > > 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. > > > Unfortunately, this is the only possible solution I see. There are already 2 > reports that bundled objects does not work correctly after unbundling. Can you please again share what exactly is the problem, with a small example? I saw discussions on openmp-dev, but that project was huge, and above you were quoting a man page and hinted to global constructors. > Plus, technically, we do not unbundle the original object file, so I would > not call this unbundling at all. Well, after this patch unbundling is strictly required to do anything with the host object. ================ 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(), ---------------- ABataev wrote: > Hahnfeld wrote: > > Hahnfeld wrote: > > > I think we should revert this change and just bundle the host object file > > > as we do for all other targets. > > To be clear: I agree that bundling + unbundling should result in the exact > > same object file, so the other changes seem good, just having the host > > object file easily accessible should be preserved. > We just cannot use partial linking, it does not work for C++. I'm only proposing to use partial linking such that external tools have easy access to the host object. I'm fine with storing another copy of the original host object into a section and fetch it from there during unbundling. 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
