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

[SPIR-V] Typed Enum Implicit Conversions Fail #6884

Open
xantos117 opened this issue Aug 28, 2024 · 3 comments · May be fixed by #6977
Open

[SPIR-V] Typed Enum Implicit Conversions Fail #6884

xantos117 opened this issue Aug 28, 2024 · 3 comments · May be fixed by #6977
Assignees
Labels
bug Bug, regression, crash spirv Work related to SPIR-V
Milestone

Comments

@xantos117
Copy link

xantos117 commented Aug 28, 2024

Description
When using typed enums in math operations, they can completely fail to resolve to correct values. Casting the enum to its type in-place can resolve the issue.

Steps to Reproduce

  1. Create a typed enum, in our case an enum : int { }.
  2. Use that enum in a floating-point operation.
  3. Observe that the enum value is now only ever compiled to 0.

Actual Behavior
Shader playground minimal example of failure here.

enum : int
{
	//Hottest star is <50000K and the coldest star is >600K
	STAR_MAX_TEMPERATURE = 50000,
	STAR_MIN_TEMPERATURE = 600,
 	STAR_TEMPERATURE_RANGE = (STAR_MAX_TEMPERATURE - STAR_MIN_TEMPERATURE),
};

...

float x = 3.0;
buffer[threadId] = STAR_TEMPERATURE_RANGE * x;

compiles to

%int_0 = OpConstant %int 0

Instead of the expected

%int_148200 = OpConstant %int 148200

Shader playground minimal example of success here.

Environment

  • DXC version trunk (I don't see one clearly on shader-playground, but we're targeting cs_6_6, and are using 1.8 locally)
  • Host Operating System: Ubuntu 22.04 LTS, or whatever shader-playground runs on (also probably linux, but I don't know).
@xantos117 xantos117 added bug Bug, regression, crash needs-triage Awaiting triage spirv Work related to SPIR-V labels Aug 28, 2024
@damyanp damyanp removed the needs-triage Awaiting triage label Aug 29, 2024
@damyanp damyanp moved this to Triaged in HLSL Triage Aug 29, 2024
@damyanp damyanp moved this from Triaged to For Google in HLSL Triage Aug 29, 2024
@s-perron
Copy link
Collaborator

The initial code generated by DXC looks correct: https://godbolt.org/z/onr1n3boP. The optimizer must be doing something wrong.

@s-perron
Copy link
Collaborator

s-perron commented Sep 16, 2024

My last comment is wrong. DXC is generating an incorrect instruction:

         %51 = OpBitcast %float %50

This should be an OpConvertUToF and not a bitcast. The bitcast of 49,600 results in subnormal, which can be flushed to 0.

@s-perron s-perron added this to the Next Release milestone Sep 16, 2024
@s-perron s-perron moved this from For Google to Triaged in HLSL Triage Sep 16, 2024
@Keenuts
Copy link
Collaborator

Keenuts commented Oct 21, 2024

This should be an OpConvertUToF and not a bitcast. The bitcast of 49,600 results in subnormal, which can be flushed to 0.

Yep, seems like this is the reason. Looking into it.

Keenuts added a commit to Keenuts/DirectXShaderCompiler that referenced this issue Oct 21, 2024
An implicit cast from int enum to a float should be a Floating_Integral
cast, not an Integral_Conversion.

Fixes microsoft#6884

Signed-off-by: Nathan Gauër <[email protected]>
@Keenuts Keenuts linked a pull request Oct 21, 2024 that will close this issue
Keenuts added a commit to Keenuts/DirectXShaderCompiler that referenced this issue Nov 12, 2024
An implicit cast from int enum to a float should be a Floating_Integral
cast, not an Integral_Conversion.

Fixes microsoft#6884

Signed-off-by: Nathan Gauër <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug, regression, crash spirv Work related to SPIR-V
Projects
Status: New
Status: Triaged
Development

Successfully merging a pull request may close this issue.

4 participants