Skip to content
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

Incorrect optimization of Phi nodes in SPIR-V #5815

Open
mitdemverkaufer opened this issue Sep 22, 2024 · 1 comment
Open

Incorrect optimization of Phi nodes in SPIR-V #5815

mitdemverkaufer opened this issue Sep 22, 2024 · 1 comment

Comments

@mitdemverkaufer
Copy link

The spirv-opt misoptimizes a series of Phi nodes and causes incorrect constant folding results.

In the following shader program:

#version 310 es

precision highp float;
precision highp int;

layout(location = 0) out vec4 color;

layout(binding = 0) uniform Uniform {
    int input1; // input1 = 1
};

bool b() {
  uint c = 0u;
  return bool(c);
}
bool d(int, int) {
  uint e = 0u;
  return bool(e);
}

void main() {
  int c = 0;
  int g = 0;
  int k = 0;
  for (int i = 0; i < input1; i++)   // input1 = 1, iterate only once
    if (b()) {    // b() is false
      if (d(int(gl_FragCoord), int(gl_FragCoord)))
        continue;
      break;
    } else {
      k = c;      // k = 0
      for (int j = 0; j < 2; j++)
        g = 1;    // g = 1
      k = g | k;  // k = 1
    }
  color = vec4(vec3(k), 1.0); // color = (1, 1, 1, 1)
}

The input1 is set to 1 during runtime. The program contains a series of control-flow structures. The outer loop should only iterate once since input = 1. The first if-statement should evaluate to false, thus the else branch is executed. The variable k is set to be 1 in the statement k = g | k since g is 1. As such, the final color should be white.

However, the spirv-opt misoptimizes the shader and the final color is black.

Steps to reproduce

I provide a minimal Vulkan application that runs the give shader program. Simply follow the instruction in the README.md in the package. It should take less than a minute to reproduce the issue. I also provide the non-optimized, optimized, and the disassembled code from the optimized SPIR-V binary in the package.

@s-perron
Copy link
Collaborator

The problem is the ssa-rewrite pass. It is an odd error with a very particular control flow.

This is the reduced spir-v, and it can be reproduced by running the ssa-rewrite pass:

Command:

spirv-opt t.spv -o o.spv --ssa-rewrite

Input:

               OpCapability Shader
               OpMemoryModel Logical GLSL450
               OpEntryPoint Fragment %main "main"
               OpExecutionMode %main OriginUpperLeft
               OpSource ESSL 310
               OpName %main "main"
               OpName %k "k"
               OpName %i "i"
               OpName %j "j"
       %void = OpTypeVoid
          %6 = OpTypeFunction %void
       %bool = OpTypeBool
      %false = OpConstantFalse %bool
        %int = OpTypeInt 32 1
%_ptr_Function_int = OpTypePointer Function %int
      %int_0 = OpConstant %int 0
      %float = OpTypeFloat 32
      %int_1 = OpConstant %int 1
       %main = OpFunction %void None %6
         %14 = OpLabel
          %k = OpVariable %_ptr_Function_int Function
          %i = OpVariable %_ptr_Function_int Function
          %j = OpVariable %_ptr_Function_int Function
               OpStore %k %int_0
               OpStore %i %int_0
               OpBranch %15
         %15 = OpLabel
               OpLoopMerge %16 %17 None
               OpBranch %18
         %18 = OpLabel
         %19 = OpLoad %int %i
         %20 = OpSLessThan %bool %19 %int_1
               OpBranchConditional %20 %21 %16
         %21 = OpLabel
               OpSelectionMerge %22 None
               OpBranchConditional %false %23 %24
         %23 = OpLabel
               OpBranchConditional %false %17 %16
         %24 = OpLabel
               OpStore %k %int_0
               OpStore %j %int_0
               OpBranch %25
         %25 = OpLabel
         %26 = OpLoad %int %j
         %27 = OpSLessThan %bool %26 %int_1
               OpLoopMerge %28 %29 None
               OpBranchConditional %27 %29 %28
         %29 = OpLabel
         %30 = OpLoad %int %j
         %31 = OpIAdd %int %30 %int_1
               OpStore %j %31
               OpBranch %25
         %28 = OpLabel
          ; If this load is removed, the result is correct.
         %32 = OpLoad %int %k
               OpStore %k %int_1
               OpBranch %22
         %22 = OpLabel
               OpBranch %17
         %17 = OpLabel
         %33 = OpLoad %int %i
         %34 = OpIAdd %int %33 %int_1
               OpStore %i %34
               OpBranch %15
         %16 = OpLabel
         %35 = OpLoad %int %k
         %36 = OpConvertSToF %float %35
               OpReturn
               OpFunctionEnd

Output:

%41 = OpPhi %int %int_0 %14 %44 %17 ; At the start of the outer loop. This is correct.
...
; In the continue for the loop. The %int_0 is incorrect, It should be %int_1 when coming 
; from block %22.
%44 = OpPhi %int %41 %23 %int_0 %22 
...
%36 = OpConvertSToF %float %41

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants