KhronosGroup / SPIRV-Tools

Apache License 2.0
1.04k stars 545 forks source link

Validator: unreachable loop with unreachable continue fails validation #2373

Open paulthomson opened 5 years ago

paulthomson commented 5 years ago

The following SPIRV contains an unreachable loop with an unreachable continue target and an unreachable merge block. It fails validation:

error: line 17: Back-edges (11 -> 13) can only be formed between a block and a loop header.
  %11 = OpLabel
               OpCapability Shader
          %1 = OpExtInstImport "GLSL.std.450"
               OpMemoryModel Logical GLSL450
               OpEntryPoint Fragment %2 "main"
               OpExecutionMode %2 OriginUpperLeft
               OpSource ESSL 310
               OpName %2 "main"
          %3 = OpTypeVoid
          %4 = OpTypeFunction %3
          %5 = OpTypeInt 32 1
          %6 = OpTypePointer Function %5
          %7 = OpTypeBool
          %8 = OpConstantFalse %7

          %2 = OpFunction %3 None %4

          %9 = OpLabel
               OpBranch %10

         %11 = OpLabel                   ; header
               OpLoopMerge %12 %13 None
               OpBranch %14
         %13 = OpLabel                   ; continue
               OpBranch %11
         %14 = OpLabel
               OpReturn
         %12 = OpLabel                   ; merge
               OpBranch %10

         %10 = OpLabel
               OpReturn
               OpFunctionEnd
alan-baker commented 5 years ago

The message is confusing for sure, but the unreachable loop does not satisfy the rules about dominance. For example, the header (%11) does not dominate the continue target (%13).

Khronos internal issue 330 is relevant to this. It is unlikely that this issue will be addressed until that issue is resolved.

paulthomson commented 5 years ago

Yeah makes sense.

Technically, I believe the spec (v1.3) says, "the loop header must dominate the Continue Target, unless the Continue Target is unreachable in the CFG". So that seems fine. But it also says: "the Continue Target must dominate the back-edge block" and "the back-edge block must post dominate the Continue Target". Maybe those hold, depending on how you interpret the definition of dominate, although the validation error seems unrelated anyway.

alan-baker commented 5 years ago

The validator error stems from the way it performs the traversal to determine back-edges. Since those blocks are unreachable they get traversed after the main graph and the order they are visited will determine how the validator sees the back-edge.

alan-baker commented 5 years ago

@dneto0 what do you think about avoiding checks for unreachable blocks or ensuring the traversal is in binary order for unreachable blocks?

paulthomson commented 5 years ago

Was the wrong issue closed? It looks like 2372 should have been closed.