Skip to content

Add device descriptors: sub_group_independent_forward_progress and preferred_vector_width #308

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 9 commits into from
Mar 25, 2021
Merged

Add device descriptors: sub_group_independent_forward_progress and preferred_vector_width #308

merged 9 commits into from
Mar 25, 2021

Conversation

1e-to
Copy link
Contributor

@1e-to 1e-to commented Mar 2, 2021 •
edited
Loading

Add sub_group_independent_forward_progress and preferred_vector_width_char
Closes #314 , the part of #286

@1e-to 1e-to linked an issue Mar 10, 2021 that may be closed by this pull request
Add device descriptors: sub_group_independent_forward_progress and preferred_vector_width #314
Closed
8 tasks
@1e-to
Copy link
Contributor Author

1e-to commented Mar 10, 2021

@diptorupd, @oleksandr-pavlyk could you please take a look?

@1e-to 1e-to changed the title Device descriptor Add device descriptors: sub_group_independent_forward_progress and preferred_vector_width Mar 10, 2021
@diptorupd
Copy link
Contributor

@1e-to Looks good. @oleksandr-pavlyk once we have the types properly supported we should have a single get_vector_width call and pass in a dtype to lookup.

@oleksandr-pavlyk
Copy link
Contributor

Can I suggest to not save properties in SyclDevice instance? The get_info call is not expensive, and Python property should just be making it. It is cleaner, I think.

@oleksandr-pavlyk
Copy link
Contributor

For now this looks good. Feel free to merge. This will cause conflicts with feature/queue_manager_v2 but I can resolve them.

@diptorupd
Copy link
Contributor

diptorupd commented Mar 13, 2021 •
edited
Loading

Can I suggest to not save properties in SyclDevice instance? The get_info call is not expensive, and Python property should just be making it. It is cleaner, I think.

I think it is better to make the change now before we add even more get_info wrappers. @1e-to please change these new attributes into properties.

I have created a PR #324 to change the existing attributes to properties.

Let us keep these attributes as is, since they involve allocating a pointer:

  • cdef const char *_vendor_name
  • cdef const char *_device_name
  • cdef const char *_driver_version
  • cdef size_t *_max_work_item_sizes

Also, rebase with master, as I have merged @oleksandr-pavlyk 's changes to SyclDevice in #321.

@diptorupd diptorupd mentioned this pull request Mar 15, 2021
Closed
8 tasks
@diptorupd
Copy link
Contributor

@1e-to can you rebase this with master and then change the attributes you added to SyclDevice class to properties.

diptorupd
diptorupd reviewed Mar 25, 2021
View reviewed changes
dpctl-capi/tests/test_sycl_device_interface.cpp
GTEST_SKIP_("Device not found");
EXPECT_NO_FATAL_FAILURE(
vector_width_double = DPCTLDevice_GetPreferredVectorWidthDouble(DRef));
if (DPCTLDevice_HasAspect(DRef, DPCTL_SyclAspectToDPCTLAspectType(
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice! You have been reading the spec a lot :)

diptorupd
diptorupd approved these changes Mar 25, 2021
View reviewed changes
Copy link
Contributor

@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.

Looks really good. Thanks!

oleksandr-pavlyk
oleksandr-pavlyk approved these changes Mar 25, 2021
View reviewed changes
Copy link
Contributor

@oleksandr-pavlyk oleksandr-pavlyk left a comment

Choose a reason for hiding this comment

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

Thanks @1e-to !

@diptorupd diptorupd merged commit 75aa0c1 into IntelPython:master Mar 25, 2021
diptorupd pushed a commit that referenced this pull request Apr 2, 2021
@1e-to
Add device descriptors: sub_group_independent_forward_progress and pr…
0a22e43 
…eferred_vector_width (#308)

* Add sub_group_independent_forward_progress and preferred_vector_width_char

* Add tests

* Add python api

* Fixes + tests

* Add preferred_vector_width funcs

* Added property

Co-authored-by: etotmeni <elena.totmenina@intel.com>
@1e-to 1e-to deleted the device_descriptor branch November 3, 2021 09:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@diptorupd diptorupd diptorupd approved these changes

@oleksandr-pavlyk oleksandr-pavlyk oleksandr-pavlyk approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.7.0
Development

Successfully merging this pull request may close these issues.

Add device descriptors: sub_group_independent_forward_progress and preferred_vector_width
4 participants
@1e-to @diptorupd @oleksandr-pavlyk @PokhodenkoSA