Skip to content

[SYCL] Fix SegFault when accessor passed to reduction is destroyed early #3498

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

v-klochkov
Copy link
Contributor

@v-klochkov v-klochkov commented Apr 6, 2021

The error may happen when the copy of the accessor passed to ONEAPI::reduction
is destroyed right after being passed to reduction.
The fix is simple - to create a new copy of the given accessor.

The corresponding LIT test changes: intel/llvm-test-suite#222

Signed-off-by: Vyacheslav N Klochkov [email protected]

The error may happen when the copy of the accessor passed to ONEAPI::reduction
is destroyed right after being passed to reduction.
The fix is simple - to create a new copy of the given accessor.

Signed-off-by: Vyacheslav N Klochkov <[email protected]>
@@ -508,30 +508,24 @@ class reduction_impl : private reduction_impl_base {
}

/// Constructs reduction_impl when the identity value is statically known.
// Note that aliasing constructor was used to initialize MRWAcc to avoid
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why "aliasing constructor" was not enough?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea of using aliasing constructor was to give the pointer to user's accessor to shared_ptr, but not release memory (i.e. user's accessor) when that shared_ptr is destroyed, at least that is how I understood it when wrote that code.

That was wrong approach for cases where user's accessor is destroyed before the accessor is needed in reduction computation.
At this line: https://github.com/v-klochkov/llvm-test-suite/blob/public_vklochkov_reduction_ctor_acc_destroyed_early/SYCL/Reduction/reduction_nd_lambda.cpp#L41
MRWAcc shared_ptr holded a pointer to released memory.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, thanks for explanation

Copy link
Contributor

@smaslov-intel smaslov-intel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@againull againull merged commit b80f13e into intel:sycl Apr 7, 2021
@v-klochkov v-klochkov deleted the public_vklockov_reduction_acc_ctor_error_fix branch April 7, 2021 06:00
v-klochkov added a commit to v-klochkov/llvm-test-suite that referenced this pull request Apr 7, 2021
The corresponding change in SYCL RT: intel/llvm#3498

Signed-off-by: Vyacheslav N Klochkov <[email protected]>
againull pushed a commit to intel/llvm-test-suite that referenced this pull request Apr 8, 2021
…or (#222)

The corresponding change in SYCL RT: intel/llvm#3498

Signed-off-by: Vyacheslav N Klochkov <[email protected]>
aelovikov-intel pushed a commit to aelovikov-intel/llvm that referenced this pull request Mar 27, 2023
…or (intel/llvm-test-suite#222)

The corresponding change in SYCL RT: intel#3498

Signed-off-by: Vyacheslav N Klochkov <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants