[Mesa-dev] [PATCH 03/38] i965/fs: Allocate a common IR builder object in fs_visitor.

[Francisco Jerez]

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()
>>  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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150605/ad02de69/attachment.sig>