[PATCH] D116542: [OpenMP] Add a flag for embedding a file into the module

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added inline comments.


================
Comment at: clang/lib/CodeGen/BackendUtil.cpp:1774
+      SectionName += ".";
+      SectionName += *BinarySection;
+    }
----------------
JonChesterfield wrote:
> jhuber6 wrote:
> > JonChesterfield wrote:
> > > This looks lossy - if two files use the same section name, they'll end up appended in an order that is probably an implementation quirk of llvm-link, and I think we've thrown away the filename info so can't get back to where we were.
> > > 
> > > Would .llvm.offloading.filename be a reasonable name for each section, with either error on duplicates or warning + discard?
> > We only care about the sections per-file right. When I extract these in the `linker-wrapper` I simply look at each file's sections, and put them into a list of device inputs, we don't need them to be unique as long as there aren't multiple in the same file.
> I think we'll have problems if multiple files are embedded with the same section string, as they'll get concatenated in the output. llvm-link or ld -r on the host bitcode files will hit that.
> 
> It would be worth testing this with two input files, for the same offloading architecture, on amdgpu since I think it will feed the host bitcode to llvm-link which will implicitly concatenate the two embedded files.
This scheme works on AMDGPU because we don't use `llvm-link` as a part of the driver anymore, the new scheme unifies the behavior between NVPTX and AMDGPU until we hit the linker wrapper. But you're right that if the user creates host bitcode and runs llvm-link on that, or performs a relocatable link, we'll get conflicts. I can add a unique string at the end of the section name to avoid this in a later patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116542