-
Notifications
You must be signed in to change notification settings - Fork 768
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
[CUDA] bindless_images tests failed after f9c8c01d38f8fb #16503
Comments
FYI. @intel/bindless-images-reviewers |
Thanks for reporting this, we're tracking this internally. |
See here
In all the above cases, the sycl compilation is failing to interact correctly with the new pass that has been switched on for all cuda targets: https://github.com/intel/llvm/blob/f9c8c01d38f8fbea81db99ab90b7d0f2bdcc8b4d/llvm/lib/Target/NVPTX/NVPTXReplaceImageHandles.cpp This is because the sycl compilation is preventing this switch statement from finding a valid instruction
I am not sure of the best way to resolve this, but it will probably involve considering the place of the NVPTXReplaceImageHandles.cpp pass in relation to other SYCL specific compilation processes, specifically dealing with the SYCL specific |
…nctionality (#17045) This separates out bindless-image tests into more unit-test like tests for orthogonal functionality. This reduces the number of tests that fail in #16503 when the upstream commit, f9c8c01 , is pulled down, from 7 to three. This also demonstrates that essentially all bindless-image functionality that maps directly to cuda (those using nd range or USM) work correctly even with the upstream pulldown, whereas some functionality that is SYCL specific (range parallel_for or buffers) fail. Complete information is described in the associated issue #16503 (comment) All the tests pass with the current DPC++ tip, but three of the tests fail when the above mentioned upstream commit is pulled down. - read_write_1D_buffer.cpp - read_write_1D_range.cpp - examples/example_2_2D_dynamic_read.cpp Additionally this PR removes the duplicate read_write_1D.cpp and read_2D_dynamic.cpp tests which match almost identically with the corresponding named tests in the examples folder. This is done to reduce unnecessary maintenance overhead. --------- Signed-off-by: JackAKirk <[email protected]>
…nctionality (intel#17045) This separates out bindless-image tests into more unit-test like tests for orthogonal functionality. This reduces the number of tests that fail in intel#16503 when the upstream commit, intel@f9c8c01 , is pulled down, from 7 to three. This also demonstrates that essentially all bindless-image functionality that maps directly to cuda (those using nd range or USM) work correctly even with the upstream pulldown, whereas some functionality that is SYCL specific (range parallel_for or buffers) fail. Complete information is described in the associated issue intel#16503 (comment) All the tests pass with the current DPC++ tip, but three of the tests fail when the above mentioned upstream commit is pulled down. - read_write_1D_buffer.cpp - read_write_1D_range.cpp - examples/example_2_2D_dynamic_read.cpp Additionally this PR removes the duplicate read_write_1D.cpp and read_2D_dynamic.cpp tests which match almost identically with the corresponding named tests in the examples folder. This is done to reduce unnecessary maintenance overhead. --------- Signed-off-by: JackAKirk <[email protected]>
The root cause of this was that the pass that was switched on regardless of compute capability in the offending upstream commit did not support all valid NVPTX 64 bit address instructions that could be used to store an image handle.
These failures have already been fixed via the following upstream patches (which have not yet been pulled down into intel/llvm):
@jsji in order to fix this in intel/llvm I guess that f9c8c01 needs to be cherry-picked before the next pulldown. I guess that you can either do this separately within a separate PR, or within the PR for the next pulldown (but prior to the later commits). If you wish to cherry-pick the offending commit separately, then you can use the following temporary fix that will allow all bindless images tests to pass:
That you can revert that once the pulldown is made. Does that make sense? |
Describe the bug
With the new optimization in llvm/llvm-project#119730
[NVPTX] Aggressively try to replace image handles with references
The following test failed for CUDA.
f9c8c01 is reverted temporaily to unblock the pulldown, please investigate/fix and reland.
See details in
https://github.com/intel/llvm/actions/runs/12563446905/job/35025604202
The text was updated successfully, but these errors were encountered: