[Mesa-dev] [PATCH 2/2] i965: Don't overwrite the math SFID with conditional mod.

[Kenneth Graunke]

On Friday, November 21, 2014 12:43:43 PM Matt Turner wrote:
> Ben was asking about the undocumented restriction that the math
> instruction cannot use the dependency control hints. I went to reconfirm
> and disabled the is_math() check in opt_set_dependency_control() and saw
> that the disassembled math instructions with dependency hints had a
> bogus SFID. We were mistakenly overwriting it by setting an empty

"bogus FC" or "bogus math function".

Yow.  I sure don't remember seeing FC == 0 back when we were setting
dependency control instructions on math.  Maybe this crept in after
we disabled them.  Good catch, though!

Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>

> conditional mod.
> 
> Unfortunately, this wasn't the cause of the aforementioned problem (I
> reproduced it). This bug is benign, since we don't set dependeny hints
> on math instructions -- but maybe some day.
> ---
> Interestingly the BSpec now says about the math instruction:
> 
> 	Restriction : DepCtrl must not be used.
> 
> but only lists BDW and newer generations, so it seems like if pre-BDW is
> known broken and BDW and newer can't set DepCtrl, it might be a while.
> 
>  src/mesa/drivers/dri/i965/brw_fs_generator.cpp   | 3 ++-
>  src/mesa/drivers/dri/i965/brw_vec4_generator.cpp | 3 ++-
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> index 4af9cbe..06b0f34 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> @@ -2010,7 +2010,8 @@ fs_generator::generate_code(const cfg_t *cfg, int dispatch_width)
>  
>           brw_inst *last = &p->store[last_insn_offset / 16];
>  
> -         brw_inst_set_cond_modifier(brw, last, inst->conditional_mod);
> +         if (inst->conditional_mod)
> +            brw_inst_set_cond_modifier(brw, last, inst->conditional_mod);
>           brw_inst_set_no_dd_clear(brw, last, inst->no_dd_clear);
>           brw_inst_set_no_dd_check(brw, last, inst->no_dd_check);
>        }
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp b/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
> index d027fda..6a70a2f 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
> @@ -1509,7 +1509,8 @@ vec4_generator::generate_code(const cfg_t *cfg)
>  
>           brw_inst *last = &p->store[pre_emit_nr_insn];
>  
> -         brw_inst_set_cond_modifier(brw, last, inst->conditional_mod);
> +         if (inst->conditional_mod)
> +            brw_inst_set_cond_modifier(brw, last, inst->conditional_mod);
>           brw_inst_set_no_dd_clear(brw, last, inst->no_dd_clear);
>           brw_inst_set_no_dd_check(brw, last, inst->no_dd_check);
>        }
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20141122/47303b5c/attachment.sig>