Fix compiler warnings for 0.99.3 release - #472
Conversation
pelesh
left a comment
pelesh
left a comment
There was a problem hiding this comment.
Great job, thanks!
I think there is a better way to do conversions from Re::Solves index_type to native HIP/CUDA index types but this is beyond the scope of this PR.
I left suggestions how type casts could be done more elegantly but haven't. test them myself. You may want to check that quickly.
| const int block_size = 256; | ||
| int num_blocks = (n + block_size - 1) / block_size; | ||
| // Launch the kernel | ||
| kernels::scale<<<num_blocks, block_size>>>(n, diag, vec); | ||
| kernels::scale<<<static_cast<unsigned int>(num_blocks), static_cast<unsigned int>(block_size)>>>(n, diag, vec); |
There was a problem hiding this comment.
I think a better solution here would be to define
const size_t block_size = 256;
const size_t num_blocks = (n + block_size - 1) / block_size;That would eliminate the need for static_casts.
Similar to the subsequent instances.
| index_type num_blocks; | ||
| index_type block_size = 512; | ||
| num_blocks = (n + block_size - 1) / block_size; |
There was a problem hiding this comment.
I would define num_blocks and block_size as const size_t type. That would eliminate the need for static casts.
shakedregev
left a comment
shakedregev
left a comment
There was a problem hiding this comment.
Tested, this works.
fe41abd to
0312f81
Compare
|
Applied the review suggestions and retested. Everything still passes. |
3 participants
Description
Cleans up compiler warnings found during 0.99.3 release testing.
@pelesh
Proposed changes
sysRefactor.Checklist
make testandmake test_installper testing instructions). Code tested on./examples/<your_example>.exe -hto get instructions how to run examples). Code tested on:-Wall -Wpedantic -Wconversion -Wextra.Further comments
CPU and HIP compile cleanly with
-Wall -Wpedantic -Wconversion -Wextra.CUDA still reports the expected cuSOLVER/cuSolverRF deprecation warnings recommending cuDSS. There are also deprecation warnings from the cuSPARSE
csrilu02ILU0 interface. Removing those would require a larger API update, so I left them unchanged for now. Let me know if you would like me to include that cleanup before the release.