[Mesa-dev] [PATCH 2/3] i965/fs: Only emit FS_OPCODE_PLACEHOLDER_HALT if there are discards
Kenneth Graunke
kenneth at whitecape.org
Sat Apr 11 01:44:22 PDT 2015
On Friday, April 10, 2015 10:49:51 PM Ben Widawsky wrote:
> On Fri, Apr 10, 2015 at 07:50:19PM -0700, Kenneth Graunke wrote:
> > On Friday, April 10, 2015 12:52:03 PM Ben Widawsky wrote:
> > > Based originally on a patch from Ken in May 2014 of the same title. Things
> > > changed enough that I didn't feel comfortable leaving his authorship. Ken, if
> > > you feel you should retain authorship, it's fine with me, just call it
> > > reviewed-by me instead.
> > >
> > > Cc: Kenneth Graunke <kenneth at whitecape.org>
> > > Signed-off-by: Ben Widawsky <ben at bwidawsk.net>
> > > ---
> > >
> > > Notes:
> > > Ken, if you feel you should retain authorship, it's fine with me, just call it
> > > reviewed-by me instead.
> >
> > No, I think you should retain authorship. I wrote a similar patch ages
> > ago and abandoned it, but your new patch is completely rewritten.
> >
> > >
> > > src/mesa/drivers/dri/i965/brw_fs.cpp | 6 +++++-
> > > 1 file changed, 5 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
> > > index 214ba40..72000cf 100644
> > > --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> > > +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> > > @@ -1700,6 +1700,9 @@ fs_visitor::emit_math(enum opcode opcode, fs_reg dst, fs_reg src0, fs_reg src1)
> > > void
> > > fs_visitor::emit_discard_jump()
> > > {
> > > + struct gl_fragment_program *fp = (struct gl_fragment_program *) this->prog;
> > > + assert(fp->UsesKill);
> > > +
> > > /* For performance, after a discard, jump to the end of the
> > > * shader if all relevant channels have been discarded.
> > > */
> >
> > Please drop this hunk - I don't think it provides any real value.
> > Or, change it to check wm_prog_data->uses_kill. Either way's fine.
> >
> > > @@ -3956,7 +3959,8 @@ fs_visitor::run_fs()
> > > if (failed)
> > > return false;
> > >
> > > - emit(FS_OPCODE_PLACEHOLDER_HALT);
> > > + if (wm_prog_data->uses_kill)
> > > + emit(FS_OPCODE_PLACEHOLDER_HALT);
> > >
> > > if (wm_key->alpha_test_func)
> > > emit_alpha_test();
> > >
> >
> > Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
>
> Cool thanks...
>
> While we're on the subject, I wouldn't mind the 5 second explanation of the
> distinction between fp->UsesKill and wm_prog_data->uses_kill. I actually only
> kept the former since your original patch had it (iirc).
They're pretty similar.
fp->UsesKill indicates that a ARB_fragment_program shader uses the KIL
instruction, or that a GLSL shader uses the "discard" insntruction
(which are analogous).
On Gen4-5, we sometimes have to simulate OpenGL's "Alpha Test" feature
by emitting shader code that implicitly does a "discard" instruction.
In the key setup, we do:
/* key->alpha_test_func means simulating alpha testing via discards,
* so the shader definitely kills pixels.
*/
prog_data.uses_kill = fp->program.UsesKill || key->alpha_test_func;
Even though the shader may not technically contain a "discard", we need
to act as if it does.
I've also been trying to move the i965 state setup code to use
brw_wm_prog_key for everything, rather than poking at core Mesa's
gl_program/gl_fragment_program/gl_shader/gl_shader_program structures.
--Ken
-------------- 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/20150411/7ded68e7/attachment-0001.sig>
More information about the mesa-dev
mailing list