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

[apinheiro]

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.


>
>
>>  
>> +   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);
-------------- next part --------------
A non-text attachment was scrubbed...
Name: pEpkey.asc
Type: application/pgp-keys
Size: 1546 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20181206/99ec3ed6/attachment.key>