On Sun, Apr 13, 2014 at 12:16 AM, Kenneth Graunke <kenn...@whitecape.org> wrote:
> On 04/11/2014 10:29 PM, Matt Turner wrote:
>> With INTEL_DEBUG=optimizer, write the output of dump_instructions() to a
>> file each time an optimization pass makes progress. This lets you easily
>> diff successive files to see what an optimization pass did.
>>
>> Example filenames written when running glxgears:
>> fs8-00-00-start
>> fs8-00-01-04-opt_copy_propagate
>> fs8-00-01-06-dead_code_eliminate
>> fs8-00-01-12-compute_to_mrf
>> fs8-00-02-06-dead_code_eliminate
>> | | | |
>> | | | `-- optimization pass name
>> | | |
>> | | `-- optimization pass number in the loop
>> | |
>> | `-- optimization loop interation
>> |
>> `-- shader program number
>>
>> Note that with INTEL_DEBUG=optimizer, we disable compact_virtual_grfs,
>> so that we can diff instruction lists across loop interations without
>> the register numbers being changes.
>> ---
>> src/mesa/drivers/dri/i965/brw_fs.cpp | 56
>> +++++++++++++++++++++++++++---------
>> 1 file changed, 43 insertions(+), 13 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp
>> b/src/mesa/drivers/dri/i965/brw_fs.cpp
>> index c1d79e1..666cfa5b 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
>> @@ -1704,6 +1704,9 @@ fs_visitor::split_virtual_grfs()
>> void
>> fs_visitor::compact_virtual_grfs()
>> {
>> + if (unlikely(INTEL_DEBUG & DEBUG_OPTIMIZER))
>> + return;
>> +
>> /* Mark which virtual GRFs are used, and count how many. */
>> int remap_table[this->virtual_grf_count];
>> memset(remap_table, -1, sizeof(remap_table));
>> @@ -3258,25 +3261,52 @@ fs_visitor::run()
>>
>> opt_drop_redundant_mov_to_flags();
>>
>> +#define OPT(pass, args...) \
>> + ({ \
>> + pass_num++; \
>> + bool progress = pass(args); \
>> + \
>> + if (unlikely(INTEL_DEBUG & DEBUG_OPTIMIZER) && progress) { \
>> + char filename[64]; \
>> + snprintf(filename, 64, "fs%d-%02d-%02d-%02d-" #pass, \
>> + dispatch_width, shader_prog->Name, iteration, pass_num); \
>> + \
>> + backend_visitor::dump_instructions(filename);
>> \
>> + } \
>> + \
>> + progress; \
>> + })
>
> I like the use of the macro. But...folding in the "progress = ... ||
> progress" might be nice too. It would also let us stick to standard C
> here, rather than GNU extensions.
I don't see a problem with using extensions, especially when we
already use them. But decluttering the loop seems like a good idea.
>> +
>> + if (unlikely(INTEL_DEBUG & DEBUG_OPTIMIZER)) {
>> + char filename[64];
>> + snprintf(filename, 64, "fs%d-%02d-00-start",
>> + dispatch_width, shader_prog->Name);
>
> I am really unexcited about fixed length buffers, although you did do
> snprintf, and it'll probably not be a problem in practice. Why not just
> asprintf or ralloc_asprintf it and then free it? It's no more code and
> will just work.
Given the lengths of our optimization pass names, 64 seems like
plenty, and if it's not that worst that happens is we lose a few
characters from our optimization pass's name. My compiler complains
about not checking the return type of asprintf, and adding a memory
allocation failure check in the macro felt really stupid.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
