should unreachable nested control flow be treated differently than reachable nested control flow?

[afd]

This archive contains a fragment shader, shader.frag, with an unreachable switch statement. The SPIR-V produced by glslangValidator 7.10.2904 (included in the archive in binary and disassembled form) has a corresponding unreachable switch statement. My understanding is, however, that glslangValidator should eliminate this unreachable SPIR-V at generation time.

Issue found using GraphicsFuzz.

[johnkslang]

It is valid for SPIR-V for Vulkan to contain unreachable code, so any issue must be more subtle than simply not eliminating unreachable code.

[alan-baker]

This issue was spawned from https://github.com/KhronosGroup/SPIRV-Tools/issues/1960.

There is an unreachable selection that branches to a block that breaks from the loop. If the selection header had been reachable, CFG would have been ok. With the unreachable selection header, the selection construct consists of only the header so that exit from the construct is invalid.

[dneto0]

The enclosed fragment shader doesn't have the loop we were seeing in the SPIRV-Tools issue.

[johnkslang]

It will be best to just place a single GLSL example here the triggers the problem.

[alan-baker]

The attachment in the description has a simplified version of the other bug. Reproducing here for simplicity:

#version 310 es

void main()
{
 if (true) {
   return;
 } else {
   return;
 }
 switch(0) {
   default: 1;
 }
}

This produces the asm attached (also reproduced here):

               OpCapability Shader
          %1 = OpExtInstImport "GLSL.std.450"
               OpMemoryModel Logical GLSL450
               OpEntryPoint Fragment %main "main"
               OpExecutionMode %main OriginUpperLeft
               OpSource ESSL 310
               OpName %main "main"
       %void = OpTypeVoid
          %3 = OpTypeFunction %void
       %bool = OpTypeBool
       %true = OpConstantTrue %bool
        %int = OpTypeInt 32 1
      %int_0 = OpConstant %int 0
      %int_1 = OpConstant %int 1
       %main = OpFunction %void None %3
          %5 = OpLabel
               OpSelectionMerge %9 None
               OpBranchConditional %true %8 %11
          %8 = OpLabel
               OpReturn
         %11 = OpLabel
               OpReturn
          %9 = OpLabel
               OpSelectionMerge %16 None
               OpSwitch %int_0 %15
         %15 = OpLabel
               OpBranch %16
         %16 = OpLabel
               OpReturn
               OpFunctionEnd

In this case the switch is unreachable (in %9). The selection construct of that switch is {%9}, so the branch to %15 is an illegal exit from the construct.

[johnkslang]

The selection construct starting at %15 should include the same blocks that would be included if %15 was reachable. The specification needs to be fixed (already internally underway with Khronos), as the spirit is to not have different behavior for nested control flow processing between reachable and unreachable code, as long as together they all form properly nested control flow.

[johnkslang]

Informally, I believe we have a solution where nested control flow has the same structural concepts in both reachable and unreachable code, but the kind of unreachable code we will allow will be very restricted, basically what would map directly to HLL code contiguous with break/continue/return. Will say more when the specification process pins it down.

[arcady-lunarg]

@johnkslang Do you recall if the spec issue was sorted out?