On 14 December 2011 02:33, Kenneth Graunke <<a href="mailto:kenneth@whitecape.org">kenneth@whitecape.org</a>> wrote: <div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <div class="im">On 12/13/2011 03:35 PM, Paul Berry wrote: > The Sandy Bridge PRM, volume 4, part 2, section 5.3.10 ("5.3.10 > Register Region Restrictions") contains the following restriction on > the execution size and operand width of instructions: > > "3. ExecSize must be equal to or greater than Width." > > When emitting an IF instruction in single program flow mode on Gen6+, > we use an ExecSize of 1, therefore the Width of each operand must also > be 1. > > The operands of an IF instruction are not actually used for their > normal purpose on Gen6+ (which is probably the reason this wasn't > causing a GPU hang), but nonetheless it seems prudent to follow this > rule. </div>I'm not actually sure what you're trying to say here. The IF statement is different on Gen4-5, Gen6, and Gen7, so I'm unclear what "normal" is. </blockquote><div> Tell you what, I'll just delete that last paragraph out of the commit message. It wasn't really adding much content anyway. </div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> I think the point is that the ARF null register doesn't have a value, so null<0,1,0> is just as good as null<4,4,1> or null<8,8,1>. Either way you get nothing and write nothing. So width is irrelevant other than dodging this assertion. If we weren't guessing execution size based on the register widths, I'd prefer to just change the null reg to have width 1. But since we do, that would be a royal pain, and this is nice and simple. I'm definitely in favor of this patch. </blockquote><div> Agreed. The whole "guessing execution size" thing has always troubled me, but I haven't thought of a good alternative. </div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> Reviewed-by: Kenneth Graunke <<a href="mailto:kenneth@whitecape.org">kenneth@whitecape.org</a>> <div class="HOEnZb"><div class="h5"> > This patch unconditionally uses 1-wide null operands for Gen6+ > IF instructions, rather than the standard null operand, which is 8 > components wide. > --- > src/mesa/drivers/dri/i965/brw_eu_emit.c | 8 ++++---- > 1 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/src/mesa/drivers/dri/i965/brw_eu_emit.c b/src/mesa/drivers/dri/i965/brw_eu_emit.c > index a46a81b..d48753c 100644 > --- a/src/mesa/drivers/dri/i965/brw_eu_emit.c > +++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c > @@ -941,11 +941,11 @@ brw_IF(struct brw_compile *p, GLuint execute_size) > } else if (intel->gen == 6) { > brw_set_dest(p, insn, brw_imm_w(0)); > insn->bits1.branch_gen6.jump_count = 0; > - brw_set_src0(p, insn, retype(brw_null_reg(), BRW_REGISTER_TYPE_D)); > - brw_set_src1(p, insn, retype(brw_null_reg(), BRW_REGISTER_TYPE_D)); > + brw_set_src0(p, insn, vec1(retype(brw_null_reg(), BRW_REGISTER_TYPE_D))); > + brw_set_src1(p, insn, vec1(retype(brw_null_reg(), BRW_REGISTER_TYPE_D))); > } else { > - brw_set_dest(p, insn, retype(brw_null_reg(), BRW_REGISTER_TYPE_D)); > - brw_set_src0(p, insn, retype(brw_null_reg(), BRW_REGISTER_TYPE_D)); > + brw_set_dest(p, insn, vec1(retype(brw_null_reg(), BRW_REGISTER_TYPE_D))); > + brw_set_src0(p, insn, vec1(retype(brw_null_reg(), BRW_REGISTER_TYPE_D))); > brw_set_src1(p, insn, brw_imm_ud(0)); > insn->bits3.break_cont.jip = 0; > insn->bits3.break_cont.uip = 0; </div></div></blockquote></div>