<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Mon, Apr 9, 2018 at 5:21 PM, Caio Marcelo de Oliveira Filho <span dir="ltr"><<a href="mailto:caio.oliveira@intel.com" target="_blank">caio.oliveira@intel.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi,<br>
<span class="gmail-"><br>
> >> Question: nir_deref_instr_get_variable will walk the deref instr<br>
> >> chain, but does it even make sense if there's a array or struct in<br>
> >> this deref chain? Should this be asserted?<br>
> >><br>
> ><br>
> > That's an interesting question.  Certainly, at this point in the patch<br>
> > series, we can't make that assumption.  This is because we haven't checked<br>
> > the mode yet.  However, once we can assume deref instructions, we can check<br>
> > the mode and then go on to find the var.  Maybe something like this<br>
> > (untested):<br>
> ><br>
> > <a href="https://gitlab.freedesktop.org/jekstrand/mesa/commit/" rel="noreferrer" target="_blank">https://gitlab.freedesktop.<wbr>org/jekstrand/mesa/commit/</a><br>
> > 33aee39955eff842d6ea263da2f36e<wbr>60287e62ee<br>
> ><br>
><br>
> It turns out that there is one system value which is an array:<br>
> gl_SampleMask.  However, due to details, we only ever load element 0 so we<br>
> can ignore the array deref in that case.  Unfortunately, this means that we<br>
> can't do any better than what we have here. :-(<br>
<br>
</span>I think we could still be strict while handling that case, by being<br>
explicit about it in the middle of the patch you shared:<br>
<br>
    nir_deref *deref = nir_src_as_deref(load_deref-><wbr>src[0]);<br>
    if (deref->mode != nir_var_system_value) {<br>
       continue;<br>
    }<br>
<br>
    if (deref->deref_type != nir_deref_type_var) {<br>
       assert(deref->deref_type == nir_deref_type_array);<br>
       assert(nir_instr_get_variable(<wbr>deref)->data.location == SYSTEM_VALUE_SAMPLE_MASK);<br>
       /* Short explanation that we only load ever position zero, maybe even assert... */<br>
       deref = nir_deref_instr_parent(deref);<br>
    }<br>
<br>
    assert(deref->deref_type == nir_deref_type_var);<br>
    nir_variable *var = deref->var;<br>
<br>
Would something like that work?<br>
</blockquote></div><br></div><div class="gmail_extra">I took another swing at it, and this one seems to make Jenkins happy:<br><br><a href="https://gitlab.freedesktop.org/jekstrand/mesa/commit/ad3cc9f301da3519d4f76767a6d9e98e5a5c118e">https://gitlab.freedesktop.org/jekstrand/mesa/commit/ad3cc9f301da3519d4f76767a6d9e98e5a5c118e</a><br></div></div>