On 14 December 2011 02:33, Kenneth Graunke <kenn...@whitecape.org> wrote:
> 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.
>
> 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.
>
Tell you what, I'll just delete that last paragraph out of the commit
message. It wasn't really adding much content anyway.
>
> 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.
>
Agreed. The whole "guessing execution size" thing has always troubled me,
but I haven't thought of a good alternative.
>
> Reviewed-by: Kenneth Graunke <kenn...@whitecape.org>
>
> > 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;
>
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
