-
Notifications
You must be signed in to change notification settings - Fork 787
[Driver][SYCL] Turn on -fsycl-dead-args-optimization by default #3004
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
Conversation
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.
The change itself LGTM. Is there something in particular that has previously been blocking the "DAE on" default before, or is it just a leftover?
Failures in Jenkins/Precommit look like real regressions and they need to be fixed before submission of the PR. |
Turn on -fsycl-dead-args-optimization by default for -fsycl compilations. Update tests to reflect the new behavior
014be95
to
5366ffa
Compare
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.
A couple suggestions & a few clarification points.
@AGindinson, I think your comments have been addressed, can you take another look? |
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.
LGTM with the current approach
@intel/llvm-reviewers-runtime, ping. |
/summary:run |
1 similar comment
/summary:run |
@bader, the failures look to be performance related. How do we proceed? |
The failures do not seems to be blocking. I suggest we commit and investigate them separately. |
* upstream/sycl: (1382 commits) [SYCL][XPTI] Report memory allocation info from SYCL runtime (intel#5172) [CI] Switch labels for OCL x64 job (intel#5185) [SYCL] Add basic support for the generic_space address space (intel#5148) [CI] Update CODEOWNERS for SYCL printf support passes (intel#5199) [SYCL][Matrix] Enable wi_slice for joint_matrix (intel#4979) [SYCL][Group algorithms] Move group sort extension to experimental (intel#5169) [SYCL] Fix kernel bundles don't really carry kernel IDs (intel#5121) [SYCL] Initial printf support for non-constant AS format strings (intel#5069) [SYCL][NFC] Fix static code analysis concerns (intel#5189) [SYCL][Doc] Fix typos to fix doc build (intel#5190) [Driver][SYCL] Turn on -fsycl-dead-args-optimization by default (intel#3004) [SYCL][L0][Plugin] Add support for batching copy commands (intel#5155) [CI] Add cache checkout script to docker containers (intel#5184) [SYCL][Doc] Add HIP backend to the filter selector (intel#5176) [Doc] Add documentation for Docker images (intel#4778) [LIBCLC] Add functionality for in-kernel asserts for CUDA backend (intel#5174) Force opt to use new pass manager in exponential-deferred-inlining test after a8c2ba1 [SYCL] Add vec and marray support to known_identity type trait (intel#5163) Correctly resolve merge conflicts Update SPV_INTEL_hw_thread_queries to rev 2 ...
Currently, when reverse translating, we hardcode either 0 or `SPIRAS_Private` as the address space for `alloca`s. This is not ideal as it creates tight coupling with the current AS mappings. Furthermore, it is not necessary, as the DataLayout holds the AllocaAS, which is what we need. Hence, this patch changes all hardcoded callsites where we create an `AllocaInst` to use the DataLayout specified AS. It's NFC as it does not modify existing behaviour. Original commit: KhronosGroup/SPIRV-LLVM-Translator@c1a7e51fb0ea6ef
Turn on -fsycl-dead-args-optimization by default for -fsycl
compilations. Update tests to reflect the new behavior