<div dir="ltr">Yeah, that's a good idea. I'll do that.<br></div><br><div class="gmail_quote"><div dir="ltr">On Tue, Sep 18, 2018 at 4:03 AM Iago Toral <<a href="mailto:itoral@igalia.com">itoral@igalia.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Should we add a similar check to validate_phi_src in nir_validate.c?<br>
<br>
On Mon, 2018-09-17 at 09:43 -0500, Jason Ekstrand wrote:<br>
> The lcssa and phis_to_regs passes are used by various NIR<br>
> optimizations<br>
> that modify the CFG.  Putting a couple of asserts will help ensure<br>
> that<br>
> we don't accidentally put derefs in phis as part of an optimization<br>
> pass.<br>
> ---<br>
>  src/compiler/nir/nir_from_ssa.c | 2 ++<br>
>  src/compiler/nir/nir_to_lcssa.c | 3 +++<br>
>  2 files changed, 5 insertions(+)<br>
> <br>
> diff --git a/src/compiler/nir/nir_from_ssa.c<br>
> b/src/compiler/nir/nir_from_ssa.c<br>
> index 1aa35509b11..19d4bc33820 100644<br>
> --- a/src/compiler/nir/nir_from_ssa.c<br>
> +++ b/src/compiler/nir/nir_from_ssa.c<br>
> @@ -901,6 +901,8 @@ nir_lower_phis_to_regs_block(nir_block *block)<br>
>  <br>
>        nir_foreach_phi_src(src, phi) {<br>
>           assert(src->src.is_ssa);<br>
> +         /* We don't want derefs ending up in phi sources */<br>
> +         assert(!nir_src_as_deref(src->src));<br>
>           place_phi_read(shader, reg, src->src.ssa, src->pred);<br>
>        }<br>
>  <br>
> diff --git a/src/compiler/nir/nir_to_lcssa.c<br>
> b/src/compiler/nir/nir_to_lcssa.c<br>
> index 9b3539193ea..0f62fc39400 100644<br>
> --- a/src/compiler/nir/nir_to_lcssa.c<br>
> +++ b/src/compiler/nir/nir_to_lcssa.c<br>
> @@ -111,6 +111,9 @@ convert_loop_exit_for_ssa(nir_ssa_def *def, void<br>
> *void_state)<br>
>     if (all_uses_inside_loop)<br>
>        return true;<br>
>  <br>
> +   /* We don't want derefs ending up in phi sources */<br>
> +   assert(def->parent_instr->type != nir_instr_type_deref);<br>
> +<br>
>     /* Initialize a phi-instruction */<br>
>     nir_phi_instr *phi = nir_phi_instr_create(state->shader);<br>
>     nir_ssa_dest_init(&phi->instr, &phi->dest,<br>
</blockquote></div>