val: invalid SPIR-V produced with "-O": block <ID> 37[%37] exits the selection headed by <ID> 11[%11], but not via a structured exit

[paulthomson]

bug_report.zip

Issue found using GraphicsFuzz.

Tool versions:

• glslangValidator commit hash: e383c5f55defdb884a77820483d3360617391d78
• spirv-opt commit hash: 230c9e437146e48ec58adb4433890403c23c98fa

To reproduce:

glslangValidator -V shader.frag -o shader.frag.spv

spirv-opt shader.frag.spv -o temp.spv --validate-after-all -O

The following shader files are included in the attached archive, some of which are also shown inline below:

• 0_glsl/shader.frag
• 1_spirv/shader.frag.spv
• 1_spirv_asm/shader.frag.asm
• 2_spirv_opt/shader.frag.spv

0_glsl/shader.frag:

#version 310 es
precision highp float;

layout(location = 0) out vec4 _GLF_color;

void main()
{
 for(
     int i = 1;
     i < 2;
     ++i
 )
  {
   if(gl_FragCoord.y < 0.0)
    {
     if(gl_FragCoord.x < 0.0)
      {
       _GLF_color = vec4(226.696, 1.0, 1.0, 1.0);
      }
     continue;
    }
   return;
  }
}

1_spirv_asm/shader.frag.asm:

; SPIR-V
; Version: 1.0
; Generator: Khronos Glslang Reference Front End; 7
; Bound: 47
; Schema: 0
               OpCapability Shader
          %1 = OpExtInstImport "GLSL.std.450"
               OpMemoryModel Logical GLSL450
               OpEntryPoint Fragment %4 "main" %22 %39
               OpExecutionMode %4 OriginUpperLeft
               OpSource ESSL 310
               OpName %4 "main"
               OpName %8 "i"
               OpName %22 "gl_FragCoord"
               OpName %39 "_GLF_color"
               OpDecorate %8 RelaxedPrecision
               OpDecorate %15 RelaxedPrecision
               OpDecorate %22 BuiltIn FragCoord
               OpDecorate %39 Location 0
               OpDecorate %45 RelaxedPrecision
               OpDecorate %46 RelaxedPrecision
          %2 = OpTypeVoid
          %3 = OpTypeFunction %2
          %6 = OpTypeInt 32 1
          %7 = OpTypePointer Function %6
          %9 = OpConstant %6 1
         %16 = OpConstant %6 2
         %17 = OpTypeBool
         %19 = OpTypeFloat 32
         %20 = OpTypeVector %19 4
         %21 = OpTypePointer Input %20
         %22 = OpVariable %21 Input
         %23 = OpTypeInt 32 0
         %24 = OpConstant %23 1
         %25 = OpTypePointer Input %19
         %28 = OpConstant %19 0
         %32 = OpConstant %23 0
         %38 = OpTypePointer Output %20
         %39 = OpVariable %38 Output
         %40 = OpConstant %19 226.695999
         %41 = OpConstant %19 1
         %42 = OpConstantComposite %20 %40 %41 %41 %41
          %4 = OpFunction %2 None %3
          %5 = OpLabel
          %8 = OpVariable %7 Function
               OpStore %8 %9
               OpBranch %10
         %10 = OpLabel
               OpLoopMerge %12 %13 None
               OpBranch %14
         %14 = OpLabel
         %15 = OpLoad %6 %8
         %18 = OpSLessThan %17 %15 %16
               OpBranchConditional %18 %11 %12
         %11 = OpLabel
         %26 = OpAccessChain %25 %22 %24
         %27 = OpLoad %19 %26
         %29 = OpFOrdLessThan %17 %27 %28
               OpSelectionMerge %31 None
               OpBranchConditional %29 %30 %31
         %30 = OpLabel
         %33 = OpAccessChain %25 %22 %32
         %34 = OpLoad %19 %33
         %35 = OpFOrdLessThan %17 %34 %28
               OpSelectionMerge %37 None
               OpBranchConditional %35 %36 %37
         %36 = OpLabel
               OpStore %39 %42
               OpBranch %37
         %37 = OpLabel
               OpBranch %13
         %31 = OpLabel
               OpReturn
         %13 = OpLabel
         %45 = OpLoad %6 %8
         %46 = OpIAdd %6 %45 %9
               OpStore %8 %46
               OpBranch %10
         %12 = OpLabel
               OpReturn
               OpFunctionEnd

[s-perron]

In this case, we are merging the continue block for the loop with the block that contains the continue. This moves the continue into the merge construct for the outer selection construct.

I'm not sure what the best fix is yet.

[s-perron]

This is a bigger problem than just continue targets. In general, the algorithm will move the successor block up into the predecessor. This can move code that was outside of a construct into a construct. I'll attach an example where a regular block is merged with a merge block causing what I believe to be a problem.

[s-perron]

2743.zip

[s-perron]

Right now I believe the best way to fix this is to move code from the predecessor into the successor when merging blocks. I need to double check what is valid and what is not.

[greg-lunarg]

What pass is generating the bad code?

[s-perron]

It is in the merge-blocks pass. Alan says the code in 2743.zip is not invalid, so this problem is really just about continues. I cannot convince myself that we can move the code into the predecessor in all cases. I suggest we simply avoid merging when the successor is a continue construct when the predecessor is in a nested construct.

[s-perron]

Correction. We now believe the original problem is a bug in the validator. The continue block should not be considered part of the selection construct. This means the branch to the header is not an exit from the selection construct, but the branch to the continue block. It is a legal exit. Passing to @alan-baker.