[Mesa-dev] [PATCH 3/8] i965 gen6+: Use 1-wide null operands for IF instructions

[Paul Berry]

On 14 December 2011 02:33, Kenneth Graunke <kenneth at 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 <kenneth at 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;
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20111214/078e0293/attachment.htm>