[Mesa-dev] [PATCH 2/2] nir/spirv: Use a nop intrinsic for tagging the ends of blocks

[Dave Airlie]

On 16 September 2016 at 14:16, Jason Ekstrand <jason at jlekstrand.net> wrote:
> Previously, we were saving off the last nir_block in a vtn_block before
> moving on so that we could find the nir_block again when it came time to
> handle phi sources.  Unfortunately, NIR's control flow modification code is
> inconsistent when it comes to how it splits blocks so the block pointer we
> saved off may point to a block somewhere else in the shader by the time we
> get around to handling phi sources.  In order to get around this, we insert
> a nop instruction and use that as the logical end of our block.  Since the
> control flow manipulation code respects instructions, the nop will keeps
> its place like any other instruction and we can easily find the end of our
> block when we need it.
>
> Signed-off-by: Jason Ekstrand <jason at jlekstrand.net>

I'm not sure I'm good enough to review it, but this makes sense after
I looked through it.

It also fixes vkQuake on radv.

Tested-by: Dave Airlie <airlied at redhat.com>
Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=97233

Dave.

> ---
>  src/compiler/spirv/vtn_cfg.c     | 6 ++++--
>  src/compiler/spirv/vtn_private.h | 4 ++--
>  2 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/src/compiler/spirv/vtn_cfg.c b/src/compiler/spirv/vtn_cfg.c
> index d9096f4..599ed69 100644
> --- a/src/compiler/spirv/vtn_cfg.c
> +++ b/src/compiler/spirv/vtn_cfg.c
> @@ -518,7 +518,7 @@ vtn_handle_phi_second_pass(struct vtn_builder *b, SpvOp opcode,
>        struct vtn_block *pred =
>           vtn_value(b, w[i + 1], vtn_value_type_block)->block;
>
> -      b->nb.cursor = nir_after_block_before_jump(pred->end_block);
> +      b->nb.cursor = nir_after_instr(&pred->end_nop->instr);
>
>        vtn_local_store(b, src, nir_deref_var_create(b, phi_var));
>     }
> @@ -576,7 +576,9 @@ vtn_emit_cf_list(struct vtn_builder *b, struct list_head *cf_list,
>
>           vtn_foreach_instruction(b, block_start, block_end, handler);
>
> -         block->end_block = nir_cursor_current_block(b->nb.cursor);
> +         block->end_nop = nir_intrinsic_instr_create(b->nb.shader,
> +                                                     nir_intrinsic_nop);
> +         nir_builder_instr_insert(&b->nb, &block->end_nop->instr);
>
>           if ((*block->branch & SpvOpCodeMask) == SpvOpReturnValue) {
>              struct vtn_ssa_value *src = vtn_ssa_value(b, block->branch[1]);
> diff --git a/src/compiler/spirv/vtn_private.h b/src/compiler/spirv/vtn_private.h
> index 7f5444e..6f34f09 100644
> --- a/src/compiler/spirv/vtn_private.h
> +++ b/src/compiler/spirv/vtn_private.h
> @@ -149,8 +149,8 @@ struct vtn_block {
>     /** Points to the switch case started by this block (if any) */
>     struct vtn_case *switch_case;
>
> -   /** The last block in this SPIR-V block. */
> -   nir_block *end_block;
> +   /** Every block ends in a nop intrinsic so that we can find it again */
> +   nir_intrinsic_instr *end_nop;
>  };
>
>  struct vtn_function {
> --
> 2.5.0.400.gff86faf
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev