On Thu, Apr 18, 2019 at 9:13 AM Juan A. Suarez Romero <jasua...@igalia.com> wrote:
> On Thu, 2019-03-14 at 11:25 -0500, Jason Ekstrand wrote: > > Looking at this a bit more, I'm not sure that just short-circuiting > actually covers all the cases. Unfortunately, we don't know what all the > cases are because the SPIR-V spec doesn't say. I'm trying to work towards > fixing that right now. > > > > Did you have time to check this? > I had a branch where I was working on a full solution but then I ran into "what does the spec even mean?" issues, got the SPIR-V working group to chatter a bit, and then haven't heard anything in weeks. I think the best thing for us to do would be to assume "worst case" and try to handle as arbitrary CFGs as possible. I don't think just an early return will do that though. Looking at my git repo, I'm not finding a branch..... The approach I took (if I can remember correctly) was to add a new vtn_branch_type_merge which was used for the merge node of the current construct. (Or maybe I only used it for if-merges, I don't remember.) Then we would try to make vtn_cfg handle an arbitrary amount of divergence so long as everything eventually either jumps to a special target (such as a break/continue) or goes through the merge. That may end up being roughly equivalent to what you've tried to do in this patch but I think it's more robust to handle as it's own branch type than to just have a short-circuit. --Jason > > J.A. > > --Jason > > On Wed, Mar 6, 2019 at 5:25 AM 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; > >
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev