On Thu, 2019-03-07 at 07:15 -0600, Jason Ekstrand wrote: > Woah, is this legal SPIR-V? I think a second OpSelectionMerge is required.
I'd say it is legal. The spec does not mandate that each branch has its own merge instruction; only that the control flow must be structured for shaders. In section 2.11 ("Structured Control Flow"), about structured control-flow constructs: - A structured control-flow construct is then defined as one of: - a selection construct: the set of blocks dominated by a selection header, minus the set of blocks dominated by the header’s merge block - [...] - The above structured control-flow constructs must satisfy the following rules: - [...] - the only blocks in a construct that can branch outside the construct are - a block branching to the construct’s merge block - a block branching from one case construct to another, for the same OpSwitch - a back-edge block - a continue block for the innermost loop it is nested inside of - a break block for the innermost loop it is nested inside of - a return block Our selection construct, which contains the two nested conditional branches, satisfies the rules, as both conditionals branches to the construct's merge block. J.A. > > --Jason > > > On March 6, 2019 05:25:26 "Juan A. Suarez Romero" <jasua...@igalia.com> wrote: > > > This fixes the case when the SPIR-V code has two nested conditional > > branches, but only one selection merge: > > > > > > [...] > > %1 = OpLabel > > OpSelectionMerge %2 None > > OpBranchConditional %3 %4 %2 > > %4 = OpLabel > > OpBranchConditional %3 %5 %2 > > %5 = OpLabel > > OpBranch %2 > > %2 = OpLabel > > [...] > > > > > > In the second OpBranchConditional, as the else-part is the end > > block (started in the first OpBranchConditional) we can just follow the > > then-part. > > > > > > This fixes dEQP-VK.vkrunner.controlflow.2-obc-triangle-triangle > > > > > > CC: Jason Ekstrand <ja...@jlekstrand.net> > > --- > > src/compiler/spirv/vtn_cfg.c | 11 ++++++++++- > > 1 file changed, 10 insertions(+), 1 deletion(-) > > > > > > diff --git a/src/compiler/spirv/vtn_cfg.c b/src/compiler/spirv/vtn_cfg.c > > index 7868eeb60bc..f749118efbe 100644 > > --- a/src/compiler/spirv/vtn_cfg.c > > +++ b/src/compiler/spirv/vtn_cfg.c > > @@ -605,7 +605,16 @@ vtn_cfg_walk_blocks(struct vtn_builder *b, struct > > list_head *cf_list, > > } > > } else if (if_stmt->then_type == vtn_branch_type_none && > > if_stmt->else_type == vtn_branch_type_none) { > > - /* Neither side of the if is something we can short-circuit. */ > > + /* Neither side of the if is something we can short-circuit, > > + * unless one of the blocks is the end block. */ > > + if (then_block == end) { > > + block = else_block; > > + continue; > > + } else if (else_block == end) { > > + block = then_block; > > + continue; > > + } > > + > > vtn_assert((*block->merge & SpvOpCodeMask) == > > SpvOpSelectionMerge); > > struct vtn_block *merge_block = > > vtn_value(b, block->merge[1], vtn_value_type_block)->block; > > -- > > 2.20.1 > > > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev