microsoft / hlsl-specs

HLSL Specifications
MIT License
118 stars 30 forks source link

Add Integer Literals chapter #208

Closed llvm-beanz closed 4 months ago

llvm-beanz commented 4 months ago

Adds a new chapter to describe the integer literal behavior as conforming to C. This documents the feature as proposed in:

https://github.com/microsoft/hlsl-specs/blob/main/proposals/0017-conforming-literals.md

llvm-beanz commented 4 months ago

Here's an image of the complicated part of the rendered text. This covers everything except the grammar bits that aren't too hard to read in the source:

Screenshot 2024-04-30 at 11 43 09 AM
llvm-beanz commented 4 months ago

Updated rendering from the latest doc push:

Screenshot 2024-04-30 at 3 58 50 PM
farzonl commented 4 months ago

im coming a bit late to this, is there a reason why we don't document short\int16_t or ushort\uint16_t here?

llvm-beanz commented 4 months ago

im coming a bit late to this, is there a reason why we don't document short\int16_t or ushort\uint16_t here?

There is no literal suffix for integers smaller than int, and this is just about the literal handling. short and ushort aren't valid keywords in HLSL, we do have int16_t and uint16_t which are documented in [Basic.types].

tex3d commented 4 months ago

There is no literal suffix for integers smaller than int, and this is just about the literal handling. short and ushort aren't valid keywords in HLSL, we do have int16_t and uint16_t which are documented in [Basic.types].

Is there a precedent for a literal suffix for these types? Can we adopt something? Otherwise, when adapting code to comply with the conforming literals language changes in HLSL 202x, code like i + 1 where i is int16_t would have to be adapted in an awkward way to i + (int16_t)1.

llvm-beanz commented 4 months ago

Is there a precedent for a literal suffix for these types?

There is no precedent in C. This is probably because C basically implicitly converts all integer types smaller than int to int before all operations (yay Usual Arithmetic Conversions).

Can we adopt something? Otherwise, when adapting code to comply with the conforming literals language changes in HLSL 202x, code like i + 1 where i is int16_t would have to be adapted in an awkward way to i + (int16_t)1.

We could come up with something ourselves, but I actually think it is better to not create something non-conformant here. Maybe it makes sense to allow literals to start as 16-bit types, and scale up because implicit promotions are cheap. I’m unconvinced we have a good enough reason to deviate from C/C++ here.

devshgraphicsprogramming commented 4 weeks ago

There is no literal suffix for integers smaller than int, and this is just about the literal handling. short and ushort aren't valid keywords in HLSL, we do have int16_t and uint16_t which are documented in [Basic.types].

Is there a precedent for a literal suffix for these types? Can we adopt something? Otherwise, when adapting code to comply with the conforming literals language changes in HLSL 202x, code like i + 1 where i is int16_t would have to be adapted in an awkward way to i + (int16_t)1.

An implementation of https://en.cppreference.com/w/cpp/language/user_literal would make people's lives somewhat bearable.

I could make my own _s16 or _u16 for shorts.

We could come up with something ourselves, but I actually think it is better to not create something non-conformant here. Maybe it makes sense to allow literals to start as 16-bit types, and scale up because implicit promotions are cheap. I’m unconvinced we have a good enough reason to deviate from C/C++ here.

What was the issue with allowing the int16_t,uint16_t at the front of the precedence list of int32_t, uint32_t, int64_t, uint64_t when -enable-16bit-types flag is present?

llvm-beanz commented 3 weeks ago

An implementation of https://en.cppreference.com/w/cpp/language/user_literal would make people's lives somewhat bearable.

I could make my own _s16 or _u16 for shorts.

Feel free to file a separate feature request, however due to how DXC implements overload resolution I'm not confident these can be implemented in DXC, so it may need to be a 202y feature instead of 202x.

What was the issue with allowing the int16_t,uint16_t at the front of the precedence list of int32_t, uint32_t, int64_t, uint64_t when -enable-16bit-types flag is present?

What is the expected behavior of 1 << 16 ? It seems to me that a compiler flag shouldn't change the result of that operation, but with your proposal this gets really interesting... Under HLSL's rules if 1 is a short the result is 1. Pretty sure all the places we see this in code (or similar things) today they expect the result to be 65536. So the proposal you've made would likely break a lot more code.

I don't really understand your concern here. In https://github.com/microsoft/hlsl-specs/issues/304, you're saying we didn't align our float literal behavior with C/C++ and that causes problems, but here you're suggesting we shouldn't have aligned our integer behavior with C/C++?

We consciously decided to deviate from C/C++ in the floating point behavior due to the computational cost of double and an assessment of how this change would impact existing code. It also has the benefit of aligning with GLSL and other shader languages, so it will be familiar to shader authors. We implemented additional warnings to help identify problematic floating point promotions too so that users can be aware of how the change impacts their code.

We choose to align with C/C++ for integers because it is what programmers expect, and the compiler is very capable of providing implicit cast warnings to help find odd cases.

I don't know that we're going to make you happy since the feedback you're giving us is self-inconsistent. It seems like you want us to align the language with how you want it to work rather than aligning based on C/C++ or existing usage.

devshgraphicsprogramming commented 3 weeks ago

so why doesn't a C++ compiler give me a lot of warnings about uint32_t being casted to uint16_t when DXC does?

I don't know that we're going to make you happy since the feedback you're giving us is self-inconsistent. It seems like you want us to align the language with how you want it to work rather than aligning based on C/C++ or existing usage.

You're right. Sorry my bad.

llvm-beanz commented 3 weeks ago

so why doesn't a C++ compiler give me a lot of warnings about uint32_t being casted to uint16_t when DXC does?

C/C++ has the most hated conversion feature ever usual arithmetic conversions. Basically all integer values smaller than int are promoted to int or unsigned int implicitly following the integer promotion rules, and more rules identify the common type from there.

Because this means a whole lot of extra conversions for shorts, most C/C++ compilers don't warn on conversions that are caused by the usual arithmetic conversions.

HLSL does not have usual arithmetic conversions because that would be awful for performance on SIMD processors.