Skip to content
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.

Sign up for GitHub

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

Cleanup warnings with nvhpc/21.9. #2582

Merged
merged 5 commits into from
Nov 5, 2021
Merged

Cleanup warnings with nvhpc/21.9. #2582

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ec220be
Cleanup warnings with nvhpc/21.9.
olupton Nov 4, 2021
79ab452
Move __NVCOMPILER check.
olupton Nov 4, 2021
eafdd3e
Be more explicit.
olupton Nov 4, 2021
0230d69
Immediately executed lambda.
olupton Nov 5, 2021
c9de97e
Fix shadowing warning.
olupton Nov 5, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion include/fmt/core.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@

// Enable minimal optimizations for more compact code in debug mode.
FMT_GCC_PRAGMA("GCC push_options")
#ifndef __OPTIMIZE__
#if !defined(__OPTIMIZE__) && !defined(__NVCOMPILER)
olupton marked this conversation as resolved.
Show resolved Hide resolved
FMT_GCC_PRAGMA("GCC optimize(\"Og\")")
#endif

Expand Down
15 changes: 9 additions & 6 deletions include/fmt/format.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -967,14 +967,17 @@ FMT_CONSTEXPR20 inline auto count_digits(uint64_t n) -> int {
template <int BITS, typename UInt>
FMT_CONSTEXPR auto count_digits(UInt n) -> int {
#ifdef FMT_BUILTIN_CLZ
if (num_bits<UInt>() == 32)
if (num_bits<UInt>() == 32) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's wrap the condition in const_check instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

template <int BITS, typename UInt>
FMT_CONSTEXPR auto count_digits(UInt n) -> int {
#ifdef FMT_BUILTIN_CLZ
  if (const_check(num_bits<UInt>() == 32))
    return (FMT_BUILTIN_CLZ(static_cast<uint32_t>(n) | 1) ^ 31) / BITS + 1;
#endif
  int num_digits = 0;

doesn't squash the warning.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's unfortunate but I think the suppression with extra blocks is too messy. Is there another way to do this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

eafdd3e is another way -- I wouldn't say it's a particularly nice way...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this is clearly worse =(. Does moving the definition of num_digits to the beginning of the function suppress the warning?

  int num_digits = 0;
#ifdef FMT_BUILTIN_CLZ
  if (const_check(num_bits<UInt>() == 32))
    return (FMT_BUILTIN_CLZ(static_cast<uint32_t>(n) | 1) ^ 31) / BITS + 1;
#endif

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice idea, but that just gives

warning #128-D: loop is not reachable

on the do { line. And I would be a little worried about other compilers giving an unused variable warning too...

I pushed yet another attempt, with an immediately executed lambda, which seems to work...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks better but please fix the CI issues.

return (FMT_BUILTIN_CLZ(static_cast<uint32_t>(n) | 1) ^ 31) / BITS + 1;
} else
#endif
int num_digits = 0;
do {
++num_digits;
} while ((n >>= BITS) != 0);
return num_digits;
{
int num_digits = 0;
do {
++num_digits;
} while ((n >>= BITS) != 0);
return num_digits;
}
}

template <> auto count_digits<4>(detail::fallback_uintptr n) -> int;
Expand Down