On 18 July 2012 15:12, Ian Romanick <span dir="ltr"><<a href="mailto:idr@freedesktop.org" target="_blank">idr@freedesktop.org</a>></span> wrote:<br><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="HOEnZb"><div class="h5">On 07/18/2012 02:50 PM, Kenneth Graunke wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
On 07/18/2012 12:16 PM, Paul Berry wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
---<br>
src/mesa/program/arbprogparse.<u></u>c | 1 +<br>
src/mesa/program/program_<u></u>parse.y | 2 ++<br>
src/mesa/program/program_<u></u>parser.h | 1 +<br>
3 files changed, 4 insertions(+), 0 deletions(-)<br>
<br>
diff --git a/src/mesa/program/<u></u>arbprogparse.c b/src/mesa/program/<u></u>arbprogparse.c<br>
index dffc8ab..72e51dd 100644<br>
--- a/src/mesa/program/<u></u>arbprogparse.c<br>
+++ b/src/mesa/program/<u></u>arbprogparse.c<br>
@@ -120,6 +120,7 @@ _mesa_parse_arb_fragment_<u></u>program(struct gl_context* ctx, GLenum target,<br>
program->PixelCenterInteger = state.option.<u></u>PixelCenterInteger;<br>
<br>
program->UsesKill = state.fragment.UsesKill;<br>
+ program->UsesDFdy = state.fragment.UsesDFdy;<br>
</blockquote>
<br>
This reminds me, could you do something similar to clean up the<br>
kill_emitted flag in brw_fs_visitor? It's kind of a hack, and I seem to<br>
recall that we wanted to do the kind of front-end plumbing you're<br>
already doing here.<br></blockquote></div></div></blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class="HOEnZb"><div class="h5"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Not to mention there already seems to be a UsesKill flag...<br></blockquote></div></div></blockquote><div><br>(Greps through the code a bit) Oh, wow. Yeah, there's a bunch of ugliness in there. Things I would want to clean up:<br>
<br>1. UsesKill is declared in core mesa (in gl_fragment_program), but it's only *sometimes* set by core Mesa (specifically, core mesa sets it for arb programs and GLSL programs that go through ir_to_mesa). For GLSL programs that don't go through ir_to_mesa, it's set by the driver back-end (brw_link_shader() or glsl_to_tgsi_visitor). Splitting the responsibility for setting the flag between core mesa and the back-end based on the type of program, that just seems like it's asking for future bugs. This should be easy to fix by setting UsesKill in do_set_program_inouts(), which is already called from all the right places.<br>
<br>2. brw_link_shader() sets UsesKill before doing optimizations. That seems silly, since optimizations might be able to get rid of discards, in which case we shouldn't set the flag. The same fix should address this problem too--if we set UsesKill in do_set_program_inouts(), then it will happen at the right time, after optimizations.<br>
<br>3. Assuming we fix #2, kill_emitted should almost always be equal to UsesKill, so there's no point in having both. (The only exception would be if front-end optimizations can't figure out how to eliminate discards, but back-end optimizations can. That seems like a rare enough situation that it's not worth worrying about.)<br>
<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class="HOEnZb"><div class="h5"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
</blockquote>
<br></div></div>
I thought there were two problems with that. First, we try to optimize away the kill instructions, and it's hard to unset a flag. Second, there are other things that cause kill_emitted to get set (alpha test?).<br>
</blockquote></div><br>The first point should get taken care of by #2 above.<br><br>As to the second point, I believe you may be conflating the kill_enabled/UsesKill flags with the GEN{6,7}_WM_KILL_ENABLE flags. The former set of flags is only set by discards. The latter set of flags is derived from UsesKill by ORing with (ctx->Color.AlphaEnabled || ctx->Multisample.SampleAlphaToCoverage), and this happens long after compilation has finished (see, for example, gen7_wm_state.c, around line 74). The clean up I anticipate doing would only affect the kill_enabled/UsesKill flags, so GEN{6,7}_WM_KILL_ENABLE should still work properly.<br>
<br>I'll try putting together some clean-up patches for kill_enabled/UsesKill. It looks like it shouldn't be too difficult.<br><br>Paul<br>