Warnings about float to int conversions are wrong

[amaiorano]

See this comment on my PR that has now landed. As shown there, for certain out-of-range float to integral conversions, the compiler warns that the literal value it will use is incorrect. Here's a DXC Compiler Explorer example showing these specific warnings, some of which are clearly incorrect. Here's the output:

<source>:13:19: warning: implicit conversion from 'literal float' to 'int' changes value from 2147483648 to 2147483647 [-Wliteral-conversion]
    store(to_int(-2147483648.0)); // MaxNegative int: -2147483648
          ~~~~~~  ^~~~~~~~~~~~
<source>:14:19: warning: implicit conversion from 'literal float' to 'int' changes value from 1.797693134862316E+308 to 2147483647 [-Wliteral-conversion]
    store(to_int(-1.7976931348623158e+308)); // MaxNegative double: -2147483648 (clamp int)
          ~~~~~~  ^~~~~~~~~~~~~~~~~~~~~~~
<source>:15:18: warning: implicit conversion from 'literal float' to 'int' changes value from 1.797693134862316E+308 to 2147483647 [-Wliteral-conversion]
    store(to_int(1.7976931348623158e+308)); // MaxPositive double: 2147483647 (clamp int)
          ~~~~~~ ^~~~~~~~~~~~~~~~~~~~~~~
<source>:16:20: warning: implicit conversion from 'literal float' to 'uint' changes value from 1.797693134862316E+308 to 4294967295 [-Wliteral-conversion]
    store(to_uint(-1.7976931348623158e+308)); // MaxNegative double: 0 (clamp uint)
          ~~~~~~~  ^~~~~~~~~~~~~~~~~~~~~~~
<source>:17:19: warning: implicit conversion from 'literal float' to 'uint' changes value from 1.797693134862316E+308 to 4294967295 [-Wliteral-conversion]
    store(to_uint(1.7976931348623158e+308)); // MaxPositive double: 4294967295 (clamp uint)
          ~~~~~~~ ^~~~~~~~~~~~~~~~~~~~~~~

The comments on each line shows what the actual value will be. We can see that the warning doesn't always match the value. For example, the first one warns that the value will be converted to 2147483647, but in fact, it will be -2147483648.

Here's a C++ Compiler Explorer example showing what Clang 3.8 vs 3.9 does to similar C++ code. Pay attention to the warnings emitted by the compiler, rather than the actual output (the output is mostly garbage, which isn't unexpected since these conversions are UB). We can see that the 3.8 version is what DXC is emitting.

So we should update this warning code with a later version. In fact, in latest Clang, it doesn't bother saying what the literal value will be changed to since it's UB. It says, instead:

<source>:28:12: warning: implicit conversion of out of range value from 'double' to 'int' is undefined [-Wliteral-conversion]

But for DXC, if we want to make these well-defined, as my patch did, then we could go with the 3.9 changes instead.

[llvm-beanz]

@amaiorano is this something you're going to take a look at or should we?

[amaiorano]

@amaiorano is this something you're going to take a look at or should we?

I wasn't planning to work on this any time soon. I also can appreciate that this is not a particularly high priority issue.

[llvm-beanz]

All good. We've got it on our backlog. If someone gets bored maybe we'll track down the LLVM 3.8/3.9 change that fixed this and cherry-pick it back.

[llvm-beanz]

@amaiorano, this may be resolved with HLSL 202x as a result of https://github.com/microsoft/hlsl-specs/blob/main/proposals/0017-conforming-literals.md.