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

Add experimental extension to query mem properties of sorting kernels #127

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jandres742
Copy link

No description provided.

@jandres742
Copy link
Author

@wdamon-intel please review

@jandres742 jandres742 added documentation Improvements or additions to documentation API: Core labels May 11, 2023
Copy link

@andreyfe1 andreyfe1 left a comment

Choose a reason for hiding this comment

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

I'm ok with changes. Thanks!

desc: "[in] number of elements to process"
- type: "$x_device_group_algorithm_memory_exp_properties_t*"
name: pGroupAlgorithmMemoryProperties
desc: "[in,out] query result for group algorithm memory properties"

Choose a reason for hiding this comment

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

Minor: seems, empty line at the end is missed.

Copy link
Contributor

@wdamon-intel wdamon-intel left a comment

Choose a reason for hiding this comment

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

Overall, looks good; just a few minor questions/comments.

scripts/core/groupalgorithmmemory.yml Show resolved Hide resolved
* Functions


* ${x}DeviceGetGroupAlgorithmMemoryPropertiesExp
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it beneficial to have an explanation of this API with an example usage after the API overview section?

Copy link
Author

Choose a reason for hiding this comment

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

@andreyfe1 : Please let me know if the following text is correct:

This extension allows applications to query for memory requirements of
the underlying compiler when selecting a particular group algorithm in
their kernels. Three types of algorithms are available:

$X_GROUP_ALGORITHM_TYPE_EXP_SORT: sorting algorithm

$X__GROUP_ALGORITHM_TYPE_EXP_SCAN: scan algorithm

$X__GROUP_ALGORITHM_TYPE_EXP_REDUCE: reduce algorithm

To know what amount of memory is needed to run use algorithms,
application would call this extension as follows:


$x_device_group_algorithm_memory_exp_properties_t groupAlgorithmMemoryProperties {];
groupAlgorithmMemoryProperties.stype = $X_STRUCTURE_TYPE_DEVICE_GROUP_ALGORITHM_MEMORY_EXP_PROPERTIES;
groupAlgorithmMemoryProperties.algorithm = $X_GROUP_ALGORITHM_TYPE_EXP_SORT;
groupAlgorithmMemoryProperties.memoryScope = $X_GROUP_ALGORITHM_MEMORY_SCOPE_EXP_SUBGROUP;

$x_result_t res = $xDeviceGetGroupAlgorithmMemoryPropertiesExp(hDevice, numOfElements, &groupAlgorithmMemoryProperties);

Upon return, groupAlgorithmMemoryProperties will contain in
globalMemorySize and sharedLocalMemorySize the global and
shared local memory needed, respectively, for executing the selected
algorithm in the target device.

Copy link
Contributor

Choose a reason for hiding this comment

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

@jandres742 Is the double underscore intentional? i.e. should $X__ be $X_ ?

Copy link
Author

Choose a reason for hiding this comment

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

@wdamon-intel thanks. that's just an error. Once I write it in the scripts, I will correct it. Text in comment above is just a draft to confirm it is good.

Choose a reason for hiding this comment

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

This extension allows applications to query for memory requirements of the underlying compiler when selecting a particular group algorithm in their kernels

@jandres742, the goal for this extension is to let users know how much temporary memory they need to allocate. When they know the value, they can allocate a memory and put it as a parameter to a group algorithm that requires additional memory. So, regarding wording. Maybe we can say something like this

This extension allows applications to query for memory requirements of
the underlying compiler when selecting a particular group algorithm in
their kernels. It gives applications the memory size that is required for
memory allocations needed for group algorithms.

- type: $x_group_algorithm_memory_scope_exp_t
name: "memoryScope"
desc: "[in] Memory scope of group algorithm"
- type: size_t*
Copy link
Author

Choose a reason for hiding this comment

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

i just realized, this needs to be scalar values, as the struct is already pointer.

.. _ZE_experimental_device_group_algorithm_memory_properties:

========================================================
Group Algorithm Memory Properties Experimental Extension

Choose a reason for hiding this comment

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

are we adding some algorithms to Level Zero ?
Is there somewhere more data on the request ?

Copy link
Author

Choose a reason for hiding this comment

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

thanks @MichalMrozek . Sent email to you on this.

Jaime Arteaga added 2 commits May 12, 2023 21:08
Signed-off-by: Jaime Arteaga <jaime.a.arteaga.molina@intel.com>
Signed-off-by: Jaime Arteaga <jaime.a.arteaga.molina@intel.com>
@jandres742
Copy link
Author

@wdamon-intel , @andreyfe1 : Added content for programmers guide.

@andreyfe1
Copy link

@wdamon-intel , @andreyfe1 : Added content for programmers guide.

Looks good to me. Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API: Core documentation Improvements or additions to documentation needs discussion
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

4 participants