On Mon, May 14, 2018 at 8:07 PM, Dave Airlie <airl...@gmail.com> wrote: > On 13 May 2018 at 10:19, Bas Nieuwenhuizen <b...@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 <airl...@redhat.com> > > Dave. > _______________________________________________ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
