On Fri, Mar 06, 2015 at 02:56:24PM +0200, Francisco Jerez wrote: > "Pohjolainen, Topi" <topi.pohjolai...@intel.com> writes: > > > On Fri, Feb 27, 2015 at 05:34:47PM +0200, Francisco Jerez wrote: > >> This is currently not a problem because the vec4 visitor happens to > >> mask out unused components from the destination, but it might become > >> an issue when we start using atomics without writeback message. In > >> any case it seems sensible to set it again here because the > >> consequences of setting the wrong writemask (random graphics memory > >> corruption) are difficult to debug and can easily go unnoticed. > > > > I started thinking if this should be an assertion here and should we force > > the logic in the visitor to consider the writemask correctly instead? I > > don't > > have a strong opinion, merely just wondering aloud. > > > > That would be rather inconvenient for my (not yet sent for review) > ARB_shader_image_load_store intrinsic lowering code. If we made it an > assertion, say: > > | emit(some_surface_opcode, vgrf(rlen), payload, surface, control); > > or > > | emit(some_surface_opcode, reg_null_ud(), payload, surface, control); > > would fail with an assertion failure. I need that to just work no > matter what surface opcode is specified. > > It would also be somewhat misleading, because these messages have the > annoying property of clobbering destination register components even if > they're masked out, so in Align16 they kind of always behave as if > WRITEMASK_XYZW was specified as far as the destination region is > concerned.
Just sent a reply with few nits. But otherwise I'm fine with the patch: Reviewed-by: Topi Pohjolainen <topi.pohjolai...@intel.com> _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
