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.

Sign up for GitHub

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

Jump to bottom

Vulkan CTS has failed tests dEQP-VK.glsl.conversions.*to_bvec* at version 4d286aa of SLANG #6466

Closed
zlatinski opened this issue Feb 26, 2025 · 13 comments
Closed

Vulkan CTS has failed tests dEQP-VK.glsl.conversions.*to_bvec* at version 4d286aa of SLANG #6466

zlatinski opened this issue Feb 26, 2025 · 13 comments
Assignees
@zlatinski
Labels
kind:bug something doesn't work like it should
Milestone
Q1 2025 (Winter)

Comments

@zlatinski
Copy link
Contributor

Failed at 4d286aa
Last passed version of slag 187ec44

2025-02-26T07:12:11.3535770Z Test case 'dEQP-VK.glsl.conversions.scalar_to_vector.float_to_bvec2_vertex'..
2025-02-26T07:12:11.5171085Z Fail (Got invalid pixels at sub-case 4)
2025-02-26T07:12:11.5173612Z Test case 'dEQP-VK.glsl.conversions.scalar_to_vector.float_to_bvec2_fragment'..
2025-02-26T07:12:11.6532456Z Fail (Got invalid pixels at sub-case 4)
2025-02-26T07:12:11.6533868Z Test case 'dEQP-VK.glsl.conversions.scalar_to_vector.float_to_bvec3_vertex'..
2025-02-26T07:12:11.7388516Z Fail (Got invalid pixels at sub-case 4)
2025-02-26T07:12:11.7390024Z Test case 'dEQP-VK.glsl.conversions.scalar_to_vector.float_to_bvec3_fragment'..
2025-02-26T07:12:11.8731731Z Fail (Got invalid pixels at sub-case 4)
2025-02-26T07:12:11.8733192Z Test case 'dEQP-VK.glsl.conversions.scalar_to_vector.float_to_bvec4_vertex'..
2025-02-26T07:12:12.0239929Z Fail (Got invalid pixels at sub-case 4)
2025-02-26T07:12:12.0241373Z Test case 'dEQP-VK.glsl.conversions.scalar_to_vector.float_to_bvec4_fragment'..
2025-02-26T07:12:12.1600926Z Fail (Got invalid pixels at sub-case 4)
2025-02-26T07:12:21.4928009Z Test case 'dEQP-VK.glsl.conversions.vector_to_vector.vec4_to_bvec4_vertex'..
2025-02-26T07:12:21.5714246Z Fail (Got invalid pixels at sub-case 0)
2025-02-26T07:12:21.5715672Z Test case 'dEQP-VK.glsl.conversions.vector_to_vector.vec4_to_bvec4_fragment'..
2025-02-26T07:12:21.6984701Z Fail (Got invalid pixels at sub-case 0)
2025-02-26T07:12:21.6986141Z Test case 'dEQP-VK.glsl.conversions.vector_to_vector.vec4_to_bvec3_vertex'..
2025-02-26T07:12:21.7757189Z Fail (Got invalid pixels at sub-case 0)
2025-02-26T07:12:21.7759001Z Test case 'dEQP-VK.glsl.conversions.vector_to_vector.vec4_to_bvec3_fragment'..
2025-02-26T07:12:21.9090453Z Fail (Got invalid pixels at sub-case 0)
2025-02-26T07:12:21.9091915Z Test case 'dEQP-VK.glsl.conversions.vector_to_vector.vec4_to_bvec2_vertex'..
2025-02-26T07:12:21.9863370Z Fail (Got invalid pixels at sub-case 0)
2025-02-26T07:12:21.9864773Z Test case 'dEQP-VK.glsl.conversions.vector_to_vector.vec4_to_bvec2_fragment'..
2025-02-26T07:12:22.1222691Z Fail (Got invalid pixels at sub-case 0)
2025-02-26T07:12:37.0049381Z Test case 'dEQP-VK.glsl.conversions.vector_to_vector.vec2_to_bvec2_vertex'..
2025-02-26T07:12:37.0823086Z Fail (Got invalid pixels at sub-case 0)
2025-02-26T07:12:37.0824651Z Test case 'dEQP-VK.glsl.conversions.vector_to_vector.vec2_to_bvec2_fragment'..
2025-02-26T07:12:37.2095768Z Fail (Got invalid pixels at sub-case 0)
2025-02-26T07:12:40.1072880Z Test case 'dEQP-VK.glsl.conversions.vector_combine.vec2_vec2_to_bvec4_vertex'..
2025-02-26T07:12:40.1625681Z Fail (Got invalid pixels at sub-case 0)
2025-02-26T07:12:40.1627787Z Test case 'dEQP-VK.glsl.conversions.vector_combine.vec2_vec2_to_bvec4_fragment'..
2025-02-26T07:12:40.2898441Z Fail (Got invalid pixels at sub-case 0)
2025-02-26T07:12:44.5315078Z Test case 'dEQP-VK.glsl.conversions.vector_combine.vec2_ivec2_to_bvec4_vertex'..
2025-02-26T07:12:44.5845151Z Fail (Got invalid pixels at sub-case 0)
2025-02-26T07:12:44.5846593Z Test case 'dEQP-VK.glsl.conversions.vector_combine.vec2_ivec2_to_bvec4_fragment'..
2025-02-26T07:12:44.6896144Z Fail (Got invalid pixels at sub-case 0)
2025-02-26T07:12:45.0443722Z Test case 'dEQP-VK.glsl.conversions.vector_combine.vec2_bvec2_to_bvec4_vertex'..
2025-02-26T07:12:45.0980805Z Fail (Got invalid pixels at sub-case 0)
2025-02-26T07:12:45.0983251Z Test case 'dEQP-VK.glsl.conversions.vector_combine.vec2_bvec2_to_bvec4_fragment'..
2025-02-26T07:12:45.2005462Z Fail (Got invalid pixels at sub-case 0)
2025-02-26T07:12:46.1564666Z Test case 'dEQP-VK.glsl.conversions.vector_combine.vec3_float_to_bvec4_vertex'..
2025-02-26T07:12:46.2330363Z Fail (Got invalid pixels at sub-case 1)
2025-02-26T07:12:46.2332211Z Test case 'dEQP-VK.glsl.conversions.vector_combine.vec3_float_to_bvec4_fragment'..
2025-02-26T07:12:46.3589818Z Fail (Got invalid pixels at sub-case 1)
2025-02-26T07:12:54.0154945Z Test case 'dEQP-VK.glsl.conversions.vector_combine.vec2_bool_to_bvec3_vertex'..
2025-02-26T07:12:54.0689288Z Fail (Got invalid pixels at sub-case 1)
2025-02-26T07:12:54.0690749Z Test case 'dEQP-VK.glsl.conversions.vector_combine.vec2_bool_to_bvec3_fragment'..
2025-02-26T07:12:54.1942932Z Fail (Got invalid pixels at sub-case 1)
@zlatinski zlatinski added the kind:bug something doesn't work like it should label Feb 26, 2025
@zlatinski zlatinski added this to the Q1 2025 (Winter) milestone Feb 26, 2025
@zlatinski zlatinski self-assigned this Feb 26, 2025
@csyonghe
Copy link
Collaborator

I think 19867ff is likely the offending commit.

@csyonghe
Copy link
Collaborator

My guess is that PR accidentally deleted conversion operator to convert between bool and float vectors. We should try compile the glsl in the test and see what is happening.

@zlatinski
Copy link
Contributor Author

It is 19867ff, indeed:
Simplify implicit cast ctors for vector & matrix. (#6408)

@zlatinski
Copy link
Contributor Author

This is the IR emitted before this change:

	let  %42	: Float	= load(%30)
	let  %43	: Bool	= castFloatToInt(%42)
	let  %44	: Vec(Bool, 2 : Int)	= MakeVectorFromScalar(%43)
	store(%out0_, %44)
	let  %45	: Vec(Float, 4 : Int)	= load(%31)
	store(%32, %45)
	let  %46	: Vec(Bool, 2 : Int)	= load(%out0_)
	let  %47	: Vec(UInt, 2 : Int)	= intCast(%46)

And after:

	let  %42	: Float	= load(%30)
	let  %43	: Vec(Float, 2 : Int)	= MakeVectorFromScalar(%42)
	let  %44	: Vec(Int, 2 : Int)	= castFloatToInt(%43)
	let  %45	: Vec(Bool, 2 : Int)	= intCast(%44)
	store(%out0_, %45)
	let  %46	: Vec(Float, 4 : Int)	= load(%31)
	store(%32, %46)
	let  %47	: Vec(Bool, 2 : Int)	= load(%out0_)
	let  %48	: Vec(UInt, 2 : Int)	= intCast(%47)

I'm also attaching the GLSL shader, SPIR-V and the complete IR.

shader_1_vertex-good.zip

shader_1_vertex-bad.zip

@csyonghe
Copy link
Collaborator

The two code here is equivalent. Why do we need to care the IR and SPIRV, if the result from running the code is the same?

@zlatinski
Copy link
Contributor Author

zlatinski commented Feb 28, 2025 •
edited
Loading

They are not equivalent. To get a better context, please refer to the archived IR and SPIR-V.

@csyonghe
Copy link
Collaborator

I looked at the SPIRV and IR. Why is it not equivalent?

@csyonghe
Copy link
Collaborator

In what situation will the invariant cast_float2_to_int2(make_float2_from_scalar(1.0)) == make_int2_from_scalar(cast_float_to_int(1.0)) no longer hold?

@zlatinski
Copy link
Contributor Author

zlatinski commented Feb 28, 2025 •
edited
Loading

Aha, I see what you mean. I thought I had posted the same files.

I guess the issue is at the SPIR-V site:

The good:

         %45 = OpConstantComposite %v2uint %uint_0 %uint_0
         %46 = OpConstantComposite %v2uint %uint_1 %uint_1
       %main = OpFunction %void None %3
          %4 = OpLabel
          %6 = OpLoad %float %in0
         **%10 = OpFUnordNotEqual %bool %6 %float_0**
         %13 = OpCompositeConstruct %v2bool %10 %10
         %18 = OpLoad %v4float %dEQP_Position
         %31 = OpSelect %v2uint %13 %46 %45
               OpStore %gl_Position %18
               OpStore %entryPointParam_main_v_out0 %31
               OpReturn
               OpFunctionEnd

The bad:

         %50 = OpConstantComposite %v2int %int_0 %int_0
         %51 = OpConstantComposite %v2uint %uint_0 %uint_0
         %52 = OpConstantComposite %v2uint %uint_1 %uint_1
       %main = OpFunction %void None %3
          %4 = OpLabel
          %6 = OpLoad %float %in0
         %10 = OpCompositeConstruct %v2float %6 %6
         %13 = OpConvertFToS %v2int %10
         **%18 = OpINotEqual %v2bool %13 %50**
         %23 = OpLoad %v4float %dEQP_Position
         %36 = OpSelect %v2uint %18 %52 %51
               OpStore %gl_Position %23
               OpStore %entryPointParam_main_v_out0 %36

@zlatinski
Copy link
Contributor Author

zlatinski commented Feb 28, 2025 •
edited
Loading

The original one (the good):
%6 = OpLoad %float %in0
%10 = OpFUnordNotEqual %bool %6 %float_0
%13 = OpCompositeConstruct %v2bool %10 %10

  • Loads the float scalar
  • Compares it with zero using OpFUnordNotEqual to get a boolean
  • Constructs a boolean vector from the scalar boolean

The bad one:
%6 = OpLoad %float %in0
%10 = OpCompositeConstruct %v2float %6 %6
%13 = OpConvertFToS %v2int %10
%18 = OpINotEqual %v2bool %13 %50

  • Loads the float scalar
  • Constructs a float vector
  • Converts the float vector to an integer vector using OpConvertFToS
  • Compares the integer vector with zero using OpINotEqual

According to the GLSL specification, when converting from floating-point values to boolean values, the conversion should be done by comparing the float value with 0.0. A float is converted to true if it's non-zero, and false if it's zero.
The GLSL spec (version 4.50, section 4.1.10 "Implicit Conversions") states:

When converting from float or double to bool, the result is false if the float or double operand is 0.0 or 0.0f, respectively, and true otherwise.
The correct implementation in the reference SPIR-V uses OpFUnordNotEqual to directly compare the float with zero, which properly implements this specification requirement.
The problematic implementation first converts the float to an integer using OpConvertFToS and then compares that integer with zero. This introduces a different semantic:

Float-to-int conversion truncates the fractional part
Small non-zero values between -1.0 and 1.0 might become 0 when converted to int
This means values like 0.5 or -0.3 would incorrectly convert to false instead of true
For example, a float value of 0.5 should convert to true according to the GLSL spec (since it's non-zero), but with the problematic implementation:

  • 0.5 gets converted to int 0
  • int 0 is compared with 0, resulting in false

This violates the GLSL specification and would cause incorrect behavior in shaders that rely on proper boolean conversion semantics.

I'm still doing more SLANG debugging on this.

@csyonghe
Copy link
Collaborator

OK, we need to fix our implementation in peephole-optimize pass when lowering BuiltinCast.

@csyonghe
Copy link
Collaborator

#6497 is a potential fix for this. I am doing this in a hurry to get this into the next VulkanSDK release.

@zlatinski
Copy link
Contributor Author

Thank you for the fix, @csyonghe!!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@zlatinski zlatinski

Labels
kind:bug something doesn't work like it should
Projects
None yet
Milestone
Q1 2025 (Winter)
Development

No branches or pull requests
2 participants
@zlatinski @csyonghe