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

Expand definition of floating literals #175

Merged

Conversation

llvm-beanz
Copy link
Collaborator

This adds a definition of floating literals that is consistent with existing HLSL documentation and other shader compilers. It is notably different from the implementations in DXC and FXC.

@llvm-beanz llvm-beanz force-pushed the cbieneman/hlsl-floating-literal branch from 3ddac95 to a1caec7 Compare March 20, 2024 18:55
This adds a definition of floating literals that is consistent with
existing HLSL documentation and other shader compilers. It is notably
different from the implementations in DXC and FXC.
@llvm-beanz llvm-beanz force-pushed the cbieneman/hlsl-floating-literal branch from a1caec7 to 752b029 Compare March 20, 2024 19:01
@llvm-beanz
Copy link
Collaborator Author

llvm-beanz commented Mar 20, 2024

A few additional notes for this PR. This adds a definition for floating literals that is:

It is not consistent with DXC's behavior.

DXC's implementation of literal types has been a significant source of bugs in the compiler(microsoft/DirectXShaderCompiler#6147, microsoft/DirectXShaderCompiler#3973, microsoft/DirectXShaderCompiler#4683, microsoft/DirectXShaderCompiler#5493, microsoft/DirectXShaderCompiler#6410, and probably more). It has also caused confusion for even highly technical users, and is probably inconsistent across DXIL and SPIR-V.

llvm-beanz added a commit to llvm-beanz/hlsl-specs that referenced this pull request Mar 22, 2024
This proposal seeks to codfiy the behavior documented in the HLSL
documentation
https://learn.microsoft.com/en-us/windows/win32/direct3dhlsl/dx-graphics-hlsl-appendix-grammar#floating-point-numbers,
and captured in microsoft#175 as the behavior for HLSL 202x.
llvm-beanz added a commit that referenced this pull request Mar 22, 2024
* [0017] Add proposal for conforming literals

This proposal seeks to codfiy the behavior documented in the HLSL
documentation
https://learn.microsoft.com/en-us/windows/win32/direct3dhlsl/dx-graphics-hlsl-appendix-grammar#floating-point-numbers,
and captured in #175 as the behavior for HLSL 202x.
@llvm-beanz
Copy link
Collaborator Author

As an experiment to attempt to gauge the disruption this change would cause. DXC has 40 tests which are fragile to this change (see: llvm-beanz/DirectXShaderCompiler@4c854da). Of those, several of them are fragile because they look for the literal float string, not because they impact correctness or code generation.

I'm trying to run some additional testing against other shaders that I have available to further gauge the potential impact on existing code.

llvm-beanz added a commit to llvm-beanz/llvm-project that referenced this pull request Apr 1, 2024
This change implements the HLSL floating literal suffixes for half and
double literals. The PR for the HLSL language specification for this
behavior is microsoft/hlsl-specs#175.

The TL;DR is that the `h` suffix on floating literals means `half`, and
the `l` suffix means `double`.

The expected behavior and diagnostics are different if native half is
supported. When native half is not enabled half is 32-bit so implicit
conversions to float do not lose precision and do not warn.

In all cases `half` and `float` are distinct types.

Resolves llvm#85712

../clang/test/SemaHLSL/literal_suffixes_no_16bit.hlsl
llvm-beanz added a commit to llvm-beanz/llvm-project that referenced this pull request Apr 5, 2024
This change implements the HLSL floating literal suffixes for half and
double literals. The PR for the HLSL language specification for this
behavior is microsoft/hlsl-specs#175.

The TL;DR is that the `h` suffix on floating literals means `half`, and
the `l` suffix means `double`.

The expected behavior and diagnostics are different if native half is
supported. When native half is not enabled half is 32-bit so implicit
conversions to float do not lose precision and do not warn.

In all cases `half` and `float` are distinct types.

Resolves llvm#85712

../clang/test/SemaHLSL/literal_suffixes_no_16bit.hlsl
llvm-beanz added a commit to llvm/llvm-project that referenced this pull request Apr 5, 2024
This change implements the HLSL floating literal suffixes for half and
double literals. The PR for the HLSL language specification for this
behavior is microsoft/hlsl-specs#175.

The TL;DR is that the `h` suffix on floating literals means `half`, and
the `l` suffix means `double`.

The expected behavior and diagnostics are different if native half is
supported. When native half is not enabled half is 32-bit so implicit
conversions to float do not lose precision and do not warn.

In all cases `half` and `float` are distinct types.

Resolves #85712
@llvm-beanz llvm-beanz merged commit 2efb89b into microsoft:main Apr 30, 2024
3 checks passed
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.

None yet

3 participants