[Mesa-dev] [PATCH v3 024/104] nir: Support deref instructions in lower_system_values

[Caio Marcelo de Oliveira Filho]

Hi,

> >> Question: nir_deref_instr_get_variable will walk the deref instr
> >> chain, but does it even make sense if there's a array or struct in
> >> this deref chain? Should this be asserted?
> >>
> >
> > That's an interesting question.  Certainly, at this point in the patch
> > series, we can't make that assumption.  This is because we haven't checked
> > the mode yet.  However, once we can assume deref instructions, we can check
> > the mode and then go on to find the var.  Maybe something like this
> > (untested):
> >
> > https://gitlab.freedesktop.org/jekstrand/mesa/commit/
> > 33aee39955eff842d6ea263da2f36e60287e62ee
> >
> 
> It turns out that there is one system value which is an array:
> gl_SampleMask.  However, due to details, we only ever load element 0 so we
> can ignore the array deref in that case.  Unfortunately, this means that we
> can't do any better than what we have here. :-(

I think we could still be strict while handling that case, by being
explicit about it in the middle of the patch you shared:

    nir_deref *deref = nir_src_as_deref(load_deref->src[0]);
    if (deref->mode != nir_var_system_value) {
       continue;
    }

    if (deref->deref_type != nir_deref_type_var) {
       assert(deref->deref_type == nir_deref_type_array);
       assert(nir_instr_get_variable(deref)->data.location == SYSTEM_VALUE_SAMPLE_MASK);
       /* Short explanation that we only load ever position zero, maybe even assert... */
       deref = nir_deref_instr_parent(deref);
    }

    assert(deref->deref_type == nir_deref_type_var);
    nir_variable *var = deref->var;

Would something like that work?


Thanks,
Caio