[Mesa-dev,RFC,1/5] i965/eu: set correct execution size in brw_NOP

Submitted by Iago Toral Quiroga on Dec. 9, 2015, 12:15 p.m.

Details

Message ID 1449663349-17772-2-git-send-email-itoral@igalia.com
State New
Headers show
Series "Skip automatic execsize for instructions with a width of 4" ( rev: 1 ) in Mesa

Not browsing as part of any series.

Commit Message

Iago Toral Quiroga Dec. 9, 2015, 12:15 p.m.
---
 src/mesa/drivers/dri/i965/brw_eu_emit.c | 1 +
 1 file changed, 1 insertion(+)

Patch hide | download patch | download mbox

diff --git a/src/mesa/drivers/dri/i965/brw_eu_emit.c b/src/mesa/drivers/dri/i965/brw_eu_emit.c
index f8c0f80..9543d5e 100644
--- a/src/mesa/drivers/dri/i965/brw_eu_emit.c
+++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c
@@ -1256,6 +1256,7 @@  brw_F16TO32(struct brw_codegen *p, struct brw_reg dst, struct brw_reg src)
 void brw_NOP(struct brw_codegen *p)
 {
    brw_inst *insn = next_insn(p, BRW_OPCODE_NOP);
+   brw_inst_set_exec_size(p->devinfo, insn, BRW_EXECUTE_4);
    brw_set_dest(p, insn, retype(brw_vec4_grf(0,0), BRW_REGISTER_TYPE_UD));
    brw_set_src0(p, insn, retype(brw_vec4_grf(0,0), BRW_REGISTER_TYPE_UD));
    brw_set_src1(p, insn, brw_imm_ud(0x0));

Comments

On Wed, Dec 9, 2015 at 4:15 AM, Iago Toral Quiroga <itoral@igalia.com> wrote:
> ---
>  src/mesa/drivers/dri/i965/brw_eu_emit.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_eu_emit.c b/src/mesa/drivers/dri/i965/brw_eu_emit.c
> index f8c0f80..9543d5e 100644
> --- a/src/mesa/drivers/dri/i965/brw_eu_emit.c
> +++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c
> @@ -1256,6 +1256,7 @@ brw_F16TO32(struct brw_codegen *p, struct brw_reg dst, struct brw_reg src)
>  void brw_NOP(struct brw_codegen *p)
>  {
>     brw_inst *insn = next_insn(p, BRW_OPCODE_NOP);
> +   brw_inst_set_exec_size(p->devinfo, insn, BRW_EXECUTE_4);

I don't follow this change. Was this implicitly set before?

At least in newer documentation, NOP is defined to have nearly all
fields 0 which would mean execution size must be 1.
On Wed, 2015-12-09 at 10:38 -0800, Matt Turner wrote:
> On Wed, Dec 9, 2015 at 4:15 AM, Iago Toral Quiroga <itoral@igalia.com> wrote:
> > ---
> >  src/mesa/drivers/dri/i965/brw_eu_emit.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_eu_emit.c b/src/mesa/drivers/dri/i965/brw_eu_emit.c
> > index f8c0f80..9543d5e 100644
> > --- a/src/mesa/drivers/dri/i965/brw_eu_emit.c
> > +++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c
> > @@ -1256,6 +1256,7 @@ brw_F16TO32(struct brw_codegen *p, struct brw_reg dst, struct brw_reg src)
> >  void brw_NOP(struct brw_codegen *p)
> >  {
> >     brw_inst *insn = next_insn(p, BRW_OPCODE_NOP);
> > +   brw_inst_set_exec_size(p->devinfo, insn, BRW_EXECUTE_4);
> 
> I don't follow this change. Was this implicitly set before?

Yes, brw_NOP's uses brw_vec4_grf() for both dst and src0, so the code in
brw_set_dest will set an execution size of 4 (otherwise we hit the
assertion in validate_reg that checks execsize >= width). I have changed
this to use brw_vec1_grf and an execsize of 1 and it seems to work fine
though, so I'll merge that change in the patch.

Thanks Matt!

Iago

> At least in newer documentation, NOP is defined to have nearly all
> fields 0 which would mean execution size must be 1.
>