[Mesa-dev] [PATCH 00/19] AMD support for NIR deref instructions.

[Rob Clark]

On Mon, May 14, 2018 at 8:07 PM, Dave Airlie <airlied at gmail.com> wrote:
> On 13 May 2018 at 10:19, Bas Nieuwenhuizen <bas at basnieuwenhuizen.nl> wrote:
>> This implements support in radv and radeonsi for NIR deref
>> instructions instead of deref chains.
>>
>> It contains 4 parts:
>> - patch 1 is a fixup for the initial series by Jason
>> - Add support everywhere for instruction based derefs
>> - Stop lowering them to deref chains (may need to be synchronized
>>   with other drivers and core nir code to keep things bisectable)
>> - Remove support for deref chains.
>
> Okay I've read over this and I'm pretty happy with it.
>
> I do wonder if we could lower the impact on some of the code by
> adding the wrappers functions and getting var in a few places.
>
> i.e. instead of
> +      nir_variable *var = uses_deref_chain ? instr->variables[0]->var :
> +
> nir_deref_instr_get_variable(nir_instr_as_deref(instr->src[0].ssa->parent_instr);
> -       int idx = instr->variables[0]->var->data.driver_location;
> +       int idx = var->data.driver_location;

I guess the uses_deref_chain part goes away, but I'd proposed some
helper to get the var before.. it saves a bit of the churn, but not
all.. (and iirc we ended up with a helper something along those lines
anyways, I'd have to switch back to deref branch to check the name..

Anyways, I'm pretty happy with Jason's series.. I didn't really bother
to try to review it patch by patch, but it is a lot of churn so
looking at the end result seemed to be the more sensible way to
approach it.  I've played with it quite a bit since I've added pointer
support for compute on top.

I guess tomorrow I should rebase the gallium/ir3/vc4/vc5 parts, and
start trying to merge this into a bisectable patchset.


BR,
-R

>
> We could do a first pass patch that adds the var =
> instr->variables[0]; and removes
> the instr->variables[0] references elsewhere.
>
> Up to you though, not sure how much extra work it would generate.
>
> Also for Jason, strct is less chars than struct_type, but I expect the
> number of times you typo it might end up more keystrokes :-P
>
> For the series:
> Reviewed-by: Dave Airlie <airlied at redhat.com>
>
> Dave.
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev