Naga SPIR-V front end generates lousy code for out-of-scope expressions

[jimblandy]

The Naga SPIR-V front end generates unnecessary temporary variables and spills for SPIR-V values referenced outside of their natural Naga IR scope.

As explained in the documentation for naga::front::spv::BlockContext:

    /// - A SPIR-V expression can be used in any SPIR-V block dominated by its
    ///   definition, whereas Naga expressions are scoped to the rest of their
    ///   subtree. This means that discovering an expression use later in the
    ///   function retroactively requires us to have spilled that expression into a
    ///   local variable back before we left its scope. (The docs for
    ///   [`Frontend::get_expr_handle`] explain this in more detail.)

Once we've generated a temporary variable and spilled the value to it, subsequent references to that expression have no need to introduce another temporary: they should just use the one that's already there. For example:

               OpCapability Shader
               OpCapability Linkage
               OpMemoryModel Logical GLSL450
       %uint = OpTypeInt 32 0
     %uint_1 = OpConstant %uint 1
     %uint_2 = OpConstant %uint 2
     %uint_3 = OpConstant %uint 3
   %ptr_uint = OpTypePointer Private %uint
    %fn_uint = OpTypeFunction %uint
          %v = OpVariable %ptr_uint Private
          %f = OpFunction %uint None %fn_uint
   %prologue = OpLabel
               OpBranch %top
        %top = OpLabel
               OpLoopMerge %merge %continue None
               OpBranch %body
       %body = OpLabel
       %load = OpLoad %uint %v
               OpBranch %merge
   %continue = OpLabel
               OpBranch %top
      %merge = OpLabel
       %sum1 = OpIAdd %uint %load %uint_1
       %sum2 = OpIAdd %uint %load %uint_2
       %sum3 = OpIAdd %uint %load %uint_3
      %sum12 = OpIAdd %uint %sum1 %sum2
     %sum123 = OpIAdd %uint %sum12 %sum3
               OpReturnValue %sum123
               OpFunctionEnd

For the above SPIR-V, Naga generates the following WGSL:

var<private> global: u32;

fn function() -> u32 {
    var local: u32;
    var local_1: u32;
    var local_2: u32;

    loop {
        let _e4 = global;
        local = _e4;
        local_1 = _e4;
        local_2 = _e4;
        break;
    }
    let _e6 = local;
    let _e9 = local_1;
    let _e12 = local_2;
    return (((_e6 + 1u) + (_e9 + 2u)) + (_e12 + 3u));
}

While it is certainly necessary to spill _e4 somewhere, so that the Naga Expression::Binary Add expressions following the loop can have something to refer to, it is certainly not necessary to introduce a separate temporary for every reference to _e4. We don't need all of local, local_1, and local_2; one would suffice.

This can probably be fixed in naga::front::spv::Frontend::get_expr_handle, by having it record the temporary variables it generates somewhere easy to find - perhaps in LookupExpression. Then subsequent calls could realize that we've already spilled the value, and re-use the temporary we introduced at the time. Only the call that actually introduced the variable would contribute an entry to BlockContext::phis.

For extra points, we could share Load expressions. But it's probably fine to have one Load per use; that's what we do for ordinary variables anyway.