[Mesa-dev] [PATCH 25/28] i965/fs: add support for shader float control to remove_extra_rounding_modes()

[Samuel Iglesias Gonsálvez]

On 06/12/2018 08:40, apinheiro wrote:
> 
> On 6/12/18 8:37, apinheiro wrote:
>> On 5/12/18 16:55, Samuel Iglesias Gonsálvez wrote:
>>> The remove_extra_rounding_modes() optimization will remove duplicated
>>> rounding mode changes.
>>>
>>> Signed-off-by: Samuel Iglesias Gonsálvez <siglesias at igalia.com>
>>> ---
>>>  src/intel/compiler/brw_fs.cpp | 9 +++++++--
>>>  1 file changed, 7 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/src/intel/compiler/brw_fs.cpp b/src/intel/compiler/brw_fs.cpp
>>> index 18dcd92219c..eb253679930 100644
>>> --- a/src/intel/compiler/brw_fs.cpp
>>> +++ b/src/intel/compiler/brw_fs.cpp
>>> @@ -3457,10 +3457,15 @@ bool
>>>  fs_visitor::remove_extra_rounding_modes()
>>>  {
>>>     bool progress = false;
>>> +   unsigned execution_mode = this->nir->info.shader_float_controls_execution_mode;
>>>  
>>> -   foreach_block (block, cfg) {
>>> -      brw_rnd_mode prev_mode = BRW_RND_MODE_UNSPECIFIED;
>>> +   brw_rnd_mode prev_mode = BRW_RND_MODE_UNSPECIFIED;
>>> +   if (execution_mode & SHADER_ROUNDING_MODE_RTE)
>>> +      prev_mode = BRW_RND_MODE_RTNE;
>>> +   if (execution_mode & SHADER_ROUNDING_MODE_RTZ)
>>> +      prev_mode = BRW_RND_MODE_RTZ;
>> I see that you move prev_mode reset out of block. This needs to be reset
>> for each block, and it is a mistake that I also committed for the v1 of
>> this optimization. See original review:
>>
>> https://lists.freedesktop.org/archives/mesa-dev/2017-September/168970.html
>>
>> Unless I'm missing something, Jason comments still applies.
> 
> 
> Having said so, most of that code is block-independent, so perhaps you
> could add a new variable "base_mode" or something, so that code
> initializes base_mode, and prev_mode remains at the beginning of the
> block, but initialized to base_mode instead to UNSPECIFIED.
> 

Thanks, I will do it.

Sam

> 
>>
>>
>>>  
>>> +   foreach_block (block, cfg) {
>>>        foreach_inst_in_block_safe (fs_inst, inst, block) {
>>>           if (inst->opcode == SHADER_OPCODE_RND_MODE) {
>>>              assert(inst->src[0].file == BRW_IMMEDIATE_VALUE);