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

[Jason Ekstrand]

Bump...

On Thu, Sep 15, 2016 at 9:33 PM, Dave Airlie <airlied at gmail.com> wrote:

> 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20161001/4917d2e4/attachment-0001.html>