This is an archive of the discontinued LLVM Phabricator instance.

[clang-offload-bundler][NFC] Library-ize ClangOffloadBundler (1/4)
AbandonedPublic

Authored by lamb-j on Jul 7 2022, 9:12 AM.

Details

Summary

Lifting the core functionalities of the clang-offload-bundler into a
user-facing library/API. This will allow online and JIT compilers to
bundle and unbundle files without spawning a new process.

This NFC patch (1/4) lifts the classes and functions used to
implement the clang-offload-bundler into a separate
OffloadBundler.cpp, and defines three top-level API functions in
OfflaodBundler.h

BundleFiles()
UnbundleFiles()
UnbundleArchives()

In successive patches, we aim to:

  1. Refactor global command-line option variables (cl::opt, cl::list, cl::bool, etc.) out of the API and into a Config class that can be passed as a local argument.
  2. Refactor dependence on the bundler executable path into an optional parameter
  3. Move OffloadBundler.cpp and OffloadBundler.h into clang/lib/Driver and clang/include/clang/Driver

Diff Detail

Event Timeline

lamb-j created this revision.Jul 7 2022, 9:12 AM
Herald added a project: Restricted Project. · View Herald Transcript
Herald added a subscriber: mgorny. · View Herald Transcript
lamb-j requested review of this revision.Jul 7 2022, 9:12 AM
Herald added a project: Restricted Project. · View Herald Transcript
lamb-j updated this revision to Diff 442945.Jul 7 2022, 9:15 AM

Adding clang-format

yaxunl added a comment.Jul 7 2022, 9:53 AM

I feel it is better to do the refactoring in one patch, since it is difficult to maintain the integrity of 4 patches. It would be easier to revert or cherry-pick the change.

lamb-j added a comment.Jul 7 2022, 6:58 PM

@yaxunl Are you recommending I combine all 4 patches down into 1 patch? Or combine a subset of patches?

Isn't the offload bundler on it's "way out" (=replaced and then deleted soon)?

Isn't the offload bundler on it's "way out" (=replaced and then deleted soon)?

HIP still uses it to create their fatbinary format for CUDA-like support for multi-architecture binaries as well as their object file format. Once D128914 is landed we can at least compile HIP without it, but we would still need to use it to create the bundled image until AMD decides to support my stuff in their runtime. Beyond that I'm not aware of what the users will be now that OpenMP uses the new interface by default. I'm not in a big hurry to delete this if people still have a use for it, but I'd prefer things to use the new interface if possible. Are there any other situations where we still need the clang-offload-bundler? If there's anything the new interface is lacking I could add it in.

Isn't the offload bundler on it's "way out" (=replaced and then deleted soon)?

HIP still uses it to create their fatbinary format for CUDA-like support for multi-architecture binaries as well as their object file format. Once D128914 is landed we can at least compile HIP without it, but we would still need to use it to create the bundled image until AMD decides to support my stuff in their runtime. Beyond that I'm not aware of what the users will be now that OpenMP uses the new interface by default. I'm not in a big hurry to delete this if people still have a use for it, but I'd prefer things to use the new interface if possible. Are there any other situations where we still need the clang-offload-bundler? If there's anything the new interface is lacking I could add it in.

I think before the new binary format supports Windows and -fno-gpu-rdc and HIP runtime support the new binary format, we cannot deprecate clang-offload-bundler. I expect that would take some time.

@yaxunl Are you recommending I combine all 4 patches down into 1 patch? Or combine a subset of patches?

I recommend combining all patches as one.

I think before the new binary format supports Windows and -fno-gpu-rdc and HIP runtime support the new binary format, we cannot deprecate clang-offload-bundler. I expect that would take some time.

Supporting -fno-gpu-rdc should be easy, this will just require a few tweaks to the driver phases and passing it to the host backend directly. Supporting Windows would be a little tougher, we would need to make sure we can embed / extract the device code with Windows. After that we would need to synthesize the start / stop pointers differently. There is a way to do this on Windows similar to how Unix linkers, there's some examples in ASAN or HWSAN I believe. It's definitely doable but would take some effort, so it definitely won't work in time for the 15 release. I personally don't have much interest in Mac or Windows, but if I want to make this format universal I may need to actually sit down and implement it.

lamb-j abandoned this revision.Jul 15 2022, 9:10 AM

Abandoned in favor of combined patch: https://reviews.llvm.org/D129873