cutlass icon indicating copy to clipboard operation
cutlass copied to clipboard
verified publisher icon NVIDIA

Fix several integer-signedness warnings

Open iskunk opened this issue 1 year ago • 16 comments

I am building cutlass 3.4.1 using the Intel LLVM (oneAPI) compiler. There are a lot of warnings being produced related to implicit integer sign conversions.

This PR does not fix all of them, but it addresses all the instances that individually result in over a thousand warnings. Test suite results are left no worse off, but please review these changes carefully.

Here is the bulk of the warnings removed from the build output, sorted in order of decreasing count:

   2158 .../cutlass-3.4.1/include/cutlass/fast_math.h:249:8: warning: implicit conversion changes signedness: 'int' to 'unsigned int' [-Wsign-conversion]
   2158 .../cutlass-3.4.1/include/cutlass/fast_math.h:368:17: warning: implicit conversion changes signedness: 'int' to 'unsigned int' [-Wsign-conversion]
   2132 .../cutlass-3.4.1/include/cute/numeric/math.hpp:154:49: warning: implicit conversion changes signedness: 'int' to 'unsigned int' [-Wsign-conversion]
   1813 .../cutlass-3.4.1/include/cutlass/arch/mma_sm60.h:134:27: warning: implicit conversion changes signedness: 'int' to 'size_type' (aka 'unsigned long') [-Wsign-conversion]
   1813 .../cutlass-3.4.1/include/cutlass/arch/mma_sm60.h:134:38: warning: implicit conversion changes signedness: 'int' to 'size_type' (aka 'unsigned long') [-Wsign-conversion]
   1813 .../cutlass-3.4.1/include/cutlass/arch/mma_sm60.h:134:5: warning: implicit conversion changes signedness: 'int' to 'size_type' (aka 'unsigned long') [-Wsign-conversion]
   1813 .../cutlass-3.4.1/include/cutlass/arch/mma_sm60.h:88:18: warning: implicit conversion changes signedness: 'int' to 'size_type' (aka 'unsigned long') [-Wsign-conversion]
   1813 .../cutlass-3.4.1/include/cutlass/arch/mma_sm60.h:88:38: warning: implicit conversion changes signedness: 'int' to 'size_type' (aka 'unsigned long') [-Wsign-conversion]
   1813 .../cutlass-3.4.1/include/cutlass/arch/mma_sm60.h:88:5: warning: implicit conversion changes signedness: 'int' to 'size_type' (aka 'unsigned long') [-Wsign-conversion]
   1809 .../cutlass-3.4.1/include/cutlass/arch/mma_sm60.h:188:28: warning: implicit conversion changes signedness: 'int' to 'size_type' (aka 'unsigned long') [-Wsign-conversion]
   1809 .../cutlass-3.4.1/include/cutlass/arch/mma_sm60.h:188:37: warning: implicit conversion changes signedness: 'int' to 'size_type' (aka 'unsigned long') [-Wsign-conversion]
   1809 .../cutlass-3.4.1/include/cutlass/arch/mma_sm60.h:188:50: warning: implicit conversion changes signedness: 'int' to 'size_type' (aka 'unsigned long') [-Wsign-conversion]
   1809 .../cutlass-3.4.1/include/cutlass/arch/mma_sm60.h:188:7: warning: implicit conversion changes signedness: 'int' to 'size_type' (aka 'unsigned long') [-Wsign-conversion]
   1809 .../cutlass-3.4.1/include/cutlass/arch/mma_sm60.h:242:13: warning: implicit conversion changes signedness: 'int' to 'size_type' (aka 'unsigned long') [-Wsign-conversion]
   1809 .../cutlass-3.4.1/include/cutlass/arch/mma_sm60.h:242:28: warning: implicit conversion changes signedness: 'int' to 'size_type' (aka 'unsigned long') [-Wsign-conversion]
   1809 .../cutlass-3.4.1/include/cutlass/arch/mma_sm60.h:242:37: warning: implicit conversion changes signedness: 'int' to 'size_type' (aka 'unsigned long') [-Wsign-conversion]
   1809 .../cutlass-3.4.1/include/cutlass/arch/mma_sm60.h:242:56: warning: implicit conversion changes signedness: 'int' to 'size_type' (aka 'unsigned long') [-Wsign-conversion]
   1809 .../cutlass-3.4.1/include/cutlass/arch/mma_sm61.h:133:13: warning: implicit conversion changes signedness: 'int' to 'size_type' (aka 'unsigned long') [-Wsign-conversion]
   1809 .../cutlass-3.4.1/include/cutlass/arch/mma_sm61.h:133:22: warning: implicit conversion changes signedness: 'int' to 'size_type' (aka 'unsigned long') [-Wsign-conversion]
   1809 .../cutlass-3.4.1/include/cutlass/arch/mma_sm61.h:86:13: warning: implicit conversion changes signedness: 'int' to 'size_type' (aka 'unsigned long') [-Wsign-conversion]
   1809 .../cutlass-3.4.1/include/cutlass/arch/mma_sm61.h:86:22: warning: implicit conversion changes signedness: 'int' to 'size_type' (aka 'unsigned long') [-Wsign-conversion]
   1788 .../cutlass-3.4.1/test/unit/gemm/device/testbed.h:158:60: warning: implicit conversion changes signedness: '::size_t' (aka 'unsigned long') to '::int64_t' (aka 'long') [-Wsign-conversion]
   1763 .../cutlass-3.4.1/include/cutlass/predicate_vector.h:162:9: warning: implicit conversion changes signedness: 'int' to 'unsigned long' [-Wsign-conversion]
   1763 .../cutlass-3.4.1/include/cutlass/predicate_vector.h:163:19: warning: implicit conversion changes signedness: 'int' to 'unsigned long' [-Wsign-conversion]
   1761 .../cutlass-3.4.1/include/cutlass/gemm/warp/mma_tensor_op.h:103:45: warning: implicit conversion changes signedness: 'unsigned int' to 'int' [-Wsign-conversion]
   1761 .../cutlass-3.4.1/include/cutlass/gemm/warp/mma_tensor_op.h:103:48: warning: implicit conversion changes signedness: 'int' to 'unsigned int' [-Wsign-conversion]
   1761 .../cutlass-3.4.1/include/cutlass/gemm/warp/mma_tensor_op.h:124:45: warning: implicit conversion changes signedness: 'unsigned int' to 'int' [-Wsign-conversion]
   1761 .../cutlass-3.4.1/include/cutlass/gemm/warp/mma_tensor_op.h:124:48: warning: implicit conversion changes signedness: 'int' to 'unsigned int' [-Wsign-conversion]
   1761 .../cutlass-3.4.1/include/cutlass/layout/permute.h:321:58: warning: implicit conversion changes signedness: 'const unsigned int' to 'Index' (aka 'int') [-Wsign-conversion]
   1761 .../cutlass-3.4.1/include/cutlass/layout/permute.h:384:58: warning: implicit conversion changes signedness: 'const unsigned int' to 'Index' (aka 'int') [-Wsign-conversion]
   1761 .../cutlass-3.4.1/include/cutlass/layout/permute.h:456:58: warning: implicit conversion changes signedness: 'const unsigned int' to 'Index' (aka 'int') [-Wsign-conversion]
   1761 .../cutlass-3.4.1/include/cutlass/layout/permute.h:517:58: warning: implicit conversion changes signedness: 'const unsigned int' to 'Index' (aka 'int') [-Wsign-conversion]
   1737 .../cutlass-3.4.1/include/cutlass/epilogue/warp/tile_iterator_tensor_op_mixed.h:405:21: warning: implicit conversion changes signedness: 'Index' (aka 'long') to 'size_t' (aka 'unsigned long') [-Wsign-conversion]
   1737 .../cutlass-3.4.1/include/cutlass/epilogue/warp/tile_iterator_tensor_op_mixed.h:606:21: warning: implicit conversion changes signedness: 'Index' (aka 'long') to 'size_t' (aka 'unsigned long') [-Wsign-conversion]
   1735 .../cutlass-3.4.1/include/cutlass/gemm/threadblock/threadblock_swizzle_streamk.h:450:131: warning: implicit conversion changes signedness: 'Index' (aka 'int') to 'size_t' (aka 'unsigned long') [-Wsign-conversion]
   1735 .../cutlass-3.4.1/include/cutlass/gemm/threadblock/threadblock_swizzle_streamk.h:450:155: warning: implicit conversion changes signedness: 'Index' (aka 'int') to 'size_t' (aka 'unsigned long') [-Wsign-conversion]
   1735 .../cutlass-3.4.1/include/cutlass/gemm/threadblock/threadblock_swizzle_streamk.h:450:201: warning: implicit conversion changes signedness: 'Index' (aka 'int') to 'size_t' (aka 'unsigned long') [-Wsign-conversion]
   1735 .../cutlass-3.4.1/include/cutlass/gemm/threadblock/threadblock_swizzle_streamk.h:450:225: warning: implicit conversion changes signedness: 'Index' (aka 'int') to 'size_t' (aka 'unsigned long') [-Wsign-conversion]
   1735 .../cutlass-3.4.1/include/cutlass/gemm/threadblock/threadblock_swizzle_streamk.h:450:62: warning: implicit conversion changes signedness: 'Index' (aka 'int') to 'size_t' (aka 'unsigned long') [-Wsign-conversion]
   1735 .../cutlass-3.4.1/include/cutlass/gemm/threadblock/threadblock_swizzle_streamk.h:450:86: warning: implicit conversion changes signedness: 'Index' (aka 'int') to 'size_t' (aka 'unsigned long') [-Wsign-conversion]
   1705 .../cutlass-3.4.1/include/cutlass/gemm/threadblock/threadblock_swizzle.h:112:31: warning: implicit conversion changes signedness: 'int' to 'unsigned int' [-Wsign-conversion]
   1705 .../cutlass-3.4.1/include/cutlass/gemm/threadblock/threadblock_swizzle.h:112:70: warning: implicit conversion changes signedness: 'int' to 'unsigned int' [-Wsign-conversion]
   1705 .../cutlass-3.4.1/include/cutlass/gemm/threadblock/threadblock_swizzle.h:112:90: warning: implicit conversion changes signedness: 'Index' (aka 'int') to 'unsigned int' [-Wsign-conversion]
   1631 .../cutlass-3.4.1/include/cutlass/gemm/threadblock/threadblock_swizzle.h:112:29: warning: implicit conversion changes signedness: 'int' to 'unsigned int' [-Wsign-conversion]
   1631 .../cutlass-3.4.1/include/cutlass/gemm/threadblock/threadblock_swizzle.h:112:68: warning: implicit conversion changes signedness: 'int' to 'unsigned int' [-Wsign-conversion]
   1631 .../cutlass-3.4.1/include/cutlass/gemm/threadblock/threadblock_swizzle.h:112:88: warning: implicit conversion changes signedness: 'Index' (aka 'int') to 'unsigned int' [-Wsign-conversion]
   1553 .../cutlass-3.4.1/test/unit/gemm/device/testbed_universal.h:135:60: warning: implicit conversion changes signedness: '::size_t' (aka 'unsigned long') to '::int64_t' (aka 'long') [-Wsign-conversion]
   1235 .../cutlass-3.4.1/include/cutlass/half.h:227:15: warning: implicit conversion changes signedness: 'uint16_t' (aka 'unsigned short') to 'int16_t' (aka 'short') [-Wsign-conversion]
   1235 .../cutlass-3.4.1/include/cutlass/half.h:251:8: warning: implicit conversion changes signedness: 'uint16_t' (aka 'unsigned short') to 'int16_t' (aka 'short') [-Wsign-conversion]
    929 .../cutlass-3.4.1/include/cute/atom/mma_traits_sm90_gmma.hpp:211:51: warning: implicit conversion loses integer precision: 'uint32_t' (aka 'unsigned int') to 'uint16_t' (aka 'unsigned short') [-Wimplicit-int-conversion]
    927 .../cutlass-3.4.1/include/cutlass/half.h:227:17: warning: implicit conversion changes signedness: '::uint16_t' (aka 'unsigned short') to '::int16_t' (aka 'short') [-Wsign-conversion]
    927 .../cutlass-3.4.1/include/cutlass/half.h:251:8: warning: implicit conversion changes signedness: '::uint16_t' (aka 'unsigned short') to '::int16_t' (aka 'short') [-Wsign-conversion]
    917 .../cutlass-3.4.1/include/cutlass/numeric_conversion.h:3708:46: warning: implicit conversion loses integer precision: 'unsigned int' to '::uint8_t' (aka 'unsigned char') [-Wimplicit-int-conversion]

iskunk avatar Mar 06 '24 00:03 iskunk

Hi! Thanks a lot for this MR! We are about to release CUTLASS 3.5 in the next 10 days or so. This update includes tons of fixes for warnings as we move towards a warnings as errors build system. Would you mind waiting for this MR until we release 3.5 and then rebasing to see if there is anything we have missed?

thakkarV avatar Mar 06 '24 16:03 thakkarV

Hi @thakkarV, is there a repo where I can try out the new version?

I have some additional (non-warning) fixes for the current tree, so I'll push those out ASAP.

iskunk avatar Mar 06 '24 19:03 iskunk

No other repo unfortunately. We expect 3.5 to be available in this repo by March 15th.

thakkarV avatar Mar 06 '24 19:03 thakkarV

Hi @mhoemmen, thanks for looking at this.

To help answer many of the above questions, let me provide you with an updated list of the warnings generated from my build, specifically for an unmodified Git revision f9ece1b4. This was generated with grep warning: build.out | sort | uniq -c | sort -nr | grep -E '^ *[0-9]{4} ' | sed 's!/tmp/cutlass/!.../!':

   2210 .../include/cutlass/half.h:251:8: warning: implicit conversion changes signedness: 'uint16_t' (aka 'unsigned short') to 'int16_t' (aka 'short') [-Wsign-conversion]
   2210 .../include/cutlass/half.h:227:15: warning: implicit conversion changes signedness: 'uint16_t' (aka 'unsigned short') to 'int16_t' (aka 'short') [-Wsign-conversion]
   2205 .../include/cutlass/fast_math.h:376:17: warning: implicit conversion changes signedness: 'int' to 'unsigned int' [-Wsign-conversion]
   2205 .../include/cutlass/fast_math.h:254:8: warning: implicit conversion changes signedness: 'int' to 'unsigned int' [-Wsign-conversion]
   2182 .../include/cute/numeric/integral_constant.hpp:454:26: warning: implicit conversion changes signedness: 'int' to 'uint64_t' (aka 'unsigned long') [-Wsign-conversion]
   2180 .../include/cute/numeric/math.hpp:154:49: warning: implicit conversion changes signedness: 'int' to 'unsigned int' [-Wsign-conversion]
   1863 .../include/cutlass/numeric_conversion.h:3708:44: warning: implicit conversion loses integer precision: 'unsigned int' to 'uint8_t' (aka 'unsigned char') [-Wimplicit-int-conversion]
   1854 .../include/cutlass/arch/mma_sm60.h:88:5: warning: implicit conversion changes signedness: 'int' to 'size_type' (aka 'unsigned long') [-Wsign-conversion]
   1854 .../include/cutlass/arch/mma_sm60.h:88:38: warning: implicit conversion changes signedness: 'int' to 'size_type' (aka 'unsigned long') [-Wsign-conversion]
   1854 .../include/cutlass/arch/mma_sm60.h:88:18: warning: implicit conversion changes signedness: 'int' to 'size_type' (aka 'unsigned long') [-Wsign-conversion]
   1854 .../include/cutlass/arch/mma_sm60.h:134:5: warning: implicit conversion changes signedness: 'int' to 'size_type' (aka 'unsigned long') [-Wsign-conversion]
   1854 .../include/cutlass/arch/mma_sm60.h:134:38: warning: implicit conversion changes signedness: 'int' to 'size_type' (aka 'unsigned long') [-Wsign-conversion]
   1854 .../include/cutlass/arch/mma_sm60.h:134:27: warning: implicit conversion changes signedness: 'int' to 'size_type' (aka 'unsigned long') [-Wsign-conversion]
   1850 .../include/cutlass/arch/mma_sm61.h:86:22: warning: implicit conversion changes signedness: 'int' to 'size_type' (aka 'unsigned long') [-Wsign-conversion]
   1850 .../include/cutlass/arch/mma_sm61.h:86:13: warning: implicit conversion changes signedness: 'int' to 'size_type' (aka 'unsigned long') [-Wsign-conversion]
   1850 .../include/cutlass/arch/mma_sm61.h:133:22: warning: implicit conversion changes signedness: 'int' to 'size_type' (aka 'unsigned long') [-Wsign-conversion]
   1850 .../include/cutlass/arch/mma_sm61.h:133:13: warning: implicit conversion changes signedness: 'int' to 'size_type' (aka 'unsigned long') [-Wsign-conversion]
   1850 .../include/cutlass/arch/mma_sm60.h:242:56: warning: implicit conversion changes signedness: 'int' to 'size_type' (aka 'unsigned long') [-Wsign-conversion]
   1850 .../include/cutlass/arch/mma_sm60.h:242:37: warning: implicit conversion changes signedness: 'int' to 'size_type' (aka 'unsigned long') [-Wsign-conversion]
   1850 .../include/cutlass/arch/mma_sm60.h:242:28: warning: implicit conversion changes signedness: 'int' to 'size_type' (aka 'unsigned long') [-Wsign-conversion]
   1850 .../include/cutlass/arch/mma_sm60.h:242:13: warning: implicit conversion changes signedness: 'int' to 'size_type' (aka 'unsigned long') [-Wsign-conversion]
   1850 .../include/cutlass/arch/mma_sm60.h:188:7: warning: implicit conversion changes signedness: 'int' to 'size_type' (aka 'unsigned long') [-Wsign-conversion]
   1850 .../include/cutlass/arch/mma_sm60.h:188:50: warning: implicit conversion changes signedness: 'int' to 'size_type' (aka 'unsigned long') [-Wsign-conversion]
   1850 .../include/cutlass/arch/mma_sm60.h:188:37: warning: implicit conversion changes signedness: 'int' to 'size_type' (aka 'unsigned long') [-Wsign-conversion]
   1850 .../include/cutlass/arch/mma_sm60.h:188:28: warning: implicit conversion changes signedness: 'int' to 'size_type' (aka 'unsigned long') [-Wsign-conversion]
   1780 .../include/cutlass/predicate_vector.h:163:19: warning: implicit conversion changes signedness: 'int' to 'unsigned long' [-Wsign-conversion]
   1780 .../include/cutlass/predicate_vector.h:162:9: warning: implicit conversion changes signedness: 'int' to 'unsigned long' [-Wsign-conversion]
   1778 .../include/cutlass/layout/permute.h:517:58: warning: implicit conversion changes signedness: 'const unsigned int' to 'Index' (aka 'int') [-Wsign-conversion]
   1778 .../include/cutlass/layout/permute.h:456:58: warning: implicit conversion changes signedness: 'const unsigned int' to 'Index' (aka 'int') [-Wsign-conversion]
   1778 .../include/cutlass/layout/permute.h:384:58: warning: implicit conversion changes signedness: 'const unsigned int' to 'Index' (aka 'int') [-Wsign-conversion]
   1778 .../include/cutlass/layout/permute.h:321:58: warning: implicit conversion changes signedness: 'const unsigned int' to 'Index' (aka 'int') [-Wsign-conversion]
   1778 .../include/cutlass/gemm/warp/mma_tensor_op.h:124:48: warning: implicit conversion changes signedness: 'int' to 'unsigned int' [-Wsign-conversion]
   1778 .../include/cutlass/gemm/warp/mma_tensor_op.h:124:45: warning: implicit conversion changes signedness: 'unsigned int' to 'int' [-Wsign-conversion]
   1778 .../include/cutlass/gemm/warp/mma_tensor_op.h:103:48: warning: implicit conversion changes signedness: 'int' to 'unsigned int' [-Wsign-conversion]
   1778 .../include/cutlass/gemm/warp/mma_tensor_op.h:103:45: warning: implicit conversion changes signedness: 'unsigned int' to 'int' [-Wsign-conversion]
   1754 .../include/cutlass/epilogue/warp/tile_iterator_tensor_op_mixed.h:964:21: warning: implicit conversion changes signedness: 'Index' (aka 'long') to 'size_t' (aka 'unsigned long') [-Wsign-conversion]
   1754 .../include/cutlass/epilogue/warp/tile_iterator_tensor_op_mixed.h:775:21: warning: implicit conversion changes signedness: 'Index' (aka 'long') to 'size_t' (aka 'unsigned long') [-Wsign-conversion]
   1754 .../include/cutlass/epilogue/warp/tile_iterator_tensor_op_mixed.h:580:21: warning: implicit conversion changes signedness: 'Index' (aka 'long') to 'size_t' (aka 'unsigned long') [-Wsign-conversion]
   1754 .../include/cutlass/epilogue/warp/tile_iterator_tensor_op_mixed.h:385:21: warning: implicit conversion changes signedness: 'Index' (aka 'long') to 'size_t' (aka 'unsigned long') [-Wsign-conversion]
   1752 .../include/cutlass/gemm/threadblock/threadblock_swizzle_streamk.h:449:86: warning: implicit conversion changes signedness: 'Index' (aka 'int') to 'size_t' (aka 'unsigned long') [-Wsign-conversion]
   1752 .../include/cutlass/gemm/threadblock/threadblock_swizzle_streamk.h:449:62: warning: implicit conversion changes signedness: 'Index' (aka 'int') to 'size_t' (aka 'unsigned long') [-Wsign-conversion]
   1752 .../include/cutlass/gemm/threadblock/threadblock_swizzle_streamk.h:449:225: warning: implicit conversion changes signedness: 'Index' (aka 'int') to 'size_t' (aka 'unsigned long') [-Wsign-conversion]
   1752 .../include/cutlass/gemm/threadblock/threadblock_swizzle_streamk.h:449:201: warning: implicit conversion changes signedness: 'Index' (aka 'int') to 'size_t' (aka 'unsigned long') [-Wsign-conversion]
   1752 .../include/cutlass/gemm/threadblock/threadblock_swizzle_streamk.h:449:155: warning: implicit conversion changes signedness: 'Index' (aka 'int') to 'size_t' (aka 'unsigned long') [-Wsign-conversion]
   1752 .../include/cutlass/gemm/threadblock/threadblock_swizzle_streamk.h:449:131: warning: implicit conversion changes signedness: 'Index' (aka 'int') to 'size_t' (aka 'unsigned long') [-Wsign-conversion]
   1695 .../test/unit/gemm/device/testbed_universal.h:135:60: warning: implicit conversion changes signedness: '::size_t' (aka 'unsigned long') to 'int64_t' (aka 'long') [-Wsign-conversion]
   1643 .../include/cutlass/gemm/threadblock/threadblock_swizzle.h:112:88: warning: implicit conversion changes signedness: 'Index' (aka 'int') to 'unsigned int' [-Wsign-conversion]
   1643 .../include/cutlass/gemm/threadblock/threadblock_swizzle.h:112:68: warning: implicit conversion changes signedness: 'int' to 'unsigned int' [-Wsign-conversion]
   1643 .../include/cutlass/gemm/threadblock/threadblock_swizzle.h:112:29: warning: implicit conversion changes signedness: 'int' to 'unsigned int' [-Wsign-conversion]
   1621 .../include/cutlass/gemm/threadblock/threadblock_swizzle.h:112:90: warning: implicit conversion changes signedness: 'Index' (aka 'int') to 'unsigned int' [-Wsign-conversion]
   1621 .../include/cutlass/gemm/threadblock/threadblock_swizzle.h:112:70: warning: implicit conversion changes signedness: 'int' to 'unsigned int' [-Wsign-conversion]
   1621 .../include/cutlass/gemm/threadblock/threadblock_swizzle.h:112:31: warning: implicit conversion changes signedness: 'int' to 'unsigned int' [-Wsign-conversion]

You might consider it worth the trouble to reproduce these with your own installation of the Intel oneAPI compiler, to cut out the middleman.

I will read through your review comments and respond/revise as appropriate. Then a test build, and (hopefully) an update to the PR.

iskunk avatar Apr 01 '24 23:04 iskunk

I've updated and rebased my PR. With these changes, the most frequent warning occurs 969 times.

iskunk avatar Apr 02 '24 23:04 iskunk

@iskunk Thanks for your interest in CUTLASS and for all your work!

You might consider it worth the trouble to reproduce these with your own installation of the Intel oneAPI compiler, to cut out the middleman.

Please note that the Intel oneAPI compiler is not currently a supported or tested build configuration. We do support and test Clang as host compiler plus NVCC as device compiler.

mhoemmen avatar Apr 03 '24 18:04 mhoemmen

This PR has been labeled inactive-30d due to no recent activity in the past 30 days. Please close this PR if it is no longer required. Otherwise, please respond with a comment indicating any updates. This PR will be labeled inactive-90d if there is no activity in the next 60 days.

github-actions[bot] avatar May 04 '24 22:05 github-actions[bot]

Hello @mhoemmen, have you had a chance to try out these changes? I've rebased the PR to current main.

iskunk avatar May 06 '24 06:05 iskunk

@iskunk Thank you for your interest in CUTLASS!

Changes like the following, that replace int with size_t,

CUTLASS_PRAGMA_UNROLL
for (size_t k = 0; k < 2; ++k) {

cannot possibly reduce the number of warnings, given that the C++ Standard guarantees that 2 is an int literal. We test with GCC, Clang, and MSVC, and don't see build warnings when using int as the index type. Thus, I strongly suspect that something is wrong with the oneAPI compiler.

You might consider it worth the trouble to reproduce these with your own installation of the Intel oneAPI compiler, to cut out the middleman.

We do not support the Intel oneAPI compiler. We would consider code changes that happen to improve the oneAPI compiler experience as long as they improve code quality overall, but changes like the above objectively don't do that.

mhoemmen avatar May 07 '24 00:05 mhoemmen

I think the oneAPI warnings on that bit come down to operator[] taking a size_t. Since overloading that with int doesn't appear to work, is there a way to declare that that is more forgiving on the integer type? Not least since specifying size_t can have the implication that an unsigned type is required.

iskunk avatar May 07 '24 23:05 iskunk

@iskunk Thanks for the suggestion! We contemplated overloading operator[], but decided against that. The C++ Standard Library has several types with an array access operator[] with one integer parameter: std::array, std::span, std::string, std::string_view, std::valarray, and std::vector. For all of them, the parameter's type is size_t. None of them have overloads for other integer types or take the type as a template parameter.

I've implemented an example and run it through the latest icx (and the 2024 and 2023 releases).

https://godbolt.org/z/P3ch9T9x1

For std::array, std::vector, std::span, and my hand-rolled versions of these three classes (to demonstrate that the compiler doesn't treat Standard Library types specially), I could not reproduce warnings with code like the following, even with -Wall -Werror.

x[0] = 1.0f;
x[1] = 2.0f;

for (int k = 0; k < 10; ++k) {
  x[k] = static_cast<float>(k) + 1.0f;
}

Please let me know if I'm using icx incorrectly.

mhoemmen avatar May 08 '24 17:05 mhoemmen

This PR has been labeled inactive-30d due to no recent activity in the past 30 days. Please close this PR if it is no longer required. Otherwise, please respond with a comment indicating any updates. This PR will be labeled inactive-90d if there is no activity in the next 60 days.

github-actions[bot] avatar Jun 07 '24 18:06 github-actions[bot]

Argh, this slipped through the cracks. My apologies.

I was not able to reproduce those particular warnings in Godbolt. It is possible that the specific C++ backend in use has something to do with it.

In any event, given that those fixes are only a few out of the bunch, I've rebased the PR and updated it to remove them. The remainder should be, as I've understood, the ones on which there is agreement. Please let me know if any further work is needed.

iskunk avatar Jun 07 '24 18:06 iskunk

This PR has been labeled inactive-30d due to no recent activity in the past 30 days. Please close this PR if it is no longer required. Otherwise, please respond with a comment indicating any updates. This PR will be labeled inactive-90d if there is no activity in the next 60 days.

github-actions[bot] avatar Jul 10 '24 16:07 github-actions[bot]

Please let me know if there are any further concerns here. My understanding is that the remaining changes are reasonable disambiguations that have not drawn any objections.

iskunk avatar Jul 10 '24 22:07 iskunk