[Mesa-dev] [PATCH 1/2] i965: Extract depctrl hazards

[Ben Widawsky]

On Fri, Nov 21, 2014 at 11:04:18AM -0800, Matt Turner wrote:
> On Fri, Nov 21, 2014 at 10:50 AM, Ben Widawsky
> <benjamin.widawsky at intel.com> wrote:
> > Move this to a separate function so that we can begin to add other little
> > caveats without making too big a mess.
> >
> > NOTE: There is some desire to improve this function eventually, but we need to
> > fix a bug first.
> >
> > Signed-off-by: Ben Widawsky <ben at bwidawsk.net>
> > ---
> >  src/mesa/drivers/dri/i965/brw_vec4.cpp | 42 ++++++++++++++++------------------
> >  1 file changed, 20 insertions(+), 22 deletions(-)
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> > index df589b8..0c2bbe9 100644
> > --- a/src/mesa/drivers/dri/i965/brw_vec4.cpp
> > +++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> > @@ -840,6 +840,25 @@ vec4_visitor::move_push_constants_to_pull_constants()
> >     pack_uniform_registers();
> >  }
> >
> > +/* Conditions for which we want to avoid setting the dependency control bits */
> > +static bool
> > +is_dep_ctrl_safe(vec4_instruction *inst)
> 
> Can mark as const.
> 
> > +{
> > +   /*
> > +    * In the presence of send messages, totally interrupt dependency
> > +    * control. They're long enough that the chance of dependency
> > +    * control around them just doesn't matter.
> > +    *
> > +    * It looks like setting dependency control on a predicated
> > +    * instruction hangs the GPU.
> > +    * NB: I can find no evidence of this in modern specs.
> 
> It's in the Destination Hazard section.
> 
> Quoting the IVB PRM:
> 
> When a sequence of NoDDChk and NoDDClr are used, the last instruction
> that completes the scoreboard
> clear must have a non-zero execution mask. This means, if any kind of
> predication can change the
> execution mask or channel enable of the last instruction, the
> optimization must be avoided. This is to
> avoid instructions being shot down the pipeline when no writes are required.
> 
> Example:
> (f0.0) mov r10.0 r11.0 {NoDDClr}
> (-f0.0) mov r10.0 r11.0 {NoDDChk, NoDDClr}
> 
> In the above case, if predication can disable all writes to r10 for
> the second instructions, the instruction
> maybe shot down the pipeline resulting in un-deterministic behavior.
> Hence, This optimization must not
> be used in these cases.

Thanks. I'll add this quote to the blurb.

> 
> > +    *
> > +    * Dependency control does not work well over math instructions.
> > +    * NB: I can find no evidence of this in modern specs.
> 
> I don't know what NB means. This one was empirically determined, so
> I'd just say it's undocumented if you want to say anything at all.

NB is the commonly used term in the kernel for "NOTE" basically, ie. this is
non-obvious. I thought it was a fairly common convention... guess not.

I intentionally didn't write it was empirically discovered because I [falsely]
assumed that came from an ancient chipset (or Sandybridge, which I don't trust).
I didn't want to imply that the empirical evidence was modern. Looking at the
commit now, it was probably run on Ivybridge, so it makes sense to add that
info.

> 
> > +    */ +   return !(inst->mlen || inst->predicate || inst->is_math());
> 
> The return value is a ! expression, and below our only caller is inverting the
> result. I'd probably just make it is_dep_ctrl_unsafe() or something.

I liked the sound of _safe better (and it fits in with my eventual plans better,
should I actually get around to it). It's fine with me to change it.

-- 
Ben Widawsky, Intel Open Source Technology Center