On Fri, May 27, 2016 at 7:05 PM, Francisco Jerez <curroje...@riseup.net>
wrote:
> The sampler EOT optimization pass naively assumes that the texturing
> instruction provides all the data used by the FB write just because
> they're standing next to each other. The least we should be checking
> is whether the source and destination regions of the FB write and
> texturing instructions match. Without this the previous seemingly
> harmless commit would have caused opt_sampler_eot() to misoptimize a
> shader from dota-2 causing DCE to eliminate all of its 78 instructions
> except for the final sampler EOT message (!).
>
Oh, well that's fun...
> ---
> src/mesa/drivers/dri/i965/brw_defines.h | 1 +
> src/mesa/drivers/dri/i965/brw_fs.cpp | 14 ++++++++++++++
> 2 files changed, 15 insertions(+)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_defines.h
> b/src/mesa/drivers/dri/i965/brw_defines.h
> index 8794d44..60b215d 100644
> --- a/src/mesa/drivers/dri/i965/brw_defines.h
> +++ b/src/mesa/drivers/dri/i965/brw_defines.h
> @@ -1395,6 +1395,7 @@ enum fb_write_logical_srcs {
> FB_WRITE_LOGICAL_SRC_SRC_STENCIL, /* gl_FragStencilRefARB */
> FB_WRITE_LOGICAL_SRC_OMASK, /* Sample Mask (gl_SampleMask) */
> FB_WRITE_LOGICAL_SRC_COMPONENTS, /* REQUIRED */
> + FB_WRITE_LOGICAL_NUM_SRCS
> };
>
> enum tex_logical_srcs {
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp
> b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index 6abf776..1574cd9 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -2647,6 +2647,20 @@ fs_visitor::opt_sampler_eot()
> if (tex_inst->prev->is_head_sentinel())
> return false;
>
> + /* Check that the FB write sources are fully initialized by the single
> + * texturing instruction.
> + */
> + for (unsigned i = 0; i < FB_WRITE_LOGICAL_NUM_SRCS; i++) {
> + if (i == FB_WRITE_LOGICAL_SRC_COLOR0) {
> + if (!fb_write->src[i].equals(tex_inst->dst) ||
> + fb_write->regs_read(i) != tex_inst->regs_written)
> + return false;
> + } else if (i != FB_WRITE_LOGICAL_SRC_COMPONENTS) {
> + if (fb_write->src[i].file != BAD_FILE)
> + return false;
> + }
> + }
> +
> assert(!tex_inst->eot); /* We can't get here twice */
> assert((tex_inst->offset & (0xff << 24)) == 0);
>
> --
> 2.7.3
>
> _______________________________________________
> 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
