[SYCL][NFC] Move SYCL Module Splitting to library. Part 1

[maksimsab]

This patch is a part 1 of moving sycl-post-link functionality to library for New Offloading initiative.
This patch replaces std::vector<char> with MemoryBuffer as a container for accessing device image in the SYCLImage class. MemoryBuffer uses mmap that allows not to hold the whole device image in the RAM.

[asudarsa]

Hi @maksimsab

Can you please provide a high-level description of why we need LazyMemoryBuffer and how it will be used in the SYCL compilation flow?

Thanks

[maksimsab]

Sure.
Currently, sycl-post-link has a trick where all splitted modules are being saved on the Disk because the total amount of data is too large to keep it in RAM.
clang-linker-wrapper is not an exception in that matter and the current interface of Offload Wrapping requires keeping all data in RAM while it is impossible for big images. This patch changes the interface taking into account this case.

[AlexeySachkov]

Overall, it seems like something that should maybe go directly to the upstream

[maksimsab]

Btw, I could try to add Lazy mode to MemoryBuffer apart from the work of upstreaming.
The current LazyMemoryBuffer is useless in upstream without its usage which is SYCL offload wrapping.

[AlexeySachkov]

Btw, I could try to add Lazy mode to MemoryBuffer apart from the work of upstreaming. The current LazyMemoryBuffer is useless in upstream without its usage which is SYCL offload wrapping.

Don't we want to unify the paths taken for both SYCL and non-SYCL offloading? I mean, it is possible to maybe switch existing linker-wrapper code to use lazy buffers? Would it make sense?

[maksimsab]

Probably, we could accept usual MemoryBuffer as they already do here:

llvm/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp

Lines 1555 to 1559 in 70cca70

	/// Creates the object file containing the device image and runtime
	/// registration code from the device images stored in \p Images.
	Expected<StringRef>
	wrapDeviceImages(ArrayRef<std::unique_ptr<MemoryBuffer>> Buffers,
	const ArgList &Args, OffloadKind Kind) {

Because, MemoryBuffer uses mmap and file's content is not loaded in the beginning.

[AlexeySachkov]

Btw, I could try to add Lazy mode to MemoryBuffer apart from the work of upstreaming. The current LazyMemoryBuffer is useless in upstream without its usage which is SYCL offload wrapping.

Don't we want to unify the paths taken for both SYCL and non-SYCL offloading? I mean, it is possible to maybe switch existing linker-wrapper code to use lazy buffers? Would it make sense?

To clarify here what I mean: my idea was to introduce LazyMemoryBuffer to the upstream and start using it in CUDA & HIP flows right away to justify addition of the components. I'm not saying that we should rewrite everything in SYCL flow right away.

[maksimsab]

My concern is that MemoryBuffer is already satisfies performance requirement that I've tried to achieve with LazyMemoryBuffer. Because MemoryBuffer uses mmap here.

llvm/llvm/lib/Support/MemoryBuffer.cpp

Lines 484 to 486 in 70cca70

	std::unique_ptr<MB> Result(
	new (NamedBufferAlloc(Filename)) MemoryBufferMMapFile<MB>(
	RequiresNullTerminator, FD, MapSize, Offset, EC));

And this is how CUDA and HIP flows are potentially do not experience RAM issues.
It isn't clear from the MemoryBuffer's doxygen page. It is clear from the source code though.

[bader]

And this is how CUDA and HIP flows are potentially do not experience RAM issues.

Just me speculating as I don't know for sure.
CUDA and HIP flows might experience RAM issues in fullLTO mode, but they also support thinLTO, which consumes much less memory. In addition to that they do not "split" module. Each "split" duplicates parts of LLVM module, which makes SYCL flow to demand more memory comparing to CUDA and HIP flows.

[maksimsab]

@asudarsa @AlexeySachkov
Updated PR and description. Please, take a look.

[asudarsa]

Is it possible to reuse or extend llvm::OffloadBinary::OffloadingImage data structure for this? On cursory look, it seems feasible. ImageKind is the same as Format here. OffloadKind is OFK_SYCL. Entries and Properties could be moved into StringData.
https://llvm.org/docs/doxygen/structllvm_1_1object_1_1OffloadBinary_1_1OffloadingImage.html

I am Ok with adding a TODO here and doing this in part 2 of changes.

Thanks

[maksimsab]

This is a good idea. I will think through it after this patch.

[asudarsa]

LGTM. Just a couple of clarifications requested. Please check the comment about 'assert' before merging this change.

Thanks

[asudarsa]

Test failures not due to this change. Merging this. Thanks