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

[Alexey Bataev via Phabricator via cfe-commits]

ABataev marked an inline comment as done.
ABataev added a comment.

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. Plus, technically, we do not unbundle the original object file, so I would not call this unbundling at all.



================
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(),
----------------
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++.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65819/new/

https://reviews.llvm.org/D65819