On 12 August 2013 13:18, Matt Turner <matts...@gmail.com> wrote:
> ---
> src/mesa/drivers/dri/i965/brw_fs.cpp | 13 +++++++++++++
> src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp | 3 +++
> src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp | 10 ++++++----
> 3 files changed, 22 insertions(+), 4 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp
> b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index a81e97f..f104f8c 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -2118,6 +2118,19 @@ fs_visitor::register_coalesce()
> }
> }
>
> + if (has_source_modifiers) {
> + for (int i = 0; i < 3; i++) {
> + if (scan_inst->src[i].file == GRF &&
> + scan_inst->src[i].reg == inst->dst.reg &&
> + scan_inst->src[i].reg_offset == inst->dst.reg_offset &&
> + inst->dst.type != scan_inst->src[i].type)
> + {
> + return false;
> + }
> + }
> + }
> +
> +
>
I don't understand the whole patch, but I'm pretty sure it's wrong to
return false in this case, because that will exit the entire
fs_visitor::register_coalesce() function.
> /* The gen6 MATH instruction can't handle source modifiers or
> * unusual register regions, so avoid coalescing those for
> * now. We should do something more specific.
> 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 234f8bd..a5cd858 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp
> @@ -221,6 +221,9 @@ fs_visitor::try_copy_propagate(fs_inst *inst, int arg,
> acp_entry *entry)
> entry->src.smear != -1) && !can_do_source_mods(inst))
> return false;
>
> + if (has_source_modifiers && (entry->dst.type != inst->src[arg].type))
> + return false;
> +
> inst->src[arg].file = entry->src.file;
> inst->src[arg].reg = entry->src.reg;
> inst->src[arg].reg_offset = entry->src.reg_offset;
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp
> b/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp
> index c28d0de..2a2d403 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp
> @@ -206,14 +206,16 @@ vec4_visitor::try_copy_propagation(vec4_instruction
> *inst, int arg,
> if (inst->src[arg].negate)
> value.negate = !value.negate;
>
> - bool has_source_modifiers = (value.negate || value.abs ||
> - value.swizzle != BRW_SWIZZLE_XYZW ||
> - value.file == UNIFORM);
> + bool has_source_modifiers = value.negate || value.abs;
>
> /* gen6 math and gen7+ SENDs from GRFs ignore source modifiers on
> * instructions.
> */
> - if (has_source_modifiers && !can_do_source_mods(inst))
> + if ((has_source_modifiers || value.file == UNIFORM ||
> + value.swizzle != BRW_SWIZZLE_XYZW) && !can_do_source_mods(inst))
> + return false;
> +
> + if (has_source_modifiers && (value.type != inst->src[arg].type))
> return false;
>
> bool is_3src_inst = (inst->opcode == BRW_OPCODE_LRP ||
> --
> 1.8.3.2
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
