[PATCH] D68166: [Clang][OpenMP Offload] Add new tool for wrapping offload device binaries

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

I think this patch is a behaviour change. Currently, the target binary is embedded in the host binary at link time. With this change, the contents of the binary are embedded in bitcode which is subsequently fed into the link. If indeed so, that seems strictly better - code in the host that cares about the size of the bitcode now has it available at opt time, instead of at link time. The target specific nastiness objcopy would introduce is neatly sidestepped.

This change takes N binaries (that I think need to be for different triples, or the loop doesn't work) and puts them in separate section-annotated bitcode arrays. Equivalent behaviour would result from calling the tool once per binary and passing the N results onward, e.g. to llvm-link.

The functionality of 'take a binary and embed it in bitcode as a const array' is likely to be useful outside of openmp. I've done similar things in the past in non-portable fashion. Aside from the section and symbol names, I don't think there's anything specific to openmp in the tool.

How would you feel about simplifying the tool to work on one file at a time, with an interface that takes the host target (could default to whatever is running the tool) and a string for section name, which generates some bitcode containing that file as a const array plus start/end symbols derived from the section name? The change would involve deleting the multiple file handling and renaming OffloadTargets to SectionName or similar.

clang-offload-wrapper than becomes binary-to-bitcode-embedder (or better, names are hard), with the intent that projects outside of the openmp target offload compiler could use it.



================
Comment at: clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp:84
+  void createImages(ArrayRef<BinaryDesc> Binaries) {
+    for (const BinaryDesc &Bin : Binaries) {
+      StringRef SectionName = SS.save(".omp_offloading." + Bin.second);
----------------
I don't think this works for multiple binaries with the same target triple. They'll all be put in the same section and there will be duplicate symbols for start/end.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68166