Fix for CUDA codegen #1442
Merged
Fix for CUDA codegen #1442
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Argument to std::ifloor should be double, otherwise invalid result on gpu target.
Use new template for dace::SharedToGlobal1D
After uplift to dace v0.15, one SDFG which was working before started to show compilation errors. The latest DaCe is moving a data access to an inter-state edge. For the data-access, the symbols that define array strides are needed for code generation. The SDFG was validated, before and after the simplify pass, but it did not compile for CPU. When skipping the simplify pass, the compilation did work. The problem has been narrowed down to the scalar-to-symbol promotion, which is moving a data access to an inter-state edge. Then, the method _used_symbols_internal needs to be update to account for data containers, including symbolic shape and strides. This commit contains a unit test to reproduce the issue and verify the proposed fix.
Keep new logic, fix cuda codegen for 1D shared-to-global
tbennun
requested changes
Nov 30, 2023
edopao
commented
Nov 30, 2023
edopao
force-pushed
the
bug-gpu-codegen
branch
from
6d75df6
to
12b3cdd
Compare
|
@tbennun Test added, please re-review. |
tbennun
requested changes
Dec 12, 2023
dace/codegen/targets/cuda.py
Outdated
| @@ -1132,10 +1132,22 @@ def _emit_copy(self, state_id, src_node, src_storage, dst_node, dst_storage, dst | |||
| func=funcname, | |||
| type=dst_node.desc(sdfg).dtype.ctype, | |||
| bdims=', '.join(_topy(self._block_dims)), | |||
| is_async='true' if state_dfg.out_degree(dst_node) > 0 else 'true', | |||
| is_async='true' if state_dfg.out_degree(dst_node) > 0 else 'false', | |||
There was a problem hiding this comment.
It should be the other way around (if there is a dependent read after it in the same state, sync).
There was a problem hiding this comment.
Correct. I did not pay enough attention to is_async before. Besides correcting the value of this argument, I have also moved the synchronization point in the template function after the thread-level copy (see my last commit on copy.cuh).
|
@tbennun Thank you for the review. As I commented above, I have done one additional change related to |
|
Gentle reminder for @tbennun: please check my last comment and whether you can approve this PR. |
tbennun
approved these changes
Dec 18, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR addresses #1388: fix python codegen and
SharedToGlobal1Dtemplate to generate correct code for write without reduction.