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

gh-886: Added 3 new device attributes and kernel's device-specific attributes #894

Merged
merged 11 commits into from
Sep 4, 2022

Conversation

oleksandr-pavlyk
Copy link
Collaborator

Closes #886

These are DPCTLDevice_GetGlobalMemCacheSize, DPCTLDevice_GlobalMemCacheLineSize,
and DPCTLDevice_GetGlobalMemCacheType.

To support the latter, introduced DPCTLGlobalMemCacheType enum in "dpctl_sycl_enum_types.h".

Tests are added to test_capi target.

  • Have you provided a meaningful PR description?
  • Have you added a test, reproducer or referred to an issue with a reproducer?
  • Have you tested your changes locally for CPU and GPU devices?
  • Have you made sure that new changes do not introduce compiler warnings?
  • If this PR is a work in progress, are you filing the PR as a draft?

@github-actions
Copy link

@oleksandr-pavlyk oleksandr-pavlyk force-pushed the gh-886-more-device-attributes branch 3 times, most recently from 103e9ee to f6369f2 Compare August 26, 2022 20:12
@coveralls
Copy link
Collaborator

coveralls commented Aug 26, 2022

Coverage Status

Coverage decreased (-0.01%) to 81.829% when pulling 706237c on gh-886-more-device-attributes into 8655e98 on master.

@oleksandr-pavlyk oleksandr-pavlyk mentioned this pull request Aug 27, 2022
1 task
These are DPCTLDevice_GetGlobalMemCacheSize, DPCTLDevice_GlobalMemCacheLineSize,
and DPCTLDevice_GetGlobalMemCacheType.

To support the latter, introduced DPCTLGlobalMemCacheType enum in dpctl_sycl_enum_types.h

Tests are added to test_capi target.
  * dpctl.SyclDevice.global_mem_cache_size
  * dpctl.SyclDevice.global_mem_cache_line_size
  * dpctl.SyclDevice.global_mem_cache_type

The last property output is a new enum dpctl.global_mem_cache_type
which can assume 3 values: none, read_only, and read_write
@oleksandr-pavlyk oleksandr-pavlyk marked this pull request as ready for review August 30, 2022 14:56
@oleksandr-pavlyk oleksandr-pavlyk changed the title Added 3 new device attributes per gh-886 gh-886: Added 3 new device attributes and kernel's device-specific attributes Sep 3, 2022
int: Cache size in bytes
"""
cdef uint64_t cache_sz = DPCTLDevice_GetGlobalMemCacheSize(
self._device_ref)
Copy link
Collaborator

Choose a reason for hiding this comment

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

close paren should be on next line.

int: Cache size in bytes
"""
cdef uint64_t cache_line_sz = DPCTLDevice_GetGlobalMemCacheLineSize(
self._device_ref)
Copy link
Collaborator

Choose a reason for hiding this comment

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

same formatting change here

a multiple, for executing the kernel on the device it was built for.
"""
cdef size_t v = DPCTLKernel_GetPreferredWorkGroupSizeMultiple(
self._kernel_ref)
Copy link
Collaborator

Choose a reason for hiding this comment

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

formatting

Copy link
Collaborator

@diptorupd diptorupd Sep 4, 2022

Choose a reason for hiding this comment

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

Also the explanation reads a bit awkward. How about:

Returns a number whose multiple is the preferred work-group size for the specified kernel on the selected device.

*
* @param KRef DPCTLSyclKernelRef pointer to an OpenCL
* @param KRef DPCTLSyclKernelRef pointer to an SYCL
Copy link
Collaborator

@diptorupd diptorupd Sep 4, 2022

Choose a reason for hiding this comment

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

typo: to a SYCL

*
* @param KRef DPCTLSyclKernelRef pointer to an OpenCL
* @param KRef DPCTLSyclKernelRef pointer to an SYCL
Copy link
Collaborator

Choose a reason for hiding this comment

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

same typo

*
* @param KRef DPCTLSyclKernelRef pointer to an SYCL
* interoperability kernel.
* @return Returns a value, of which work-group size is preferred to be a
Copy link
Collaborator

Choose a reason for hiding this comment

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

My reuse the docstring from the Python function corresponding to the C API function.

diptorupd
diptorupd previously approved these changes Sep 4, 2022
Copy link
Collaborator

@diptorupd diptorupd left a comment

Choose a reason for hiding this comment

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

Thank! Please fix the minor typo etc. and merge.

@oleksandr-pavlyk oleksandr-pavlyk merged commit f48cf30 into master Sep 4, 2022
@oleksandr-pavlyk oleksandr-pavlyk deleted the gh-886-more-device-attributes branch September 4, 2022 13:25
@github-actions
Copy link

github-actions bot commented Sep 4, 2022

Deleted rendered PR docs from intelpython.github.com/dpctl, latest should be updated shortly. 🤞

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.

Request for additonal attribute preferred_work_group_size_multiple
3 participants