rust-gpu icon indicating copy to clipboard operation
rust-gpu copied to clipboard

Inefficient loop structurization (unnecessarily converts `do`-`while` to `while`).

Open eddyb opened this issue 3 years ago • 0 comments

There's this comment I left in the structurizer (and the do-while->while conversion that follows it): https://github.com/EmbarkStudios/rust-gpu/blob/03f89e8ba6f236218b3c5f9b18fe03c25a4d6a5c/crates/rustc_codegen_spirv/src/linker/structurizer.rs#L307-L310

Which seemed reasonable until I realized that do-while makes more sense for structured control-flow, and SPIR-V special-cases while-like CFGs to even allow them, e.g. RVSDG only has do-while (called θ/"theta").

And another look at OpLoopMerge's docs does confirm it can be applied to an unconditional OpBranch, and combined with conditional breaks out of loops (which SPIR-V also supports), that implies do-while is possible.

As another data point, I looked at what glslang outputs for a simple do-while loop:

               OpBranch %6

          %6 = OpLabel ; loop header
               OpLoopMerge %8 %9 None
               OpBranch %7

          %7 = OpLabel ; loop body
         ; ...
               OpBranch %9

          %9 = OpLabel ; loop condition / "continue" merge
         ; ...
               OpBranchConditional ... %6 %8

          %8 = OpLabel ; loop exit / "break" merge

So that definitely is the kind of do-while-like CFG shape I would've wanted to use when I wrote the structurizer, but I must've assumed the wrong limitation without double-checking.

Cleaning this up should be possible, but I'm not sure what edge cases have to be taken into account (e.g. the whole idea of a "continue" merge is useful for emitting SPIR-V from GLSL, but gets in the way of structured reasoning).

It might be possible that roundtripping through SPIR-T's structured encoding of control-flow will end up replacing the existing structurizer before we get to do any more cleanups like this.

eddyb avatar May 16 '22 01:05 eddyb