On 04/06/2014 09:31 PM, Chia-I Wu wrote:
> From: Chia-I Wu <o...@lunarg.com>
> 
> Given
> 
>   mov vgrf7, vgrf9.xyxz
>   add vgrf9.xyz, vgrf4.xyzw, vgrf5.xyzw
>   add vgrf10.x, vgrf6.xyzw, vgrf7.wwww
> 
> the last instruction would be wrongly changed to
> 
>   add vgrf10.x, vgrf6.xyzw, vgrf9.zzzz
> 
> during copy propagation.
> 
> The issue is that when deciding if a record should be cleared, the old code
> checked for
> 
>   inst->dst.writemask & (1 << ch)
> 
> instead of
> 
>   inst->dst.writemask & (1 << BRW_GET_SWZ(src->swizzle, ch))
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=76749
> Signed-off-by: Chia-I Wu <o...@lunarg.com>
> Cc: Jordan Justen <jljus...@gmail.com>
> Cc: Matt Turner <matts...@gmail.com>

Mark for stable?  It looks like the same code exists in the 10.1 branch.

The fix looks correct to me, but you should wait for Matt, Eric, or Ken
to comment as well.

Reviewed-by: Ian Romainck <ian.d.roman...@intel.com>

> ---
>  .../drivers/dri/i965/brw_vec4_copy_propagation.cpp  | 21 
> ++++++++++++++++-----
>  1 file changed, 16 insertions(+), 5 deletions(-)
> 
> 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 3d68f0e..83cf191 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp
> @@ -58,6 +58,21 @@ is_dominated_by_previous_instruction(vec4_instruction 
> *inst)
>  }
>  
>  static bool
> +is_channel_updated(vec4_instruction *inst, src_reg *values[4], int ch)
> +{
> +   const src_reg *src = values[ch];
> +
> +   /* consider GRF only */
> +   assert(inst->dst.file == GRF);
> +   if (!src || src->file != GRF)
> +      return false;
> +
> +   return (src->reg == inst->dst.reg &&
> +        src->reg_offset == inst->dst.reg_offset &&
> +        inst->dst.writemask & (1 << BRW_GET_SWZ(src->swizzle, ch)));
> +}
> +
> +static bool
>  try_constant_propagation(vec4_instruction *inst, int arg, src_reg *values[4])
>  {
>     /* For constant propagation, we only handle the same constant
> @@ -357,11 +372,7 @@ vec4_visitor::opt_copy_propagation()
>        else {
>           for (int i = 0; i < virtual_grf_reg_count; i++) {
>              for (int j = 0; j < 4; j++) {
> -               if (inst->dst.writemask & (1 << j) &&
> -                   cur_value[i][j] &&
> -                   cur_value[i][j]->file == GRF &&
> -                   cur_value[i][j]->reg == inst->dst.reg &&
> -                   cur_value[i][j]->reg_offset == inst->dst.reg_offset) {
> +               if (is_channel_updated(inst, cur_value[i], j)){
>                    cur_value[i][j] = NULL;
>                 }
>              }

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to