Skip to content

Rework the logic in memory's copy_from_device#618

Merged
oleksandr-pavlyk merged 1 commit intomasterfrom
crash-in-usm_ndarray-to_device
Oct 4, 2021
Merged

Rework the logic in memory's copy_from_device#618
oleksandr-pavlyk merged 1 commit intomasterfrom
crash-in-usm_ndarray-to_device

Conversation

@oleksandr-pavlyk
Copy link
Copy Markdown
Contributor

In deciding where a copy from device could be done using a direct
call to memcpy, we queried usm type of the source allocation with
respect to the context to which the destination allocation was
bound.

The test was assuming that any answer other than 'unknown' indicated
that the USM allocation was known and a memcpy could be called.

This did not work well when destination was a host device. For example,
this crashed:

SYCL_ENABLE_HOST_DEVICE=1 python -m pytest dpctl/tests/test_usm_ndarray_ctor.py::test_to_device

It passes now.

In deciding where a copy from device could be done using a direct
call to memcpy, we queried usm type of the source allocation with
respect to the context to which the destination allocation was
bound.

The test was assuming that any answer other than 'unknown' indicated
that the USM allocation was known and a memcpy could be called.

This did not work well when destination was a host device. For example,
this crashed:

```
SYCL_ENABLE_HOST_DEVICE=1 python -m pytest dpctl/tests/test_usm_ndarray_ctor.py::test_to_device
```

It passes now.
@coveralls
Copy link
Copy Markdown
Collaborator

Coverage Status

Coverage increased (+0.03%) to 74.228% when pulling 323988f on crash-in-usm_ndarray-to_device into a8f4254 on master.

@oleksandr-pavlyk oleksandr-pavlyk changed the title Reword logic in copy_from_device Rework the logic in memory's copy_from_device Oct 4, 2021
@oleksandr-pavlyk
Copy link
Copy Markdown
Contributor Author

Good to go in.

@oleksandr-pavlyk oleksandr-pavlyk merged commit 42c25df into master Oct 4, 2021
@oleksandr-pavlyk oleksandr-pavlyk deleted the crash-in-usm_ndarray-to_device branch October 4, 2021 21:46
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.

2 participants