[Mesa-dev] [RFC: PATCH 4/4] i965: Refactor all upload_<stage>_prog functions up into brw_upload_programs

Carl Worth cworth at cworth.org
Fri May 15 17:09:56 PDT 2015


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



More information about the mesa-dev mailing list