Skip to content

Uninitialized refactors#2549

Merged
danhoeflinger merged 19 commits intomainfrom
dev/dhoeflin/unitialized_refactors
Mar 11, 2026
Merged

Uninitialized refactors#2549
danhoeflinger merged 19 commits intomainfrom
dev/dhoeflin/unitialized_refactors

Conversation

@danhoeflinger
Copy link
Copy Markdown
Contributor

This commit refactors fill operations and corrects access modes for uninitialized memory APIs in oneDPL.

1. Removed Redundant Fill Functor

  • Eliminated the fill_functor struct that was wrapping a simple assignment operation
  • Updated __pattern_fill to directly use __brick_fill instead, simplifying the implementation

2. Fixed Access Modes for Uninitialized APIs

  • Added __pattern_uninitialized_walk1 and __pattern_uninitialized_walk1_n functions that properly use sycl::access_mode::write with _NoInit=false
  • Updated the following uninitialized memory operations to use the corrected access modes:
    • uninitialized_fill / uninitialized_fill_n
    • uninitialized_default_construct / uninitialized_default_construct_n
    • uninitialized_value_construct / uninitialized_value_construct_n

3. Removed Unnecessary Pattern Functions

  • Removed __pattern_walk_brick and __pattern_walk_brick_n wrappers that were no longer needed

MikeDvorskiy
MikeDvorskiy previously approved these changes Dec 23, 2025
Copy link
Copy Markdown
Contributor

@MikeDvorskiy MikeDvorskiy left a comment

Choose a reason for hiding this comment

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

LGTM

@danhoeflinger
Copy link
Copy Markdown
Contributor Author

This PR is a follow up to #2519 so lets land that first. Thanks for the approval.

@danhoeflinger danhoeflinger force-pushed the dev/dhoeflin/unitialized_refactors branch from 6788a87 to ac2143a Compare December 29, 2025 14:43
@danhoeflinger danhoeflinger changed the title Unitialized refactors Uninitialized refactors Dec 31, 2025
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors uninitialized memory operations in oneDPL to improve code clarity and correctness. It removes the redundant fill_functor wrapper struct, introduces specialized __pattern_uninitialized_walk1 functions with proper SYCL access modes (write with _NoInit=false), and eliminates unnecessary wrapper functions __pattern_walk_brick and __pattern_walk_brick_n.

Key changes:

  • Removed fill_functor struct and replaced its usage with direct __brick_fill calls
  • Added __pattern_uninitialized_walk1 and __pattern_uninitialized_walk1_n functions that use correct SYCL access modes for uninitialized memory operations
  • Updated all uninitialized memory APIs (uninitialized_fill, uninitialized_value_construct, uninitialized_default_construct, and their _n variants) to use the new pattern functions

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.

Show a summary per file
File Description
include/oneapi/dpl/pstl/hetero/algorithm_impl_hetero.h Removed fill_functor struct and __pattern_walk_brick* functions; added __pattern_uninitialized_walk1* functions; updated __pattern_fill to use __brick_fill directly
include/oneapi/dpl/pstl/algorithm_impl.h Removed __pattern_walk_brick* implementations; added __pattern_uninitialized_walk1* wrappers
include/oneapi/dpl/pstl/algorithm_fwd.h Updated forward declarations to replace __pattern_walk_brick* with __pattern_uninitialized_walk1*
include/oneapi/dpl/pstl/glue_memory_impl.h Updated uninitialized memory APIs to use __pattern_uninitialized_walk1* and __pattern_fill*
include/oneapi/dpl/pstl/memory_ranges_impl.h Updated ranges-based uninitialized APIs to use new pattern functions
include/oneapi/dpl/pstl/hetero/histogram_impl_hetero.h Replaced fill_functor with __brick_fill
include/oneapi/dpl/internal/async_impl/async_impl_hetero.h Replaced fill_functor with __brick_fill
test/general/implementation_details/device_copyable.pass.cpp Removed fill_functor device copyability tests

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@danhoeflinger danhoeflinger force-pushed the dev/dhoeflin/unitialized_refactors branch from 36ae926 to 03c83d1 Compare January 2, 2026 22:33
@danhoeflinger danhoeflinger added this to the 2022.12.0 milestone Jan 5, 2026
@danhoeflinger danhoeflinger force-pushed the dev/dhoeflin/unitialized_refactors branch from 03c83d1 to 08eb860 Compare January 5, 2026 18:12
@SergeyKopienko
Copy link
Copy Markdown
Contributor

SergeyKopienko commented Jan 13, 2026

I see you have some conflicts between the branches dev/dhoeflin/unitialized_refactors and dev/dhoeflin/align_write_no_init
So let's resolve them at first and then review your changes.

@danhoeflinger danhoeflinger force-pushed the dev/dhoeflin/unitialized_refactors branch from 08eb860 to f02042d Compare January 13, 2026 13:50
@danhoeflinger
Copy link
Copy Markdown
Contributor Author

I see you have some conflicts between the branches dev/dhoeflin/unitialized_refactors and dev/dhoeflin/align_write_no_init So let's resolve them at first and then review your changes.

Fixed, this was due to the pulling out of fill access mode changes into #2555

Comment thread include/oneapi/dpl/pstl/hetero/algorithm_impl_hetero.h
SergeyKopienko
SergeyKopienko previously approved these changes Feb 10, 2026
Copy link
Copy Markdown
Contributor

@SergeyKopienko SergeyKopienko left a comment

Choose a reason for hiding this comment

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

LGTM

@danhoeflinger danhoeflinger force-pushed the dev/dhoeflin/unitialized_refactors branch from f02042d to e029ce4 Compare February 10, 2026 16:40
@danhoeflinger
Copy link
Copy Markdown
Contributor Author

I had to resolve some conflicts with the previous branch.

Also, this needs to wait for #2519 to merge, so please review that if you want these changes in.

@danhoeflinger danhoeflinger force-pushed the dev/dhoeflin/align_write_no_init branch from af1999d to c817512 Compare February 19, 2026 14:20
Base automatically changed from dev/dhoeflin/align_write_no_init to main February 20, 2026 15:33
@danhoeflinger danhoeflinger dismissed stale reviews from SergeyKopienko and MikeDvorskiy February 20, 2026 15:33

The base branch was changed.

@danhoeflinger danhoeflinger force-pushed the dev/dhoeflin/unitialized_refactors branch from e029ce4 to 46ff098 Compare February 20, 2026 16:41
@danhoeflinger
Copy link
Copy Markdown
Contributor Author

@SergeyKopienko @MikeDvorskiy I've now rebased this after merging #2519. CI looks to be in good shape. Please take another look.

Comment thread include/oneapi/dpl/pstl/algorithm_fwd.h
Comment thread include/oneapi/dpl/pstl/hetero/algorithm_impl_hetero.h Outdated
danhoeflinger and others added 17 commits March 2, 2026 11:05
fix access modes for uninitialized apis
remove unnecessary pattern

Signed-off-by: Dan Hoeflinger <dan.hoeflinger@intel.com>
Signed-off-by: Dan Hoeflinger <dan.hoeflinger@intel.com>
Signed-off-by: Dan Hoeflinger <dan.hoeflinger@intel.com>
Signed-off-by: Dan Hoeflinger <dan.hoeflinger@intel.com>
Signed-off-by: Dan Hoeflinger <dan.hoeflinger@intel.com>
Signed-off-by: Dan Hoeflinger <dan.hoeflinger@intel.com>
Signed-off-by: Dan Hoeflinger <dan.hoeflinger@intel.com>
Signed-off-by: Dan Hoeflinger <dan.hoeflinger@intel.com>
Signed-off-by: Dan Hoeflinger <dan.hoeflinger@intel.com>
Signed-off-by: Dan Hoeflinger <dan.hoeflinger@intel.com>
Signed-off-by: Dan Hoeflinger <dan.hoeflinger@intel.com>
Signed-off-by: Dan Hoeflinger <dan.hoeflinger@intel.com>
Signed-off-by: Dan Hoeflinger <dan.hoeflinger@intel.com>
Signed-off-by: Dan Hoeflinger <dan.hoeflinger@intel.com>
Co-authored-by: Alexey Kukanov <alexey.kukanov@intel.com>
@danhoeflinger danhoeflinger force-pushed the dev/dhoeflin/unitialized_refactors branch from 0b61bcf to 0a56ec7 Compare March 2, 2026 16:05
Copy link
Copy Markdown
Contributor

@MikeDvorskiy MikeDvorskiy left a comment

Choose a reason for hiding this comment

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

Left a comment about _IsNoInitRequested

Comment thread include/oneapi/dpl/pstl/hetero/memory_impl_hetero.h Outdated
Comment thread include/oneapi/dpl/pstl/hetero/memory_impl_hetero.h
Signed-off-by: Dan Hoeflinger <dan.hoeflinger@intel.com>
@akukanov akukanov dismissed MikeDvorskiy’s stale review March 6, 2026 19:26

The indicated issues have been addressed.

Copy link
Copy Markdown
Contributor

@akukanov akukanov left a comment

Choose a reason for hiding this comment

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

Looks good to me.

Comment thread include/oneapi/dpl/pstl/memory_impl.h Outdated
Co-authored-by: Alexey Kukanov <alexey.kukanov@intel.com>
@danhoeflinger danhoeflinger requested a review from akukanov March 11, 2026 15:29
@danhoeflinger danhoeflinger merged commit 4526a70 into main Mar 11, 2026
23 checks passed
@danhoeflinger danhoeflinger deleted the dev/dhoeflin/unitialized_refactors branch March 11, 2026 17:10
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.

5 participants