[0017] Begin Review Period

[llvm-beanz]

Which proposal does this relate to?
0017 - Conforming Literals

Overview
This sets a review period for the Conforming Literals proposal starting on 4/22/2024 and ending on 4/29/2024 if all feedback is addressed.

[simondeschenes]

In our studio, it is in the coding standard to always use the f suffix because we were hit by problems in the past where the shaders would use double instead of float, mainly when generating unoptimized shaders with -O0.
There were known problems when you didn't use the f suffix and I am kind of happy to see them solved.

For the integer literals, it is the same thing, we fell in the trap of using literals with RWByteAddressBuffer.Store where it would write 64 bits instead of 32 bits.

Since it happened, we added a coding standard rule where we no longer allow anyone to just put a literal as a parameter to a method or any HLSL function that wasn't written by one of us, just in case. They must now all have a name like:

const uint storeZero =0;
RWByteAddressBuffer.Store(offset, storeZero);

For us, conforming literals would be really appreciated.

[llvm-beanz]

One piece of feedback that came from a user through an outside communication channel was that it would be nice to be able to test this feature in isolation. We agree.

The current plan for rolling out this feature is to include this feature in a DXC release soon (the next month or two). It is currently the only behavior changing HLSL 202x feature (see: 0020 HLSL 202x & 202y for more details about other 202x features being considered).

At the moment the only other 202x feature being considered is 0003 Numeric Constants, which would be an additive feature that does not remove or change functionality.

Because this is the only behavior changing feature currently under consideration we're not adding a separate flag for it, but passing -HV 202x will enable this behavior (and only this behavior) in DXC once the feature lands.

[devshgraphicsprogramming]

Will u, ll, ull, f16, f32, f64 suffices be available?

[devshgraphicsprogramming]

Also in C L suffix on a float means long double which may be one of those 80 bit extended precision things.

GCC implements it as a 128bit float type.
https://godbolt.org/z/nPPre5rx8
and
https://godbolt.org/z/ffa6Y1zeE

So does Clang, only MSVC doesn't.

[llvm-beanz]

Will u, ll, ull, f16, f32, f64 suffices be available?

There is no change to the available suffixes or their intended meanings.

Integer literal suffixes are defined in this PR:
#208

Valid suffixes for integers are u, l, ul, and lu. u denotes unsigned, l denotes 64-bit. The ll, ull and llu suffixes will continue to denote 64-bit for portability and convenience but are not included in the standard. I made that decision because HLSL does not have a long long type, so thell suffix isn't really idiomatic to HLSL, it's more of a Windows-ism that ll means 64-bit (this is unchanged from DXC today).

Floating literal suffixes are defined here:
#175

Valid suffixes for floating literals are h, f, and l. The h suffix denotes a half, the f suffix denotes a float, and the l suffix denotes a double (this is unchanged from DXC today).

[llvm-beanz]

Also in C L suffix on a float means long double which may be one of those 80 bit extended precision things.

GCC implements it as a 128bit float type. https://godbolt.org/z/nPPre5rx8 and https://godbolt.org/z/ffa6Y1zeE

So does Clang, only MSVC doesn't.

The specific sizes of standard integer and floating point types is not defined in the C or C++ specifications. There is a nice writeup about this on CPP Reference.

The Microsoft C++ ABI (MSVC or Clang-CL) adopts LLP64, and other platforms adopt LP64:

• LLP64 or 4/4/8 (int and long are 32-bit, pointer is 64-bit)
• LP64 or 4/8/8 (int is 32-bit, long and pointer are 64-bit)

The behavior of double vs long double is even more complicated because it can vary by target architecture as well as C++ ABI. HLSL has no long double type, so the l suffix on a floating literal just means double.

[tex3d]

While I think this is the right direction, in my opinion there isn't a sufficient discussion of the change in type promotion behavior this entails due to the change in literal rank from the lowest to one matching the new default type. This change could change the operations produced in some cases from min-precision or 16-bit to full 32-bit precision operations when the expression includes an unqualified literal value.

There is one statement that is meant to address this at the end of the spec, but I don't think the full impact is made clear, nor do I think this will always be sufficient to catch subtle behavioral changes that could result in performance losses.

With this change the minimum precision types are lower in conversion rank to all the literal types that can be explicitly specified. This will result in conversion warnings on implicit conversion to minimum precision types which will notify users of the places where their code may need to be updated.

This is relying on a warning when implicitly converting some expression result, likely upon assignment to a min-precision variable. It's not clear that it's indicating operations that might have been silently and unintentionally promoted to higher-precision operations. It won't happen if, for some reason, there is an explicit cast making it unnecessary to insert an implicit down-conversion cast.

If one doesn't fully understand the implications of the warning, they might just insert an explicit cast to silence the warning, not realizing it was evidence that other operations had been promoted to full precision when they might not have meant for that to happen.

Also, you've only pointed this out with regards to min-precision, but it's an issue with native 16-bit types as well. The only difference is that it's possible to add a suffix on the literal to specify the desired 16-bit type, unlike with min-precision.

min16float x,y;
// ...

// Warning on conversion during assignment to 'f' because
// the '*' and '+' expressions have been promoted to 32-bits.
min16float f = x * 0.5 + y;

// Precision of intermediate operations has been changed, but
// no warning because no implicit down-conversion required.
// This change also looks like what the warning suggests
min16float g = (min16float)(x * 0.5 + y);

// The real fix (no way to specify min-precision literal types)
min16float h = x * (min16float)0.5 + y;

Given all this, I would suggest it could be useful to have a special, DefaultIgnore type promotion warning for when lower (rank) precision types are implicitly promoted to a higher (rank) precision (no precision loss). This would be useful for expression operators and promotions in function overload resolution, though less useful for assignments. It would be ideal if this could be tuned to provide the warning only when the promotion came from an undecorated literal.

It would also be useful to list, as an option to be considered, a rewriter mode that can help decorate or cast unqualified literal types to preserve type promotion behavior for these kinds of cases when moving to HLSL 202x. Another option is providing a fix-it with the suggested new promotion warning above.

[tex3d]

A realistic example where no warning is produced when the interpretation has changed would be when performing some number of min-precision or 16-bit precision operations in an expression, then ultimately assigning the result of that expression to a floating point value. The expectation is that all the operations would be performed at the lower precision specified, then the promotion happens silently and automatically at the end. The precision-loss warning will not fire in this case to indicate an area where behavior has changed and code needs to be updated for HLSL 202x.

For an example, you could just change the type of f in my previous comment to float.

[llvm-beanz]

This is relying on a warning when implicitly converting some expression result, likely upon assignment to a min-precision variable. It's not clear that it's indicating operations that might have been silently and unintentionally promoted to higher-precision operations. It won't happen if, for some reason, there is an explicit cast making it unnecessary to insert an implicit down-conversion cast.

We can, should, and do, warn on intermediate expression conversions.

If one doesn't fully understand the implications of the warning, they might just insert an explicit cast to silence the warning, not realizing it was evidence that other operations had been promoted to full precision when they might not have meant for that to happen.

Our users are intelligent and well versed in what they do. We shouldn't assume they won't understand a warning or the implications of it. 15 years ago we used compiler implicit conversion warnings to find performance bottlenecks, I'm not sure why we wouldn't expect people to do that today.

min16float x,y;
// ...

// Warning on conversion during assignment to 'f' because
// the '*' and '+' expressions have been promoted to 32-bits.
min16float f = x * 0.5 + y;

// Precision of intermediate operations has been changed, but
// no warning because no implicit down-conversion required.
// This change also looks like what the warning suggests
min16float g = (min16float)(x * 0.5 + y);

// The real fix (no way to specify min-precision literal types)
min16float h = x * (min16float)0.5 + y;

Given all this, I would suggest it could be useful to have a special, DefaultIgnore type promotion warning for when lower (rank) precision types are implicitly promoted to a higher (rank) precision (no precision loss). This would be useful for expression operators and promotions in function overload resolution, though less useful for assignments. It would be ideal if this could be tuned to provide the warning only when the promotion came from an undecorated literal.

These examples do show a gap in the existing warnings, that we should fill for HLSL (and it broadly useful outside this proposal). We do not warn on floating point promotions, but we should since those can have enormous impact on performance. I see no reason why we should only issue this warning on undecorated literals, this seems like something we should emit for all implicit promotions.

I'll update the spec to add these warning cases.

[tex3d]

We can, should, and do, warn on intermediate expression conversions.

But we don't for promotions, as you point out later:

These examples do show a gap in the existing warnings, that we should fill for HLSL (and it broadly useful outside this proposal). We do not warn on floating point promotions, but we should since those can have enormous impact on performance.

Exactly, my concern was with this gap.

I see no reason why we should only issue this warning on undecorated literals, this seems like something we should emit for all implicit promotions.

While adding the general promotion warning will close this gap, I fear there may be a lot of noise (and difficulty seeing the signal through that noise) if someone attempts to use this warning to find changes in behavior caused by switching to HLSL 202x, if we provide no easy way to separate pre-existing implicit promotions from unintended ones caused by the literal change. You could argue that they should always explicitly cast everywhere, but that is unlikely to be consistently followed in practice - implicit casts exist for a reason.

However, this point is minor, and we could adjust to help detect these cases separately in the future if it becomes a significant problem. One way to do so without any change would be to compare warning output between language versions when enabling this warning. New instances of the warning should be looked at first for potential changes in behavior when migrating language versions. That could be a recommendation in our migration guide.

[tex3d]

If it wasn't clear, I think the current addition to the spec for -Wdouble-promotion warnings sufficiently addresses the concern for a warning.

However, the spec does not directly mention the impact of the change to type promotions in expressions containing literal values. This section implies that the only significant impact is in regards to literal evaluation. I think it's important to explicitly call out the change in rank for undecorated literals, and the expression and function overload type promotion impact this will cause as well.

[llvm-beanz]

However, the spec does not directly mention the impact of the change to type promotions in expressions containing literal values. This section implies that the only significant impact is in regards to literal evaluation. I think it's important to explicitly call out the change in rank for undecorated literals, and the expression and function overload type promotion impact this will cause as well.

Updated in #222