[SYCL] Prevent stream buffer leak on constructor exception #4594
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The `sycl::stream` class is currently throwing a `sycl::invalid_parameter_error` exception in the constructor when the `MaxStatementSize` parameter exceeds the `MAX_STATEMENT_SIZE` limitation. However, this exception is thrown after the underlying `stream_impl` object and the corresponding accessors are initialized. `stream_impl` allocates the buffers for the stream but leaves it to the scheduler to deallocate it. Since the exception is thrown prior to registering the stream with the command-group handler, the allocated buffers will leak if the exception is thrown. These changes prevents the memory leak by making the check prior to the initialization of the `stream_impl`, which in turn avoids the creation of the stream buffers and accessors if the check fails. This also avoids allocating memory that will never be used. Signed-off-by: Steffen Larsen <[email protected]>
|
I will add a unit test for this asap. |
I decided to go with a regression test instead. Test PR is up here: intel/llvm-test-suite#467 |
Signed-off-by: Steffen Larsen <[email protected]>
|
@romanovvlad suggested I make the regression test a unit test instead, so I have moved it as suggested. |
Signed-off-by: Steffen Larsen <[email protected]>
Signed-off-by: Steffen Larsen <[email protected]>
alexbatashev
added a commit
to alexbatashev/llvm
that referenced
this pull request
Sep 24, 2021
* upstream/sycl: (2344 commits) [ESIMD] Rename slm_load4/slm_store4 to slm_load_rgba/slm_store_rgba (intel#4158) [SYCL] Avoid re-computing group_range in nd_item::get_group_range() (intel#4621) [clang-offload-extract] Ignore zero padding in .tgting section (intel#4622) [Driver][SYCL] Fix -fsycl-help output when redirected (intel#4619) [Driver][SYCL][FPGA] Do not unbundle aoco as an archive for hardware (intel#4477) [Driver][SYCL] Fix offload-bundler and offload-deps triples (intel#4616) [SYCL] Fix bit_cast for half type (intel#4603) [SYCL] Fix a typo in accessor::get_range method (intel#4556) [SYCL] Detach allocas from their dependencies regardless of linked alloca presence (intel#4573) [SYCL][L0] Make sure that we only query/sync host-visible events from the host. (intel#4613) Fix tests with wrong alias metadata [Driver][SYCL] Fixup arguments to llc call for PIC and code-model (intel#4614) [SYCL][L0] Add ownership control for LeveL-Zero kernel_bundle interop. (intel#4576) [SYCL][Driver] Expose llvm-foreach --jobs functionality through a driver option (intel#4543) [SYCL] Prevent stream buffer leak on constructor exception (intel#4594) [ESIMD] Replace mask_type_t with simd_mask to represent Gen predicates. (intel#4230) Fix for a bunch of fixed point integer SPIR-V instructions (intel#1213) Amend SingleElementVectorINTEL decoration use cases according to spec update (intel#1192) Enforce UserSemantic decoration if no FPGA decorations found [SYCL][CUDA] Fix context scope in kernel launch (intel#4606) ...
steffenlarsen
deleted the
steffen/prevent_stream_buffer_allocation_on_exception
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The
sycl::streamclass is currently throwing asycl::invalid_parameter_errorexception in the constructor when theMaxStatementSizeparameter exceeds theMAX_STATEMENT_SIZElimitation. However, this exception is thrown afterthe underlying
stream_implobject and the corresponding accessors are initialized.stream_implallocates the buffers for the stream but leaves it to the scheduler to deallocate it. Since the exception is thrown prior to registering the stream with the command-group handler, the allocated buffers will leak if the exception is thrown.These changes prevents the memory leak by making the check prior to the initialization of the
stream_impl, which in turn avoids the creation of the stream buffers and accessors if the check fails. This also avoids allocating memory that will never be used.