-
Notifications
You must be signed in to change notification settings - Fork 1.1k
[rls-v3.12] build: exclude Xe3p from default DNNL_ENABLE_PRIMITIVE_GPU_ISA #5105
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
base: rls-v3.12
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,7 +21,7 @@ endif() | |
|
|
||
| # Use oneDNN names for ALL to ensure string replacement functions correctly | ||
| set(GPUS ${DNNL_ENABLE_PRIMITIVE_GPU_ISA}) | ||
| string(REPLACE "ALL" "XELP;XEHP;XEHPG;XEHPC;XE2;XE3;XE3P" GPUS "${GPUS}") | ||
| string(REPLACE "ALL" "XELP;XEHP;XEHPG;XEHPC;XE2;XE3" GPUS "${GPUS}") | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it be reasonable to use a runtime guard (via environment variable) rather than compile time? This way we can still use the same build for testing purpose.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. From my perspective runtime opt-in is the best choice. It would be challenging to design and implement it in time for v3.12 RTM.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What we can do before the RTM is opt-in via
@vpirogov @rjoursler Do you see issues with any of the bullets? Are frameworks fine with environment variable control for opt-in (if they still want to enable Xe3p platforms)?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would rather focus on developing proper guard consistent with the way we handle environment variables for v3.13. Build time opt-in looks adequate for v3.12. |
||
| string(REPLACE "XELP" "12LP" GPUS "${GPUS}") | ||
| string(REPLACE "XEHPG" "12p7" GPUS "${GPUS}") | ||
| string(REPLACE "XEHPC" "12p8" GPUS "${GPUS}") | ||
|
|
@@ -63,14 +63,16 @@ if(DPCPP_HOST_COMPILER_KIND STREQUAL "DEFAULT") | |
| ${ONEDNN_GEMMSTONE_DIR}/generator/generator.cpp | ||
| ) | ||
|
|
||
| if (DNNL_ENABLE_PRIMITIVE_GPU_ISA STREQUAL "ALL") | ||
| set(DNNL_GPU_ISA_LIST "XELP;XEHP;XEHPG;XEHPC;XE2;XE3;XE3P") | ||
| else() | ||
| foreach(isa ${DNNL_ENABLE_PRIMITIVE_GPU_ISA}) | ||
| string(TOUPPER ${isa} ISA) | ||
| set(DNNL_GPU_ISA_LIST "${DNNL_GPU_ISA_LIST};${ISA}") | ||
| endforeach() | ||
| endif() | ||
| set(DNNL_GPU_ISA_LIST "") | ||
| foreach(isa ${DNNL_ENABLE_PRIMITIVE_GPU_ISA}) | ||
| string(TOUPPER ${isa} ISA) | ||
| if("${ISA}" STREQUAL "ALL") | ||
| list(APPEND DNNL_GPU_ISA_LIST XELP XEHP XEHPG XEHPC XE2 XE3) | ||
| else() | ||
| list(APPEND DNNL_GPU_ISA_LIST ${ISA}) | ||
| endif() | ||
| endforeach() | ||
| list(REMOVE_DUPLICATES DNNL_GPU_ISA_LIST) | ||
|
|
||
| foreach(isa ${DNNL_GPU_ISA_LIST}) | ||
| if(${isa} STREQUAL "XE3P") | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need any gating on OpenCL implementations? For the most part, I would expect those to just work, but there are some workarounds for driver issues.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't have any control on driver version at the user end, so I'd say no.