[Mesa-dev] [PATCH 1/1] i965: Do not overwrite optimizer dumps

[Jason Ekstrand]

On Wed, Nov 25, 2015 at 10:52 AM, Matt Turner <mattst88 at gmail.com> wrote:
> On Wed, Nov 25, 2015 at 10:46 AM, Jason Ekstrand <jason at jlekstrand.net> wrote:
>> On Wed, Nov 25, 2015 at 10:45 AM, Matt Turner <mattst88 at gmail.com> wrote:
>>> On Wed, Nov 25, 2015 at 10:37 AM, Jason Ekstrand <jason at jlekstrand.net> wrote:
>>>> On Wed, Nov 25, 2015 at 10:31 AM, Matt Turner <mattst88 at gmail.com> wrote:
>>>>> On Wed, Nov 25, 2015 at 4:15 AM, Juan A. Suarez Romero
>>>>> <jasuarez at igalia.com> wrote:
>>>>>> When using INTEL_DEBUG=optimizer, each optimizing step is dump to disk,
>>>>>> in a separate file.
>>>>>>
>>>>>> But as fs_visitor::optimize() and vec4_visitor::run() are called more
>>>>>> than once, it ends up overwriting the files already on disk, loosing
>>>>>> then previous optimizer steps.
>>>>>
>>>>> Huh. I guess this happens when non-orthogonal state changes and we
>>>>> recompile the program?
>>>>>
>>>>> If so, yeah, that would lead to some confusing results.
>>>>>
>>>>>> To avoid this, add a new static variable that tracks the global
>>>>>> iteration across the entire life of the program running.
>>>>>> ---
>>>>>>  src/mesa/drivers/dri/i965/brw_fs.cpp   | 13 +++++++++----
>>>>>>  src/mesa/drivers/dri/i965/brw_vec4.cpp | 11 +++++++----
>>>>>>  2 files changed, 16 insertions(+), 8 deletions(-)
>>>>>>
>>>>>> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
>>>>>> index 29f19cc..9520a62 100644
>>>>>> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
>>>>>> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
>>>>>> @@ -4947,6 +4947,8 @@ fs_visitor::calculate_register_pressure()
>>>>>>  void
>>>>>>  fs_visitor::optimize()
>>>>>>  {
>>>>>> +   static int global_iteration = 0;
>>>>>
>>>>> I don't know that adding a static variable is the way to solve this. I
>>>>> know this is debugging code, but using a static variable will make
>>>>> this thread-unsafe, and I *really* don't want to end up in a situation
>>>>> where I can't figure out what the optimizer is doing because we were
>>>>> compiling shaders in parallel...
>>>>
>>>> If we really care, we can use an atomic, but meh.  How often are you
>>>> seriously using INTEL_DEBUG=optimizer on something that's compiling
>>>> enough shaders in parallel for this to realistically be a problem?  I
>>>> wouldn't want to dig through that many results.  But, like I said, if
>>>> it bothers you, make it an atomic.
>>>
>>> That doesn't help.
>>
>> Then what thread-saftey issue are you concerned with here?
>
> The code *reads* a global variable that might be changed by another
> thread during the first thread's optimization loop.

Right.  I didn't pay that much attention to the exact implementation.
But we could do something like.

atomic int global_iteration_atomic = 0;

const int global_iteration = atomic_inc(global_iteration_atomic);

// use global_iteration everywhere

That wouldn't have the threading issues.  (Note, the above is pseudo-code)
--Jason