Yes. That patch was severely broken in several ways. In its place, here is a pair of patches intending to do the same thing in two steps. Thanks to Ken for helpful review and guidance to get to this point.
At this point, I'm quite confident that the logic of the code is undisturbed by the refactoring of the following two pages. And to the extent that our jenkins-based testing with piglit over a variety of platforms is accurate, it agrees. That said, there are some pieces of the final code that look somewhat ugly, (see the need_ff_gs condition or any of the four "continue" statements within the loop within brw_upload_programs). I don't think any of this ugliness is a reason to reject these patches---the exact ugliness exists already, (but is a bit more hidden since this code is split across a handful of files). I do think the ugliness could help motivate some further cleanups here if anyone is interested. (I would have done more, but I'm running out of my depth to some extent. It was easy for me to leave the logic unchanged, and it would be harder for me to change the logic in ways that would look cleaner, but would leave the final effects of the code equivalent.) For example, Ken looked at the final result and pondered whether it would make sense to merge the FF_GS and GS stages somehow, (notice that the need_ff_gs condition ensures that only one or the other is used here, never both). Aside from those cleanups, though, as I discuss in the commit message of the second patch, for the shader-cache, what I still need it to be able to pull out the "per_stage_codegen" from the loop in brw_upload_porograms and place it into its own loop after the first. Between the loops, then I would have the perfect place to insert a call for shader-cache lookups. It's the code within the per_stage_vue_map_update function that makes it non-trivial to do this last bit of refactoring. Ken has some ideas about moving the vue_map calculation away from here and to link-time instead. It would be great if he could do that. In the meantime, I may experiment with a less-invasive change to the vue_map code that would allow me to refactor this function to allow the shader-cache to live here like I want it to. -Carl _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
