Skip to content

[SYCL] Implement SYCL_INTEL_mem_channel_property extension#2762

Merged
romanovvlad merged 11 commits intointel:syclfrom
MrSidims:private/MrSidims/MemChannel
Dec 22, 2020
Merged

[SYCL] Implement SYCL_INTEL_mem_channel_property extension#2762
romanovvlad merged 11 commits intointel:syclfrom
MrSidims:private/MrSidims/MemChannel

Conversation

@MrSidims
Copy link
Contributor

@MrSidims MrSidims commented Nov 11, 2020

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 a team and smaslov-intel as code owners November 11, 2020 17:42
@MrSidims
Copy link
Contributor Author

MrSidims commented Nov 11, 2020

/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

alexbatashev
alexbatashev suggested changes Nov 23, 2020
View reviewed changes
@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

MrSidims commented Nov 27, 2020

@smaslov-intel could you please take a look?

@MrSidims
Copy link
Contributor Author

MrSidims commented Nov 30, 2020

@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

Dmitry Sidorov added 2 commits December 10, 2020 13:26
[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>
Make properties enum numbering be explicit
9d888a1 
Signed-off-by: Dmitry Sidorov <dmitry.sidorov@intel.com>
@s-kanaev
Copy link
Contributor

s-kanaev commented Dec 10, 2020

@MrSidims, friendly ping.

@MrSidims
Copy link
Contributor Author

MrSidims commented Dec 11, 2020

@MrSidims, friendly ping.

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

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

MrSidims commented Dec 14, 2020

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

smaslov-intel commented Dec 17, 2020

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.

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

smaslov-intel
smaslov-intel reviewed Dec 21, 2020
View reviewed changes
romanovvlad
romanovvlad suggested changes Dec 21, 2020
View reviewed changes
romanovvlad
romanovvlad reviewed Dec 21, 2020
View reviewed changes
Dmitry Sidorov added 2 commits December 22, 2020 11:46
Apply several comments
4b71b32 
Signed-off-by: Dmitry Sidorov <dmitry.sidorov@intel.com>
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
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
@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

@vmaksimo vmaksimo Awaiting requested review from vmaksimo

4 more 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

Reviewers whose approvals may not affect merge requirements

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