From 7aa53cd725cc2287fc206033120e08cde74cde2a Mon Sep 17 00:00:00 2001 From: Iago Toral Quiroga Date: Wed, 23 Mar 2016 12:02:21 +0100 Subject: [PATCH] i965/fs: disallow type change in copy-propagation if types have different sizes Because the semantics of source modifiers are type-dependent, the type of the original source of the copy must be kept unmodified while propagating it into some instruction, which implies that we need to have the guarantee that the meaning of the instruction is going to remain the same after we have changed the types. Whenthe size of the new type is different from the size of the old type the new and old instructions cannot possibly be equivalent because the new instruction will be reading more data than the old one was. Prevents that we turn this: load_payload(8) vgrf17:DF, |vgrf4+0.0|:DF 1sthalf mov(8) vgrf18:DF, vgrf17:DF 1sthalf load_payload(8) vgrf5:DF, vgrf18:DF, vgrf20:DF NoMask 1sthalf WE_all load_payload(8) vgrf21:UD, vgrf5+0.4<2>:UD 1sthalf mov(8) vgrf22:UD, vgrf21:UD 1sthalf into: load_payload(8) vgrf17:DF, |vgrf4+0.0|:DF 1sthalf mov(8) vgrf18:DF, |vgrf4+0.0|:DF 1sthalf load_payload(8) vgrf5:DF, |vgrf4+0.0|:DF, |vgrf4+2.0|:DF NoMask 1sthalf WE_all load_payload(8) vgrf21:UD, vgrf5+0.4<2>:UD 1sthalf mov(8) vgrf22:DF, |vgrf4+0.4|<2>:DF 1sthalf where the semantics of the last instruccion have changed. v2 (Curro): - Update commit log and add comment to explain the problem better. - Simplify the condition. Reviewed-by: Francisco Jerez Reviewed-by: Kenneth Graunke --- src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) 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 875928b0d30..b9ff031bf82 100644 --- a/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp +++ b/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp @@ -410,9 +410,16 @@ fs_visitor::try_copy_propagate(fs_inst *inst, int arg, acp_entry *entry) type_sz(inst->src[arg].type)) % type_sz(entry->src.type) != 0) return false; + /* Since semantics of source modifiers are type-dependent we need to + * ensure that the meaning of the instruction remains the same if we + * change the type. If the sizes of the types are different the new + * instruction will read a different amount of data than the original + * and the semantics will always be different. + */ if (has_source_modifiers && entry->dst.type != inst->src[arg].type && - !inst->can_change_types()) + (!inst->can_change_types() || + type_sz(entry->dst.type) != type_sz(inst->src[arg].type))) return false; if (devinfo->gen >= 8 && (entry->src.negate || entry->src.abs) && -- 2.30.2