wgpu icon indicating copy to clipboard operation
wgpu copied to clipboard

Naga should map `break if` 1:1 to the backend (i.e. `do`-`while` for GLSL/HLSL/MSL).

Open eddyb opened this issue 1 year ago • 3 comments

Quoting myself from https://github.com/gfx-rs/wgpu/issues/4558:

I do not understand why break if turns into that kind of conditional break.

AIUI it was only added to WGSL to represent SPIR-V conditional backedges. And SPIR-V itself only has conditional backedges to encode do-while loops. So that's the natural translation.

That is, loop { ... continuing { ... break if !(select(false, true, _e23)); } } should turn into do { ... } while(!!(_e23 ? true : false)); - without any overcomplicated propagation across the backedge.

I believe Naga already maps break if 1:1 to WGSL (as expected) and to SPIR-V (as conditional backedges).

So only the C-like backends (GLSL/HLSL/MSL) remain, and they should all allow a 1:1 mapping via do-while.

eddyb avatar Jun 21 '23 19:06 eddyb

The story is a little more complicated than that. WGSL continuing blocks with break if statements do two things:

  • They give you a conditional back edge.

  • They let you put arbitrary code between the target of a continue statement and that back edge.

This is needed specifically to support SPIR-V input, because SPIR-V allows (almost) arbitrary control flow in the continuing block. (This makes it easier to generate SPIR-V that represents a function call inlined into a C-style for loop's third expression.)

But I think this means that back ends cannot always generate do-while loops for Naga Loop statements. If the loop's body block is exited by a Continue statement, and the continuing block can't be represented as a single expression, then that can't be represented as a do-while. Modulo bugs, of course, the more awkward transform we do now handles all the cases.

Of course, it wouldn't be surprising if compilers downstream from Naga generated better code if we could hand them a do-while loop when applicable. That would be an optimization, then.

jimblandy avatar Jun 22 '23 17:06 jimblandy

Ah, I see, I was focusing on continuing {...} without control-flow (or with control-flow that could be handled by ?: expressions alone).

There... is another way, but it's probably worse in some respects.

The insight comes from inlining SPIR-V (which you mention), and how ... (click to open rant)

... it's actually incredibly annoying because SPIR-V is not "structured" as much as it is conservatively approximating "structurally reconvergent" (useful definitions of "structured" tend to be sufficient for inlining by substitution - i.e. break/continue/return are "unstructured non-local exits" which break up structured constructs like if-else/switch/loop, they just happen to be "structurally reconvergent" which is what drivers want - note that Rust-style arbitrary break-with-label would be too).

What spirv-opt does to inline a function is... wrap it in switch(0) { default: ... } and replace returns with breaks.

So, this is one approach that doesn't require propagating a condition across the loop backedge:

loop {
    // ...
    if a { break; }
    // ...
    if b { continue; }
    // ...
    continuing {
        // ...
        break if c;
    }
}

could be turned into:

do {
    bool loop_break = false;
    switch(0) {
        default: // loop body
            // ...
            if (a) { loop_break = true; break; }
            // ...
            if (b) { break; }
            // ...
    }
    if (loop_break) break;

    // continuing block
    // ...
} while(!c);

It should be relatively simple transform, and there's a chance it will optimize better than something that looks like it needs loop state. I suspect it will still be useful to put as much as possible from the continuing {...} block into the condition at the end (esp. if GLSL and HLSL kept expr, expr expressions from C).


Come to think of it, are loops even allowed in continuing {...}? I would expect not, unless infinite loops are UB, because continuing {...} (or at least SPIR-V's concept that WGSL integrated) is required to arrive at the terminator that is the source of the backedge (i.e. the backedge terminator must post-dominate the start of continuing {...}, and unreachable/infinite loops break that. dynamically-potentially-infinite loops might too, but I don't know off the top of my head if they do).

I'm asking because a tree of if-else can be handled as a bunch of nested ?:, if you have expr, expr expressions (since you can use them like Rust block syntax).

eddyb avatar Jul 14 '23 03:07 eddyb

A switch with just a default case isn't handled well in FXC, see https://github.com/gfx-rs/wgpu/issues/4514

So it would probably need to be written as a do {} while(false). I don't know if this negates the potential improvements to optimization chances? A dummy switch case that is never hit might workaround the bug in FXC, I don't think I have observed any issues in this scenario?

Imberflur avatar May 04 '24 18:05 Imberflur