[Mesa-dev] [PATCH 03/38] i965/fs: Allocate a common IR builder object in fs_visitor.
mattst88 at gmail.com
Fri Jun 5 14:19:56 PDT 2015
On Fri, Jun 5, 2015 at 1:54 PM, Francisco Jerez <currojerez at riseup.net> wrote:
> Francisco Jerez <currojerez at riseup.net> writes:
>> Matt Turner <mattst88 at gmail.com> writes:
>>> On Thu, Jun 4, 2015 at 9:04 AM, Francisco Jerez <currojerez at 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()
>>>> + /* 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
>> 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.
More information about the mesa-dev