[Mesa-dev] [PATCH] nir/spirv: short-circuit when conditional branch contains end block

[Jason Ekstrand]

On Thu, Apr 18, 2019 at 9:13 AM Juan A. Suarez Romero <jasuarez at 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 <jasuarez at 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 <jason at 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;
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20190418/5366a68e/attachment-0001.html>