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

Add Integer Literals chapter #208

Merged
merged 2 commits into from
May 2, 2024

Conversation

llvm-beanz
Copy link
Collaborator

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

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
Copy link
Collaborator Author

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

specs/language/lex.tex Outdated Show resolved Hide resolved
@llvm-beanz
Copy link
Collaborator Author

Updated rendering from the latest doc push:

Screenshot 2024-04-30 at 3 58 50 PM

@farzonl
Copy link
Collaborator

farzonl commented May 1, 2024

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
Copy link
Collaborator Author

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
Copy link
Collaborator

tex3d commented May 1, 2024

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
Copy link
Collaborator Author

llvm-beanz commented May 2, 2024

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.

@llvm-beanz llvm-beanz merged commit d15b754 into microsoft:main May 2, 2024
3 checks passed
@devshgraphicsprogramming

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
Copy link
Collaborator Author

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 #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

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
Copy link
Collaborator Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants