[Mesa-dev] [PATCH V2] i965 : Optimize atom state flag checks

[Ian Romanick]

On 07/20/2017 12:57 PM, Marathe, Yogesh wrote:
> Ian,
> 
>> -----Original Message-----
>> From: mesa-dev [mailto:mesa-dev-bounces at lists.freedesktop.org] On Behalf
>> Of Ian Romanick
>> Sent: Friday, July 21, 2017 12:33 AM
>> To: Muthukumar, Aravindan <aravindan.muthukumar at intel.com>; mesa-
>> dev at lists.freedesktop.org
>> Subject: Re: [Mesa-dev] [PATCH V2] i965 : Optimize atom state flag checks
>>
>> On 07/20/2017 04:35 AM, aravindan.muthukumar at intel.com wrote:
>>> From: Aravindan Muthukumar <aravindan.muthukumar at intel.com>
>>>
>>> This patch improves CPI Rate(Cycles per Instruction) and branch
>>> mispredict for i965. The function check_state() was showing CPI
>>> retired rate.
>>>
>>> Performance stats with android:
>>> CPI retired lowered by 28% (lower is better) Branch missprediction
>>> lowered by 13% (lower is better) 3DMark improved by 2%
>>>
>>> The dissassembly doesn't show difference, although above results were
>>> observed with patch.
>>>
>>> Signed-off-by: Aravindan Muthukumar <aravindan.muthukumar at intel.com>
>>> Signedd-off-by: Yogesh Marathe <yogesh.marathe at intel.com>
>>
>>   Signed-off-by
> 
> Thanks. Will correct it. May I add you and all who commented as Reviewed-by? 
> I won't make a V3 for this since its a change in commit msg.

No.  You don't add someone's R-b unless they actually say "Reviewed-by".
 Various people still have issues with the content of this change.

>>> Tested-by: Asish <asish at intel.com>
>>> ---
>>>
>>> Changes since V1:
>>> - Removed memset() change
>>> - Changed commit message as per review comments
>>
>> This information should be in the main part of the commit message.
>>
> 
> Sure. 
> 
>>>
>>>  src/mesa/drivers/dri/i965/brw_defines.h      |  4 ++++
>>>  src/mesa/drivers/dri/i965/brw_state_upload.c | 12 ++++++++----
>>>  2 files changed, 12 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/src/mesa/drivers/dri/i965/brw_defines.h
>>> b/src/mesa/drivers/dri/i965/brw_defines.h
>>> index 2a8dbf8..8c9a510 100644
>>> --- a/src/mesa/drivers/dri/i965/brw_defines.h
>>> +++ b/src/mesa/drivers/dri/i965/brw_defines.h
>>> @@ -1687,3 +1687,7 @@ enum brw_pixel_shader_coverage_mask_mode {  #
>>> define CSDBG2_CONSTANT_BUFFER_ADDRESS_OFFSET_DISABLE (1 << 4)
>>>
>>>  #endif
>>> +
>>> +/* Checking the state of mesa and brw before emitting atoms */
>>> +#define CHECK_BRW_STATE(a,b) ((a.mesa & b.mesa) | (a.brw & b.brw))
>>> +
>>> diff --git a/src/mesa/drivers/dri/i965/brw_state_upload.c
>>> b/src/mesa/drivers/dri/i965/brw_state_upload.c
>>> index acaa97e..1c8b969 100644
>>> --- a/src/mesa/drivers/dri/i965/brw_state_upload.c
>>> +++ b/src/mesa/drivers/dri/i965/brw_state_upload.c
>>> @@ -443,10 +443,8 @@ check_and_emit_atom(struct brw_context *brw,
>>>                      struct brw_state_flags *state,
>>>                      const struct brw_tracked_state *atom)  {
>>> -   if (check_state(state, &atom->dirty)) {
>>>        atom->emit(brw);
>>>        merge_ctx_state(brw, state);
>>> -   }
>>>  }
>>>
>>>  static inline void
>>> @@ -541,7 +539,10 @@ brw_upload_pipeline_state(struct brw_context *brw,
>>>  	 const struct brw_tracked_state *atom = &atoms[i];
>>>  	 struct brw_state_flags generated;
>>>
>>> -         check_and_emit_atom(brw, &state, atom);
>>> +         /* Checking the state and emitting atoms */
>>> +         if (CHECK_BRW_STATE(state, atom->dirty)) {
>>> +            check_and_emit_atom(brw, &state, atom);
>>> +         }
>>>
>>>  	 accumulate_state(&examined, &atom->dirty);
>>>
>>> @@ -558,7 +559,10 @@ brw_upload_pipeline_state(struct brw_context *brw,
>>>        for (i = 0; i < num_atoms; i++) {
>>>  	 const struct brw_tracked_state *atom = &atoms[i];
>>>
>>> -         check_and_emit_atom(brw, &state, atom);
>>> +         /* Checking the state and emitting atoms */
>>> +         if (CHECK_BRW_STATE(state, atom->dirty)) {
>>> +            check_and_emit_atom(brw, &state, atom);
>>> +         }
>>>        }
>>>     }
>>>
>>>
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev