[PATCH] D124525: [OpenMP][ClangLinkerWrapper] Extending linker wrapper to embed metadata for multi-arch fat binaries

saiislam updated this revision to Diff 433401. saiislam added a comment. Added the multi-entry logic in libomptarget. Yet to move the image compatibility testing to plugin. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124525/new/

[PATCH] D124525: [OpenMP][ClangLinkerWrapper] Extending linker wrapper to embed metadata for multi-arch fat binaries

jhuber6 added a comment. Also I probably should've discussed this earlier, but another potential solution is to use the binary format that we use to embed the object files for the images as well. This is

[PATCH] D124525: [OpenMP][ClangLinkerWrapper] Extending linker wrapper to embed metadata for multi-arch fat binaries

jhuber6 added inline comments. Comment at: clang/tools/clang-linker-wrapper/OffloadWrapper.cpp:229-232 +// store value of these variables (i.e. offload archs) into a custom +// section which will be used by "offload-arch -f". It won't be +// removed during binary

[PATCH] D124525: [OpenMP][ClangLinkerWrapper] Extending linker wrapper to embed metadata for multi-arch fat binaries

saiislam added a comment. Thanks for the detailed review. I will update rest of the patch soon. Comment at: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp:613-614 CmdArgs.push_back("-shared"); + std::string ArchArg =

[PATCH] D124525: [OpenMP][ClangLinkerWrapper] Extending linker wrapper to embed metadata for multi-arch fat binaries

saiislam updated this revision to Diff 432352. saiislam marked 8 inline comments as done. saiislam added a comment. Addressed some simple review changes. Will update remaining in the next iteration. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D124525: [OpenMP][ClangLinkerWrapper] Extending linker wrapper to embed metadata for multi-arch fat binaries

jhuber6 added a comment. Where is the code to change the target registration? We need a new initialization runtime call to use and change the old one to reallocate the `__tgt_device_image` array. Did you work around that some other way? Comment at:

[PATCH] D124525: [OpenMP][ClangLinkerWrapper] Extending linker wrapper to embed metadata for multi-arch fat binaries

saiislam updated this revision to Diff 431975. saiislam added a comment. Changed the embedding scheme to add ImageInfo field in __tgt_device_image. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124525/new/ https://reviews.llvm.org/D124525 Files:

[PATCH] D124525: [OpenMP][ClangLinkerWrapper] Extending linker wrapper to embed metadata for multi-arch fat binaries

saiislam added a comment. In D124525#3491170 , @jhuber6 wrote: > I'm suggesting instead we define a new `__tgt_device_image` and a new > `__tgt_register_lib` function to support this. This new `__tgt_device_image` > will simply contain a pointer to an

[PATCH] D124525: [OpenMP][ClangLinkerWrapper] Extending linker wrapper to embed metadata for multi-arch fat binaries

jhuber6 added a comment. I'm suggesting instead we define a new `__tgt_device_image` and a new `__tgt_register_lib` function to support this. This new `__tgt_device_image` will simply contain a pointer to an optional information struct. struct __tgt_device_image_v2 { void*

[PATCH] D124525: [OpenMP][ClangLinkerWrapper] Extending linker wrapper to embed metadata for multi-arch fat binaries

yaxunl added inline comments. Comment at: clang/tools/clang-linker-wrapper/OffloadWrapper.cpp:98-104 + // struct __tgt_image_info { + // int32_t version; + // int32_t image_number; + // int32_t number_images; + // char* offload_arch; + // char*

[PATCH] D124525: [OpenMP][ClangLinkerWrapper] Extending linker wrapper to embed metadata for multi-arch fat binaries

yaxunl added inline comments. Comment at: clang/tools/clang-linker-wrapper/OffloadWrapper.cpp:98-104 + // struct __tgt_image_info { + // int32_t version; + // int32_t image_number; + // int32_t number_images; + // char* offload_arch; + // char*

[PATCH] D124525: [OpenMP][ClangLinkerWrapper] Extending linker wrapper to embed metadata for multi-arch fat binaries

jhuber6 added inline comments. Comment at: clang/tools/clang-linker-wrapper/OffloadWrapper.cpp:246 IRBuilder<> Builder(BasicBlock::Create(C, "entry", Func)); + // Create calls to __tgt_register_image_info for each image + auto *NullPtr =

[PATCH] D124525: [OpenMP][ClangLinkerWrapper] Extending linker wrapper to embed metadata for multi-arch fat binaries

saiislam added a comment. In D124525#3478469 , @yaxunl wrote: > need a test for the generated registration code Yes, I will add tests. Comment at: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp:1161 -

[PATCH] D124525: [OpenMP][ClangLinkerWrapper] Extending linker wrapper to embed metadata for multi-arch fat binaries

yaxunl added a comment. need a test for the generated registration code Comment at: clang/tools/clang-linker-wrapper/OffloadWrapper.cpp:98-104 + // struct __tgt_image_info { + // int32_t version; + // int32_t image_number; + // int32_t number_images; + // char*

[PATCH] D124525: [OpenMP][ClangLinkerWrapper] Extending linker wrapper to embed metadata for multi-arch fat binaries

jhuber6 added inline comments. Comment at: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp:1161 -LinkedImages.push_back(*ImageOrErr); +LinkedImages.emplace_back(TheArch, *ImageOrErr); } I'm doing something similar in D123810, I just used the

[PATCH] D124525: [OpenMP][ClangLinkerWrapper] Extending linker wrapper to embed metadata for multi-arch fat binaries

jdoerfert added inline comments. Comment at: openmp/libomptarget/include/omptarget.h:163 + int32_t image_number; // Image number in image library starting from 0 + int32_t number_images; // Number of images, used for initial allocation + char *offload_arch;//

[PATCH] D124525: [OpenMP][ClangLinkerWrapper] Extending linker wrapper to embed metadata for multi-arch fat binaries

saiislam created this revision. saiislam added reviewers: jdoerfert, JonChesterfield, jhuber6, yaxunl. Herald added a subscriber: guansong. Herald added a project: All. saiislam requested review of this revision. Herald added subscribers: openmp-commits, cfe-commits, sstefan1. Herald added