-
Notifications
You must be signed in to change notification settings - Fork 263
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
Change SPIR-V lowering of switch
with non-trivial fallthrough to avoid undefined reconvergence behavior
#6441
Comments
If SPIRV doesn't provide a well-defined semantics for fallthrough cases, we will have to leave the semantics of We could probably improve this by duplicating SPIRV code for cases that has fallthrough, but we will not be lowering all switch to |
Developers have an intuitive notion that a C-like Our default disposition for Slang is currently to map high-level-language constructs to their "natural" equivalent in a target language whenever that is possible/practical. This is especially true when it is possible that using the natural/obvious mapping might afford compilers downstream from Slang (e.g., in GPU drivers) more opportunities for optimization, or enable those compilers to do less work in discovering opportunities for such optimization. Of course we also need to be concerned about giving developers enough of a foundation that they can write portable code that is also semantically meaningful. If there is a significant body of evidence that the performance cost of mapping For now, the intended divergence/reconvergence guarantees for |
You all get to make decisions how you see fit, but this logic seems confused to me.
We polled users and found that overwhelmingly they assume switch statements reconverge at fallthrough. I don't think most developers writing shaders are experts at either SPIRV or DXIL, certainly not to the degree to assume that This will be an area of difference between HLSL and Slang going forward since HLSL is giving users a defined behavior for this case. |
I tried @llvm-beanz's test shader and Slang is producing invalid SPIRV. We have more fundamental issues regarding fallthrough switch support. The latest thinking is that we can continue to lower all switches without fallthrough as |
Basically, for
we can lower into something like:
|
The lowering we intend to implement in Clang is something like the example below. switch(selector) {
default:
something();
case 1:
case 3:
somethingElse();
break;
case 2:
anotherThing();
break;
} It should be accurate to expand it to: do {
if (!(selector == 1 && selector == 2 && selector == 3)) {
something();
selector = 1; // This case falls through so we now need to execute case 1.
}
if (selector == 1) {
somethingElse();
break; // breaks can still be breaks;
}
if (selector == 2) {
anotherThing();
break;
}
} while(false) This has the advantage of generating deterministic reconvergence. Generally the optimizations that a backend could perform with |
For the example in llvm/llvm-project#112056:
We can lower as:
|
OpSwitch has well defined reconvergence behavior when there is only trivial fallthrough, so it may still be the better option here. The reason to prefer switch is that it enables O(1) implementation of branching. Consider the case where you have 100+ or even 1000+ cases, |
@csyonghe I don't think your example above is correct, although I agree with you on the basic approach that can be taken. Given your input:
I believe the correct output is:
(And then the redundant |
@llvm-beanz In your example expansion, introducing the |
Yes, to avoid using In either case we believe that the complexity in the front end is required to deterministically meet user expectations for behavior. |
Yong and I have discussed this issue offline and come to the conclusion that Slang should change its lowering of We will still need to perform a more thorough audit of all of the targets that Slang supports to determine what, if any, guarantees we are able to provide from the high-level-language compiler about divergence/reconvergence behavior across targets. |
switch
statements with fallthrough?switch
with non-trivial fallthrough to avoid undefined reconvergence behavior
Given this shader:
Compiler Explorer
Slang currently generates the
switch
statement as anOpSwitch
instruction which has undefined behavior for reconvergence of cases with fall through.Is this expected to be undefined behavior in Slang?
Additional Context from HLSL
The HLSL draft specification states in section [Intro.Model.Restrictions]:
This language is specifically intended to disallow restructuring of control flow and to force reconvergence at the earliest possible point.
We're tracking an alternate SPIR-V lowering for
switch
statements in llvm/llvm-project#112056.The text was updated successfully, but these errors were encountered: