From f68ec2baf49e37f9ce4fffe95f13177eb7225015 Mon Sep 17 00:00:00 2001 From: Francisco Jerez Date: Mon, 13 Jul 2015 15:40:18 +0300 Subject: [PATCH] i965/fs: Make sure that the type sizes are compatible during copy propagation. It's surprising that we weren't checking for this already. A future patch will cause code like the following to be emitted: MOV(16) tmp<1>:uw, src MOV(8) dst<1>:ud, tmp<8,8,1>:ud The second MOV comes from the expansion of a LOAD_PAYLOAD header copy, so I don't have control over its types. Copy propagation will happily turn this into: MOV(8) dst<1>:ud, src Which has different semantics. Fix it by preventing propagation in cases where a single channel of the instruction would span several channels of the copy (this requirement could in fact be relaxed if the copy is just a trivial memcpy, but this case is unusual enough that I don't think it matters in practice). I'm deliberately only checking if the type of the instruction is larger than the original, because the converse case seems to be handled correctly already in the code below. Reviewed-by: Jason Ekstrand Reviewed-by: Kenneth Graunke --- src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp b/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp index 54e9114fe6b..269bdb5b272 100644 --- a/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp +++ b/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp @@ -339,6 +339,14 @@ fs_visitor::try_copy_propagate(fs_inst *inst, int arg, acp_entry *entry) if (entry->src.stride * inst->src[arg].stride > 4) return false; + /* Bail if the instruction type is larger than the execution type of the + * copy, what implies that each channel is reading multiple channels of the + * destination of the copy, and simply replacing the sources would give a + * program with different semantics. + */ + if (type_sz(entry->dst.type) < type_sz(inst->src[arg].type)) + return false; + /* Bail if the result of composing both strides cannot be expressed * as another stride. This avoids, for example, trying to transform * this: -- 2.30.2