<div class="gmail_quote">On Thu, Jun 16, 2011 at 9:08 AM, Brian Paul <span dir="ltr">&lt;<a href="mailto:brianp@vmware.com">brianp@vmware.com</a>&gt;</span> wrote: 
<blockquote class="gmail_quote" style="PADDING-LEFT: 1ex; MARGIN: 0px 0px 0px 0.8ex; BORDER-LEFT: #ccc 1px solid"><br>Looks like nice work, Bryan.<br><br>Just a few minor questions/comments for now:<br><br>1. The st_fragment/vertex/geometry_program structs now have a glsl_to_tgsi field.  I did a grep, but I couldn&#39;t find where that field is assigned.  Can you clue me in?<br>
</blockquote>
<div> </div>
<div>It&#39;s assigned at the end of the get_mesa_program function in st_glsl_to_tgsi.cpp.</div>
<div> </div>
<blockquote class="gmail_quote" style="PADDING-LEFT: 1ex; MARGIN: 0px 0px 0px 0.8ex; BORDER-LEFT: #ccc 1px solid"><br>2. The above mentioned program structs contains an old Mesa instruction program AND/OR(?) a GLSL IR.  Do both types of representations co-exist sometimes?  Perhaps you could update the comments on those structs to explain that.<br>
</blockquote>
<div> </div>
<div>They used to co-exist, because after my first commit, st_glsl_to_tgsi still generated Mesa IR in addition to TGSI.  But I removed the Mesa IR generation in &quot;st/mesa: stop generating Mesa IR in st_glsl_to_tgsi&quot;, so now it has either one or the other - glsl_to_tgsi_visitor for GLSL shaders, Mesa IR programs for everything else.</div>

<div> </div>
<blockquote class="gmail_quote" style="PADDING-LEFT: 1ex; MARGIN: 0px 0px 0px 0.8ex; BORDER-LEFT: #ccc 1px solid"><br>3. Kind of a follow-on: for glDrawPixels and glBitmap we take the original program code (in Mesa form) and prepend extra instructions for fetching the fragment color or doing the fragment kill.  Do we always have the Mesa instructions for this?  It seems we don&#39;t normally want to generate Mesa instructions all the time but we still need them sometimes.<br>
</blockquote>
<div> </div>
<div>No, I didn&#39;t realize Mesa did that, and we don&#39;t have the Mesa instructions for GLSL programs anymore.  I&#39;m not sure what the right way to handle that is.</div>
<div> </div>
<blockquote class="gmail_quote" style="PADDING-LEFT: 1ex; MARGIN: 0px 0px 0px 0.8ex; BORDER-LEFT: #ccc 1px solid"><br>4. At least one commit message is slightly mis-named: changes to the gallium/util/tgsi/ files were labeled as &quot;softpipe&quot;.  Not a big deal, but maybe be more careful about that.<br>
</blockquote>
<div> </div>
<div>I thought only softpipe used tgsi_exec, but I&#39;ll keep in mind to be more careful in the future.</div></div>