Samuel Iglesias Gonsálvez <sigles...@igalia.com> writes: > On Thu, 2017-04-20 at 10:26 -0700, Francisco Jerez wrote: >> Samuel Iglesias Gonsálvez <sigles...@igalia.com> writes: >> >> > It was setting XYWZ swizzle to all uniforms, no matter if they were >> > a vector or not. >> > >> > Signed-off-by: Samuel Iglesias Gonsálvez <sigles...@igalia.com> >> > Cc: curroje...@riseup.net >> >> Don't you need to CC mesa-stable here and in the next patch? >> > > I considered it but I has doubts about which tag use "17.1.0-rc1" or > just "17.1.0" or whatever. So my plan is to notify Emil once they are > merged (and add Cc to stable in the commit log before pushing it to > master). > > If you are more comfortable with Cc mesa-stable, I will do it next time > (or if I need to send v2 of this series). >
I believe Emil will notice them even if you don't put a version tag. I don't really care how you nominate it for stable as long as it hits the 17.1 branch before the end of the release cycle. ;) >> > --- >> > src/intel/compiler/brw_vec4_nir.cpp | 1 + >> > 1 file changed, 1 insertion(+) >> > >> > diff --git a/src/intel/compiler/brw_vec4_nir.cpp >> > b/src/intel/compiler/brw_vec4_nir.cpp >> > index a82d52088a8..5f4488c7e86 100644 >> > --- a/src/intel/compiler/brw_vec4_nir.cpp >> > +++ b/src/intel/compiler/brw_vec4_nir.cpp >> > @@ -863,6 +863,7 @@ >> > vec4_visitor::nir_emit_intrinsic(nir_intrinsic_instr *instr) >> > unsigned offset = const_offset->u32[0] + shift * 4; >> > src.offset = ROUND_DOWN_TO(offset, 16); >> > shift = (offset % 16) / 4; >> > + src.swizzle = brw_swizzle_for_size(instr- >> > >num_components); >> >> What about the indirect case a few lines below? Isn't the swizzle >> passed >> to the mov indirect instruction still bogus? >> > > This is different. It is expecting to have a swizzle of XYZW because > MOV_INDIRECT will copy all the contents. See assert in > move_uniform_array_access_to_pull_constants() I believe the ultimate problem here is that the MOV_INDIRECT gets a writemask of XYZW even if you're reading a scalar, so setting the minimal swizzle will lead to a situation in which the resulting swizzle is not the identity so you will run into trouble to turn it into scratch access. That brings me to the following question which I don't think I can answer by looking at this patch alone: If having an XYZW swizzle is a problem for direct moves (I assume this patch is fixing something?), how come it is not a problem for indirect moves? > and the comment in pack_uniform_registers(): > > /* We just mark every register touched by a MOV_INDIRECT as being > * fully used. This ensures that it doesn't broken up piecewise by > * the next part of our packing algorithm. > */ > Not sure why this is relevant to the discussion? The same will be the case if you set the same swizzle as for the direct move. > Sam > >> > src.swizzle += BRW_SWIZZLE4(shift, shift, shift, shift); >> > >> > emit(MOV(dest, src)); >> > -- >> > 2.11.0
signature.asc
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
