fix compiler warnings by TorreZuk · Pull Request #1623 · ROCm/Tensile

TorreZuk · 2022-11-17T17:16:40Z

No description provided.

AlexBrownAMD · 2022-11-17T17:37:50Z

                auto compM = [&count, Debug](Entry const& e, long const M) {
                    if(Debug)
-                        printf("[%ld,%ld,%ld]\n", e.key[0], e.key[1], e.key[2]);
+                        printf("[%lld,%lld,%lld]\n", e.key[0], e.key[1], e.key[2]);


I'm not as familiar with this part of the code, but it looks like key can also be type float? Maybe it doesn't hit this code path if the only warning was for lld

Can we use std::cout instead of using printf?

@nakajee you can push a commit to this PR with cout as I just want the warning fixed, you can use whatever Tensile team prefers (these seem to be size_t members for the warnings you get in CI, see ). Is there still no run of your own windows CI run directly on Tensile?

@TorreZuk

I pushed cout version.
Even though we changed ld to lld, I can still see the opposite version of warning as follows.

===

/var/jenkins_home/workspace/clang_precheckin_Tensile_PR-1623/ZwdxdW0y1/tensile/Tensile/Source/lib/include/Tensile/PropertyMatching.hpp:698:64: warning: format specifies type 'long long' but the argument has type '__gnu_cxx::__alloc_traits<std::allocator, long>::value_type' (aka 'long') [-Wformat]

printf("[%lld,%lld,%lld]\n", e.key[0], e.key[1], e.key[2]); ~~~~ ^~~~~~~~ %ld

===

Let's see if using cout can fix the warning or not.

By the way, probably, this is not just win compiler issue.

TorreZuk

Very good, windows clang is slightly different version to linux clang, my change built rocblas without windows warnings. But yes if you are happy with that format should fix both, it's debug output so hopefully not parsed.

TorreZuk · 2022-11-17T20:25:44Z

PS - someone on tensile side should probably see if the type instantiation should be the same on windows and linux (i.e. long or long long)). Unrelated to fixing the warning but could be a problem down the road.

TorreZuk · 2022-11-18T18:37:39Z

@nakajee thanks for taking this over, I assume I can leave it to you to merge as you will know what failures are the norm and can be ignored?

nakajee · 2022-11-18T18:45:33Z

@nakajee thanks for taking this over, I assume I can leave it to you to merge as you will know what failures are the norm and can be ignored?

I can handle merge.
I am still waiting for CI tests.
Fails on extended 906 is expected (will be fixed with #1617).
Not sure why 908 taking so long time.

…ings (#12326) ## Motivation This fixes ~500 warnings when building on Windows (see ROCm/TheRock#47), saving 1-2MB of log size per GPU family build. For example: ``` 3.4 In file included from C:/home/runner/_work/TheRock/TheRock/rocm-libraries/projects/hipblaslt/tensilelite/src/msgpack/MessagePack.cpp:27: 3.4 In file included from C:/home/runner/_work/TheRock/TheRock/rocm-libraries/projects/hipblaslt/tensilelite/include\Tensile/msgpack/MessagePack.hpp:31: 3.4 In file included from C:/home/runner/_work/TheRock/TheRock/rocm-libraries/projects/hipblaslt/tensilelite/include\Tensile/ContractionLibrary.hpp:34: 3.4 In file included from C:/home/runner/_work/TheRock/TheRock/rocm-libraries/projects/hipblaslt/tensilelite/include\Tensile/GranularitySelectionLibrary.hpp:38: 3.4 C:/home/runner/_work/TheRock/TheRock/rocm-libraries/projects/hipblaslt/tensilelite/include\Tensile/PropertyMatching.hpp:1106:67: warning: format specifies type 'long' but the argument has type 'long long' [-Wformat] 3.4 1106 | printf("[ %ld ,%ld,%ld,%ld]\n", e.key[0], e.key[1], e.key[2], e.key[3]); 3.4 | ~~~ ^~~~~~~~ 3.4 | %lld 3.4 C:/home/runner/_work/TheRock/TheRock/rocm-libraries/projects/hipblaslt/tensilelite/include\Tensile/PropertyMatching.hpp:1106:77: warning: format specifies type 'long' but the argument has type 'long long' [-Wformat] 3.4 1106 | printf("[ %ld ,%ld,%ld,%ld]\n", e.key[0], e.key[1], e.key[2], e.key[3]); 3.4 | ~~~ ^~~~~~~~ 3.4 | %lld ... 3.4 C:/home/runner/_work/TheRock/TheRock/rocm-libraries/projects/hipblaslt/tensilelite/src/msgpack/MessagePack.cpp:413:9: note: in instantiation of function template specialization 'TensileLite::MessagePackLoadLibraryFile<TensileLite::ContractionProblemGemm, TensileLite::ContractionSolution>' requested here 3.4 413 | MessagePackLoadLibraryFile<ContractionProblemGemm, ContractionSolution>( 3.4 | ^ ``` ## Technical Details The `%ld` format specifier expects `long`, while the values are `int64_t`. * On 64-bit Linux, `long` is typically 64 bits * On 64-bit Windows, `long` is 32 bits while `int64_t` is always 64 bits (`long long`) See https://en.cppreference.com/cpp/types/integer#Format_macro_constants, `PRId64` is recommended for `int64_t`. > [!NOTE] > The `tensilelite/include/Tensile/PropertyMatching.hpp` fix was already applied to _Tensile_ in ROCm/Tensile#1623 / 37278db, including switching from `printf` to `std::cout`. > https://github.com/ROCm/rocm-libraries/blob/ab0a5742e4480ade753a606fab6d6945fd2c0a51/shared/tensile/Tensile/Source/lib/include/Tensile/PropertyMatching.hpp#L698-L701 ## Test Plan Check that CI logs no longer contain these warnings. Run details | Logs link | file size -- | -- | --: baseline | [35375442872-windows/...hipBLASLt_build.log](https://therock-ci-artifacts-external.s3.amazonaws.com/ROCm-rocm-libraries/35375442872-windows/logs/math-libs/gfx1151/hipBLASLt_build.log) | 1247KB this PR | [35397683840-windows/...hipBLASLt_build.log](https://therock-ci-artifacts-external.s3.amazonaws.com/ROCm-rocm-libraries/35397683840-windows/logs/math-libs/gfx1151/hipBLASLt_build.log) | 87KB ## Submission Checklist - [x] Look over the contributing guidelines at https://github.com/ROCm/TheRock/blob/main/GOVERNANCE.md#pull-requests.

fix(hipblaslt): use portable formatting for 64-bit values to fix warnings (#12326) ## Motivation This fixes ~500 warnings when building on Windows (see ROCm/TheRock#47), saving 1-2MB of log size per GPU family build. For example: ``` 3.4 In file included from C:/home/runner/_work/TheRock/TheRock/rocm-libraries/projects/hipblaslt/tensilelite/src/msgpack/MessagePack.cpp:27: 3.4 In file included from C:/home/runner/_work/TheRock/TheRock/rocm-libraries/projects/hipblaslt/tensilelite/include\Tensile/msgpack/MessagePack.hpp:31: 3.4 In file included from C:/home/runner/_work/TheRock/TheRock/rocm-libraries/projects/hipblaslt/tensilelite/include\Tensile/ContractionLibrary.hpp:34: 3.4 In file included from C:/home/runner/_work/TheRock/TheRock/rocm-libraries/projects/hipblaslt/tensilelite/include\Tensile/GranularitySelectionLibrary.hpp:38: 3.4 C:/home/runner/_work/TheRock/TheRock/rocm-libraries/projects/hipblaslt/tensilelite/include\Tensile/PropertyMatching.hpp:1106:67: warning: format specifies type 'long' but the argument has type 'long long' [-Wformat] 3.4 1106 | printf("[ %ld ,%ld,%ld,%ld]\n", e.key[0], e.key[1], e.key[2], e.key[3]); 3.4 | ~~~ ^~~~~~~~ 3.4 | %lld 3.4 C:/home/runner/_work/TheRock/TheRock/rocm-libraries/projects/hipblaslt/tensilelite/include\Tensile/PropertyMatching.hpp:1106:77: warning: format specifies type 'long' but the argument has type 'long long' [-Wformat] 3.4 1106 | printf("[ %ld ,%ld,%ld,%ld]\n", e.key[0], e.key[1], e.key[2], e.key[3]); 3.4 | ~~~ ^~~~~~~~ 3.4 | %lld ... 3.4 C:/home/runner/_work/TheRock/TheRock/rocm-libraries/projects/hipblaslt/tensilelite/src/msgpack/MessagePack.cpp:413:9: note: in instantiation of function template specialization 'TensileLite::MessagePackLoadLibraryFile<TensileLite::ContractionProblemGemm, TensileLite::ContractionSolution>' requested here 3.4 413 | MessagePackLoadLibraryFile<ContractionProblemGemm, ContractionSolution>( 3.4 | ^ ``` ## Technical Details The `%ld` format specifier expects `long`, while the values are `int64_t`. * On 64-bit Linux, `long` is typically 64 bits * On 64-bit Windows, `long` is 32 bits while `int64_t` is always 64 bits (`long long`) See https://en.cppreference.com/cpp/types/integer#Format_macro_constants, `PRId64` is recommended for `int64_t`. > [!NOTE] > The `tensilelite/include/Tensile/PropertyMatching.hpp` fix was already applied to _Tensile_ in ROCm/Tensile#1623 / ROCm/rocm-libraries@37278db, including switching from `printf` to `std::cout`. > https://github.com/ROCm/rocm-libraries/blob/ab0a5742e4480ade753a606fab6d6945fd2c0a51/shared/tensile/Tensile/Source/lib/include/Tensile/PropertyMatching.hpp#L698-L701 ## Test Plan Check that CI logs no longer contain these warnings. Run details | Logs link | file size -- | -- | --: baseline | [35375442872-windows/...hipBLASLt_build.log](https://therock-ci-artifacts-external.s3.amazonaws.com/ROCm-rocm-libraries/35375442872-windows/logs/math-libs/gfx1151/hipBLASLt_build.log) | 1247KB this PR | [35397683840-windows/...hipBLASLt_build.log](https://therock-ci-artifacts-external.s3.amazonaws.com/ROCm-rocm-libraries/35397683840-windows/logs/math-libs/gfx1151/hipBLASLt_build.log) | 87KB ## Submission Checklist - [x] Look over the contributing guidelines at https://github.com/ROCm/TheRock/blob/main/GOVERNANCE.md#pull-requests.

fix win compiler warnings

493acd9

TorreZuk requested review from AlexBrownAMD, babakpst and lringham November 17, 2022 17:16

AlexBrownAMD approved these changes Nov 17, 2022

View reviewed changes

Use std::cout instead of printf

b8c7d86

nakajee changed the title ~~fix win compiler warnings~~ fix compiler warnings Nov 17, 2022

TorreZuk commented Nov 17, 2022

View reviewed changes

fix clang format issue

d1ceb20

nakajee merged commit ead1931 into ROCm:develop Nov 19, 2022

ScottTodd mentioned this pull request Sep 18, 2026

fix(hipblaslt): use portable formatting for 64-bit values to fix warnings ROCm/rocm-libraries#12326

Merged

1 task done

Navigation Menu

Repository navigation

fix compiler warnings - #1623

fix compiler warnings#1623
nakajee merged 3 commits into
ROCm:developfrom
TorreZuk:fix-win-warnings

TorreZuk commented Nov 17, 2022

Uh oh!

AlexBrownAMD Nov 17, 2022

Uh oh!

nakajee Nov 17, 2022

Uh oh!

TorreZuk Nov 17, 2022

Uh oh!

nakajee Nov 17, 2022 •

edited

Loading

Uh oh!

nakajee Nov 17, 2022

Uh oh!

TorreZuk left a comment

Uh oh!

TorreZuk commented Nov 17, 2022

Uh oh!

TorreZuk commented Nov 18, 2022

Uh oh!

nakajee commented Nov 18, 2022

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

TorreZuk commented Nov 17, 2022

Uh oh!

AlexBrownAMD Nov 17, 2022

Choose a reason for hiding this comment

Uh oh!

nakajee Nov 17, 2022

Choose a reason for hiding this comment

Uh oh!

TorreZuk Nov 17, 2022

Choose a reason for hiding this comment

Uh oh!

nakajee Nov 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nakajee Nov 17, 2022

Choose a reason for hiding this comment

Uh oh!

TorreZuk left a comment

Choose a reason for hiding this comment

Uh oh!

TorreZuk commented Nov 17, 2022

Uh oh!

TorreZuk commented Nov 18, 2022

Uh oh!

nakajee commented Nov 18, 2022

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

nakajee Nov 17, 2022 •

edited

Loading