Skip to content

[SYCL] Implement SYCL_INTEL_mem_channel_property extension #2762

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 11 commits into from
Dec 22, 2020
Merged

[SYCL] Implement SYCL_INTEL_mem_channel_property extension #2762

merged 11 commits into from
Dec 22, 2020

Conversation

MrSidims
Copy link
Contributor

On some targets manual assignment of buffers to memory regions can
improve memory bandwidth. This extension adds a buffer property to
indicate in which memory channel a particular buffer should be allocated.
This information is an optimization hint to the runtime and thus it is
legal to ignore.

Spec:
#2688

Signed-off-by: Dmitry Sidorov dmitry.sidorov@intel.com

@MrSidims MrSidims requested review from smaslov-intel and a team as code owners November 11, 2020 17:42
@MrSidims
Copy link
Contributor Author

/summury:run

@MrSidims MrSidims marked this pull request as draft November 11, 2020 20:12
@MrSidims MrSidims force-pushed the private/MrSidims/MemChannel branch 8 times, most recently from 0860456 to d957114 Compare November 20, 2020 08:34
@MrSidims MrSidims marked this pull request as ready for review November 20, 2020 09:21
@MrSidims MrSidims requested a review from vmaksimo November 23, 2020 07:08
@MrSidims
Copy link
Contributor Author

MrSidims commented Nov 23, 2020 •
edited
Loading

@intel/llvm-reviewers-runtime @smaslov-intel please take a look

@MrSidims MrSidims force-pushed the private/MrSidims/MemChannel branch from d957114 to 799a659 Compare November 25, 2020 11:09
alexbatashev
alexbatashev previously approved these changes Nov 25, 2020
View reviewed changes
Copy link
Contributor

@alexbatashev alexbatashev left a comment

Choose a reason for hiding this comment

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

LGTM

@MrSidims
Copy link
Contributor Author

@smaslov-intel could you please take a look?

@MrSidims
Copy link
Contributor Author

@smaslov-intel friendly ping

smaslov-intel
smaslov-intel suggested changes Nov 30, 2020
View reviewed changes
Copy link
Contributor

@smaslov-intel smaslov-intel left a comment

Choose a reason for hiding this comment

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

Please add support in Level-Zero plugin too

@MrSidims
[SYCL] Implement SYCL_INTEL_mem_channel_property extension
aff5fee 
On some targets manual assignment of buffers to memory regions can
improve memory bandwidth. This extension adds a buffer property to
indicate in which memory channel a particular buffer should be allocated.
This information is an optimization hint to the runtime and thus it is
legal to ignore.

Spec:
intel#2688

Signed-off-by: Dmitry Sidorov <dmitry.sidorov@intel.com>
@MrSidims
Make properties enum numbering be explicit
9d888a1 
Signed-off-by: Dmitry Sidorov <dmitry.sidorov@intel.com>
@s-kanaev
Copy link
Contributor

@MrSidims, friendly ping.

@MrSidims
Copy link
Contributor Author

@MrSidims, friendly ping.

Sorry, for the delay, now I returned to this one.

@MrSidims
Apply most of the comments
e876fd1 
Signed-off-by: Dmitry Sidorov <dmitry.sidorov@intel.com>
@MrSidims MrSidims force-pushed the private/MrSidims/MemChannel branch from 799a659 to e876fd1 Compare December 14, 2020 12:54
@MrSidims
Copy link
Contributor Author

Thanks for valuable comments and sorry for a delay - had to switch to a more urgent task for a while.

Please add support in Level-Zero plugin too

The only comment that isn't yet applied. @smaslov-intel is your suggestion is to pass properties via ze_device_mem_alloc_desc_t in zeMemAllocDevice? In this case, I think, we need an extension of level zero for that to describe this descriptor. Otherwise, I don't think level-zero has API for that. OpenCL 3.0 adds clCreateBufferWithProperties function to replace clCreateBuffer, which is being re-used by Intel extension.

@MrSidims MrSidims dismissed stale reviews from s-kanaev and alexbatashev via 4a3e3d4 December 16, 2020 14:15
@MrSidims
One more
541a905 
Signed-off-by: Dmitry Sidorov <dmitry.sidorov@intel.com>
alexbatashev
alexbatashev previously approved these changes Dec 16, 2020
View reviewed changes
s-kanaev
s-kanaev previously approved these changes Dec 17, 2020
View reviewed changes
smaslov-intel
smaslov-intel suggested changes Dec 17, 2020
View reviewed changes
@smaslov-intel
Copy link
Contributor

Please add support in Level-Zero plugin too

The only comment that isn't yet applied.

That's OK, we should clarify if Level-Zero API may support this and add it separately.

@MrSidims
Set -> Map
8f78d1f 
Signed-off-by: Dmitry Sidorov <dmitry.sidorov@intel.com>
@MrSidims MrSidims dismissed stale reviews from s-kanaev and alexbatashev via 8f78d1f December 21, 2020 11:52
alexbatashev
alexbatashev previously approved these changes Dec 21, 2020
View reviewed changes
Copy link
Contributor

@alexbatashev alexbatashev left a comment

Choose a reason for hiding this comment

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

Still LGTM

romanovvlad
romanovvlad requested changes Dec 21, 2020
View reviewed changes
@MrSidims
Apply several comments
4b71b32 
Signed-off-by: Dmitry Sidorov <dmitry.sidorov@intel.com>
@MrSidims
Remove properties check
1f1135b 
Signed-off-by: Dmitry Sidorov <dmitry.sidorov@intel.com>
romanovvlad
romanovvlad previously approved these changes Dec 22, 2020
View reviewed changes
smaslov-intel
smaslov-intel previously approved these changes Dec 22, 2020
View reviewed changes
@MrSidims MrSidims dismissed stale reviews from smaslov-intel and romanovvlad via cdec00a December 22, 2020 16:54
@MrSidims
Fix merge errors
e29fe01 
Signed-off-by: Dmitry Sidorov <dmitry.sidorov@intel.com>
@romanovvlad romanovvlad merged commit 2f1f316 into intel:sycl Dec 22, 2020
jsji pushed a commit that referenced this pull request Nov 7, 2024
@MrSidims @jsji
Add fast math flag translation for OpenCL std lib (#2762)
bd2ac52 
Such possibility was added in SPIR-V 1.6.
This patch also introduces limited translation of nofpclass LLVM parameter attribute.

Signed-off-by: Sidorov, Dmitry <dmitry.sidorov@intel.com>

Original commit:
KhronosGroup/SPIRV-LLVM-Translator@ae8fa3825a699b2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@smaslov-intel smaslov-intel smaslov-intel approved these changes

@alexbatashev alexbatashev alexbatashev left review comments

@romanovvlad romanovvlad romanovvlad left review comments

@s-kanaev s-kanaev s-kanaev left review comments

@vmaksimo vmaksimo Awaiting requested review from vmaksimo

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

Successfully merging this pull request may close these issues.
5 participants
@MrSidims @s-kanaev @smaslov-intel @alexbatashev @romanovvlad