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.

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

-Wimplicit-float-size-conversion is implied by -Wimplicit-float-conversion #16393

Open
Maetveis opened this issue Dec 17, 2024 · 1 comment
Open
Labels
bug Something isn't working

Comments

@Maetveis
Copy link
Contributor

Maetveis commented Dec 17, 2024

Describe the bug

Currently the DPC++ compiler warns when a float variable is assigned a double literal, even if the converted value is precisely representable in float. This warning is enabled by -Wimplicit-float-conversion, but that causes the meaning of that option to be different between DPC++ and upstream clang / GCC.

Quoting GCC because clang generally has no place where its warnings are specified, but it behaves the same way as GCC in this case.

The description of -Wconversion (which also enables -Wimplicit-float-conversion in clang) reads:

-Wconversion
Warn for implicit conversions that may alter a value.

(https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wconversion)

To reproduce

const float a = 1.0;
> icpx -Wimplicit-float-conversion -c source.cpp
source.cpp:1:17: warning: implicit conversion between floating point types of different sizes [-Wimplicit-float-size-conversion]
    1 | const float a = 1.0;
      |             ~   ^~~

Expected same behavior as clang:

> clang++ -Wimplicit-float-conversion -c source.cpp
# No warnings emitted

Environment

  • OS: Any
  • Target device and vendor: Any
  • DPC++ version: 2025.0
  • Dependencies version: N/A

Additional context

Breaking compatibility with upstream clang is problematic because it makes it harder to build projects that build cleanly with clang. As an example the LLVM libc subproject has the following code that trigers the warning when using DPC++ (despite not explicitly enabling -Wimplicit-float-size-conversion and not using -fsycl)

libc/src/math/generic/sincosf16_utils.h:25-41

Compiler explorer link

-Wimplicit-float-size-conversion was introduced in #6323 to address #5783

False positives

#5783 is about detecting accidental implicit conversions that might cause slower 64-bit arithmetic to be emitted. An assignment to a float variable with a double literal will clearly not cause such runtime slowdown.

In my opinion these are false positives of the warning, but I see that in the discussion of #6323:

I think what we have here (diagnosing any floating-point conversion that isn't covered by one of the precision warnings) has enough utility to be worth supporting

So if this is is the intention then the below points are not relevant, except that I think that definition is more noisy than useful.

E.g. the diagnostic could exclude assignments, e.g. none of the following should warn:

float a = 1.0;
float b = flip_coin() ? a : 1.5;
float c[3] = {1.0, 2.0, 3.0}
void func(float f); func(1.0); // Will almost certainly not use double at runtime
float get_pi() { return 3.14; }
std::vector<float> complex = {1.0, 2.0}; // std::initializer_list with literals only
// This might be hard, in the AST this is a constructor call, but this special case should
// likely be okay. Though a false positive here is probably okay.
std::complex<float> complex = 1.0;

While the following should still warn with -Wimplicit-float-size-conversion

float a;
double d;
void func(float f); func(d);
float b = a + 1.0; // Addition is in double
float c = flip_coin() ? a : d;
@Maetveis Maetveis added the bug Something isn't working label Dec 17, 2024
@Maetveis
Copy link
Contributor Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant