On Fri, Jun 5, 2015 at 1:54 PM, Francisco Jerez <curroje...@riseup.net> wrote: > Francisco Jerez <curroje...@riseup.net> writes: > >> Matt Turner <matts...@gmail.com> writes: >> >>> On Thu, Jun 4, 2015 at 9:04 AM, Francisco Jerez <curroje...@riseup.net> >>> wrote: >>>> --- >>>> src/mesa/drivers/dri/i965/brw_fs.cpp | 11 +++++++++++ >>>> src/mesa/drivers/dri/i965/brw_fs.h | 2 ++ >>>> src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 4 +++- >>>> 3 files changed, 16 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp >>>> b/src/mesa/drivers/dri/i965/brw_fs.cpp >>>> index 28a19bd..c1dd0a6 100644 >>>> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp >>>> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp >>>> @@ -3986,6 +3986,17 @@ fs_visitor::calculate_register_pressure() >>>> void >>>> fs_visitor::optimize() >>>> { >>>> + /* bld is the common builder object pointing at the end of the program >>>> we >>>> + * used to translate it into i965 IR. For the optimization and >>>> lowering >>>> + * passes coming next, any code added after the end of the program >>>> without >>>> + * having explicitly called fs_builder::at() clearly points at a >>>> mistake. >>>> + * Ideally optimization passes wouldn't be part of the visitor so they >>>> + * wouldn't have access to bld at all, but they do, so just in case >>>> some >>>> + * pass forgets to ask for a location explicitly set it to NULL here to >>>> + * make it trip. >>>> + */ >>>> + bld = bld.at(NULL, NULL); >>> >>> I like it. I know I've wasted a bunch of time in the last by >>> emit()'ing an instruction in an optimization instead of inserting it. >>> This should make that class of mistakes really simple to debug. >>> >>> But I'm not sure what your plan is for the builder in optimization >>> passes (I mean beyond this series)? I agree that it'd be nice to >>> separate the translation into the backend IR from the optimization >>> passes, but how could we ever remove access to the builder from the >>> optimization passes? They're of course going to need to insert >>> instructions. >> >> I had two possibilities in mind: We could pass the optimization passes a >> backend_shader pointer only, and let them create their own builder (what >> would require adding a dispatch_width field to backend_shader which >> seems like a good idea anyway), or we could pass them a builder pointing >> at the NULL instruction, kind of like what this patch does. > > Hmm, I think I'm going to go change the constructor of fs_builder to > initialize cursor to NULL by default instead of to the end of the > program, in anticipation of these two possibilities.
Seems fine to me. _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev