diff options
author | Francisco Jerez <[email protected]> | 2015-07-13 15:40:18 +0300 |
---|---|---|
committer | Francisco Jerez <[email protected]> | 2015-07-29 14:12:46 +0300 |
commit | f68ec2baf49e37f9ce4fffe95f13177eb7225015 (patch) | |
tree | 2d6b62c13907c201dca199a1d92d1324bccdebfa /src | |
parent | fa75f2d56616cba81014d4fc02931dcfaedaf5b9 (diff) |
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 <[email protected]>
Reviewed-by: Kenneth Graunke <[email protected]>
Diffstat (limited to 'src')
-rw-r--r-- | src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp | 8 |
1 files changed, 8 insertions, 0 deletions
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: |