-
Notifications
You must be signed in to change notification settings - Fork 211
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
slang-test: Some tests show up as passing even though they print out error messages #5291
Comments
@csyonghe This is pretty worrying. I hope it's not some issue with slang-test itself! |
I added |
The first several cases did not repro anything other than a warning for me, but I did repro with texture2d-gather.hlsl
|
The following wgsl source is passed into wgpuDeviceCreateShaderModule(), and despite producing errors on console, it returns a non-null pointer which is treated as success by slang-rhi:
This error output shows up:
|
The error itself can be seen in isolation using this website: https://shader-playground.timjones.io/04ab99f50368049943c2adcf0ec696c8 But the issue is the lack of an error code coming out of wgpuDeviceCreateShaderModule, which seems to be returning success according to the non-null return. I guess that's a Dawn issue, or a user error, not sure yet which. |
Let's not worry about fixing this issue for now, and instead we should focus on addressing the invalid wgsl output by Slang. |
Thanks for getting to the bottom of this, @cheneym2. Given that we know this is not a bug in our stack, I think we should close this issue. |
Yes I am working on the error with tests/bugs/texture2d-gather.hlsl Is it only test printing error message? |
WebGPU uses an error callback to report errors. At the moment we just print them out. |
I think we need to propagate the error seen in the callback, as Anders mentioned and not close the issue until then. |
shader-slang/slang-rhi#95 would make the failed compilation at least return SLANG_FAIL, which is faithfully recorded as a failed error code by the test harness, however, being an HLSL comparison test, the fact that the HLSL and Slang paths both return the same compilation error, texture2d-gather.hlsl still passes. |
The inner test in spawnAndWaitSharedLibrary sees an error code, storing it in ExecuteResult outRes.errorCode doRenderComparisonTestRun merely builds the error code into an output string, and returns TestResult::Pass. The test is an overall match success. |
The test should definitely propagate any error code and return Fail when shader compilation failed. |
Testing https://github.com/shader-slang/slang/pull/5485/files#diff-45451d657441da687eb40563c21b977d64ba42d643fd5783a1bc937b3eac62b3 as a possible fix, though I assume it will also require disabling newly failing tests once I see which they are. |
tests/language-feature/shader-params/entry-point-uniform-params-implicit.slang
tests/metal/groupshared-threadlocal-same-parameter.slang
tests/bugs/gh-2959.slang
tests/bugs/gh-471.slang
tests/bugs/static-var.slang
tests/bugs/texture2d-gather.hlsl
tests/language-feature/anonymous-struct.slang
tests/language-feature/constants/static-const-in-generic-interface.slang
tests/language-feature/enums/nested-enum.slang
tests/language-feature/enums/strongly-typed-id.slang
tests/language-feature/generics/variadic-void.slang
tests/language-feature/generics/variadic-0.slang
tests/language-feature/generics/tuple.slang
tests/language-feature/generics/irwarray.slang
Example:
The text was updated successfully, but these errors were encountered: