Skip to content
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

Allow Kokkos::create_mirror* to be used with WithoutInitializing #4486

Merged

Conversation

masterleinad
Copy link
Contributor

@masterleinad masterleinad commented Oct 29, 2021

Fixes #4259. For the moment, I only considered the overloads for Kokkos::View and not yet the other View-like containers.

@masterleinad
Copy link
Contributor Author

With

int main(int argc, char* argv[]) {
  Kokkos::initialize(argc, argv);
  {
    Kokkos::View<int*, Kokkos::Experimental::SYCLDeviceUSMSpace> device_view("device view", 10);
    Kokkos::View<int*, Kokkos::HostSpace> host_view("host view", 10);

    auto mirror_device = Kokkos::create_mirror(device_view);
    auto mirror_host = Kokkos::create_mirror(Kokkos::Experimental::SYCLDeviceUSMSpace{}, host_view);
    auto mirror_device_view = Kokkos::create_mirror_view(device_view);
    auto mirror_host_view = Kokkos::create_mirror_view(Kokkos::Experimental::SYCLDeviceUSMSpace{}, host_view);
  }
  {
    Kokkos::View<int*, Kokkos::Experimental::SYCLDeviceUSMSpace> device_view("device view", 10);
    Kokkos::View<int*, Kokkos::HostSpace> host_view("host view", 10);

    auto mirror_device_wi = Kokkos::create_mirror(Kokkos::WithoutInitializing, device_view);
    auto mirror_host_wi = Kokkos::create_mirror(Kokkos::WithoutInitializing, Kokkos::Experimental::SYCLDeviceUSMSpace{}, host_view);
    auto mirror_device_view_wi = Kokkos::create_mirror(Kokkos::WithoutInitializing, device_view);
    auto mirror_host_view_wi = Kokkos::create_mirror(Kokkos::WithoutInitializing, Kokkos::Experimental::SYCLDeviceUSMSpace{}, host_view);
  }
  Kokkos::finalize();
}

I am getting

KokkosP: Example Library Initialized (sequence is 0, version: 20210623)
KokkosP: Allocate<SYCLDeviceUSM> name: device view pointer: 0x2970a80 size: 40
KokkosP: Executing parallel-for kernel on device 117440513 with unique execution identifier 0
KokkosP: Kokkos::View::initialization [device view] via memset
KokkosP: Execution of kernel 0 is completed.
KokkosP: Allocate<Host> name: host view pointer: 0x2926180 size: 40
KokkosP: Executing parallel-for kernel on device 1 with unique execution identifier 1
KokkosP: Kokkos::View::initialization [host view] via memset
KokkosP: Execution of kernel 1 is completed.
KokkosP: Allocate<Host> name: device view_mirror pointer: 0x2972780 size: 40
KokkosP: Executing parallel-for kernel on device 1 with unique execution identifier 2
KokkosP: Kokkos::View::initialization [device view_mirror] via memset
KokkosP: Execution of kernel 2 is completed.
KokkosP: Allocate<SYCLDeviceUSM> name: host view pointer: 0x2972f00 size: 40
KokkosP: Executing parallel-for kernel on device 117440513 with unique execution identifier 3
KokkosP: Kokkos::View::initialization [host view] via memset
KokkosP: Execution of kernel 3 is completed.
KokkosP: Allocate<Host> name: device view_mirror pointer: 0x2973e40 size: 40
KokkosP: Executing parallel-for kernel on device 1 with unique execution identifier 4
KokkosP: Kokkos::View::initialization [device view_mirror] via memset
KokkosP: Execution of kernel 4 is completed.
KokkosP: Allocate<SYCLDeviceUSM> name: host view pointer: 0x2974600 size: 40
KokkosP: Executing parallel-for kernel on device 117440513 with unique execution identifier 5
KokkosP: Kokkos::View::initialization [host view] via memset
KokkosP: Execution of kernel 5 is completed.
KokkosP: Deallocate<SYCLDeviceUSM> name: ceUSM pointer: 0x2974600 size: 40
KokkosP: Deallocate<Host> name: device view_mirror pointer: 0x2973e40 size: 40
KokkosP: Deallocate<SYCLDeviceUSM> name: ceUSM pointer: 0x2972f00 size: 40
KokkosP: Deallocate<Host> name: device view_mirror pointer: 0x2972780 size: 40
KokkosP: Deallocate<Host> name: host view pointer: 0x2926180 size: 40
KokkosP: Deallocate<SYCLDeviceUSM> name: ceUSM pointer: 0x2970a80 size: 40
KokkosP: Kokkos library finalization called.

for the overloads without WithoutInitializing and

KokkosP: Example Library Initialized (sequence is 0, version: 20210623)
KokkosP: Allocate<SYCLDeviceUSM> name: device view pointer: 0x1ef2180 size: 40
KokkosP: Executing parallel-for kernel on device 117440513 with unique execution identifier 0
KokkosP: Kokkos::View::initialization [device view] via memset
KokkosP: Execution of kernel 0 is completed.
KokkosP: Allocate<Host> name: host view pointer: 0x1e9bb40 size: 40
KokkosP: Executing parallel-for kernel on device 1 with unique execution identifier 1
KokkosP: Kokkos::View::initialization [host view] via memset
KokkosP: Execution of kernel 1 is completed.
KokkosP: Allocate<Host> name: device view_mirror pointer: 0x1ef3bc0 size: 40
KokkosP: Allocate<SYCLDeviceUSM> name: host view pointer: 0x1ef4300 size: 40
KokkosP: Allocate<Host> name: device view_mirror pointer: 0x1ef4cc0 size: 40
KokkosP: Allocate<SYCLDeviceUSM> name: host view pointer: 0x1ef5400 size: 40
KokkosP: Deallocate<SYCLDeviceUSM> name: ceUSM pointer: 0x1ef5400 size: 40
KokkosP: Deallocate<Host> name: device view_mirror pointer: 0x1ef4cc0 size: 40
KokkosP: Deallocate<SYCLDeviceUSM> name: ceUSM pointer: 0x1ef4300 size: 40
KokkosP: Deallocate<Host> name: device view_mirror pointer: 0x1ef3bc0 size: 40
KokkosP: Deallocate<Host> name: host view pointer: 0x1e9bb40 size: 40
KokkosP: Deallocate<SYCLDeviceUSM> name: ceUSM pointer: 0x1ef2180 size: 40
KokkosP: Kokkos library finalization called.

with WithoutInitializing. We should add a test like this once we have #4331.

@masterleinad masterleinad marked this pull request as ready for review November 1, 2021 14:37
@masterleinad masterleinad added this to In progress in Kokkos Release 3.6 via automation Nov 1, 2021
@masterleinad masterleinad added this to Awaiting Feedback in Developer: Daniel Arndt Nov 1, 2021
@masterleinad masterleinad moved this from In progress to Awaiting Feedback in Kokkos Release 3.6 Nov 1, 2021
@masterleinad masterleinad force-pushed the create_mirror_withoutinitializing branch from e965035 to 00ee5c8 Compare November 1, 2021 15:33
@masterleinad masterleinad force-pushed the create_mirror_withoutinitializing branch from 00ee5c8 to 597d312 Compare November 8, 2021 22:19
@masterleinad
Copy link
Contributor Author

I added a test after we merged #4331. This only fixes some issues when including the basic testing include files.

@masterleinad
Copy link
Contributor Author

Retest this please.

@masterleinad masterleinad force-pushed the create_mirror_withoutinitializing branch from 597d312 to 5028a81 Compare November 10, 2021 16:39
Copy link
Contributor

@janciesko janciesko left a comment

Choose a reason for hiding this comment

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

LGTM

@rgayatri23 rgayatri23 self-requested a review November 24, 2021 19:02
Copy link
Member

@crtrott crtrott left a comment

Choose a reason for hiding this comment

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

Are we introducing all kinds of overloads with unconstrained additional arguments here? If so shouldn't those be functions in the Impl namespace?

@masterleinad
Copy link
Contributor Author

Are we introducing all kinds of overloads with unconstrained additional arguments here? If so shouldn't those be functions in the Impl namespace?

All the overloads with unconstrained arguments (for WithouutInitializing and other view properties) are in namespace Impl already.

Copy link
Member

@crtrott crtrott left a comment

Choose a reason for hiding this comment

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

Ah saw the namespace stuff now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Developer: Daniel Arndt
Awaiting Feedback
Development

Successfully merging this pull request may close these issues.

None yet

4 participants