That is fine by me. I've come up with a workaround for now (just adding some dummy output components to handle the .x__w case) so at least he .w varying doesn't get lost with that shader.
BR, -R On Sun, Dec 9, 2018 at 8:20 PM Timothy Arceri <tarc...@itsqueeze.com> wrote: > > I'd much rather land the first 3 patches from this series if possible. > > https://patchwork.freedesktop.org/series/53800/ > > I've confirmed it packs the shaders you were looking at as expected once > you patch 2 is applied. The series makes this code much more flexible > (for future improvements) and easier to follow. > > On 9/12/18 5:28 am, Rob Clark wrote: > > Previously, if we had a .z or .w component that could be compacted > > to .y, we'd could overlook that opportunity. > > > > Signed-off-by: Rob Clark <robdcl...@gmail.com> > > --- > > src/compiler/nir/nir_linking_helpers.c | 30 ++++++++++++++++++++------ > > 1 file changed, 23 insertions(+), 7 deletions(-) > > > > diff --git a/src/compiler/nir/nir_linking_helpers.c > > b/src/compiler/nir/nir_linking_helpers.c > > index 1ab9c095657..ce368a3c132 100644 > > --- a/src/compiler/nir/nir_linking_helpers.c > > +++ b/src/compiler/nir/nir_linking_helpers.c > > @@ -431,12 +431,30 @@ compact_components(nir_shader *producer, nir_shader > > *consumer, uint8_t *comps, > > uint8_t interp = get_interp_type(var, type, > > default_to_smooth_interp); > > for (; cursor[interp] < 32; cursor[interp]++) { > > uint8_t cursor_used_comps = comps[cursor[interp]]; > > + uint8_t unused_comps = ~cursor_used_comps; > > > > - /* We couldn't find anywhere to pack the varying continue on. > > */ > > - if (cursor[interp] == location && > > - (var->data.location_frac == 0 || > > - cursor_used_comps & ((1 << (var->data.location_frac)) - > > 1))) > > - break; > > + /* Don't search beyond our current location, we are just trying > > + * to pack later varyings to lower positions: > > + */ > > + if (cursor[interp] == location) { > > + if (var->data.location_frac == 0) > > + break; > > + > > + /* If not already aligned to slot, see if we can shift it > > up. > > + * Note that if we get this far it is a scalar so we know > > that > > + * shifting this var to any other open position won't > > conflict > > + * with it's current position. > > + */ > > + unsigned p = ffs(unused_comps & 0xf); > > + if (!p) > > + break; > > + > > + /* ffs returns 1 for bit zero: */ > > + p--; > > + > > + if (p >= var->data.location_frac) > > + break; > > + } > > > > /* We can only pack varyings with matching interpolation > > types */ > > if (interp_type[cursor[interp]] != interp) > > @@ -460,8 +478,6 @@ compact_components(nir_shader *producer, nir_shader > > *consumer, uint8_t *comps, > > if (!cursor_used_comps) > > continue; > > > > - uint8_t unused_comps = ~cursor_used_comps; > > - > > for (unsigned i = 0; i < 4; i++) { > > uint8_t new_var_comps = 1 << i; > > if (unused_comps & new_var_comps) { > > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
