[Mesa-dev] [PATCH v2 2/4] i965/fs: Make get_timestamp() return an fs_inst * rather than emitting.

[Kenneth Graunke]

On Friday, February 27, 2015 11:37:01 PM Pohjolainen, Topi wrote:
> On Fri, Feb 27, 2015 at 11:15:35AM -0800, Kenneth Graunke wrote:
> > This makes another part of the INTEL_DEBUG=shader_time code emittable
> > at arbitrary locations, rather than just at the end of the instruction
> > stream.
> > 
> > v2: Don't lose smear!  Caught by Topi Pohjolainen.
> > 
> > Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> > ---
> >  src/mesa/drivers/dri/i965/brw_fs.cpp | 24 +++++++++++++-----------
> >  src/mesa/drivers/dri/i965/brw_fs.h   |  2 +-
> >  2 files changed, 14 insertions(+), 12 deletions(-)
> > 
> > Yikes, good catch!  Thanks for the review, Topi!
> > 
> > diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
> > index 9c6f084..d65f1f1 100644
> > --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> > +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> > @@ -680,8 +680,8 @@ fs_visitor::type_size(const struct glsl_type *type)
> >     return 0;
> >  }
> >  
> > -fs_reg
> > -fs_visitor::get_timestamp()
> > +fs_inst *
> > +fs_visitor::timestamp_read()
> >  {
> >     assert(brw->gen >= 7);
> >  
> > @@ -692,12 +692,6 @@ fs_visitor::get_timestamp()
> >  
> >     fs_reg dst = fs_reg(GRF, alloc.allocate(1), BRW_REGISTER_TYPE_UD, 4);
> >  
> > -   fs_inst *mov = emit(MOV(dst, ts));
> > -   /* We want to read the 3 fields we care about even if it's not enabled in
> > -    * the dispatch.
> > -    */
> > -   mov->force_writemask_all = true;
> > -
> >     /* The caller wants the low 32 bits of the timestamp.  Since it's running
> >      * at the GPU clock rate of ~1.2ghz, it will roll over every ~3 seconds,
> >      * which is plenty of time for our purposes.  It is identical across the
> > @@ -710,14 +704,21 @@ fs_visitor::get_timestamp()
> >      */
> >     dst.set_smear(0);
> >  
> > -   return dst;
> > +   fs_inst *mov = MOV(dst, ts);
> 
> Previously the smear wasn't set for the destination in the instruction
> itself. I had to check what set_smear() really does. It also sets stride to
> zero which the original logic left to the init value of one. I guess this is
> not what you intented?

Augh.  Good catch - that totally breaks things.  It makes the timestamp
read a mov(1) instead of a mov(4).

I rebased this code, and discovered that shader_time was just totally
hosed again.  I think I've patched it up, and will send out a respin
shortly.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150308/fe4e65fa/attachment.sig>