Re: [Mesa-dev] [PATCH 25/28] i965/fs: add support for shader float control to remove_extra_rounding_modes()
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
>>> ---
>>> 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);
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 25/28] i965/fs: add support for shader float control to remove_extra_rounding_modes()
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
>> ---
>> 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);
pEpkey.asc
Description: application/pgp-keys
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 25/28] i965/fs: add support for shader float control to remove_extra_rounding_modes()
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
> ---
> 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.
>
> + 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);
pEpkey.asc
Description: application/pgp-keys
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 25/28] i965/fs: add support for shader float control to remove_extra_rounding_modes()
The remove_extra_rounding_modes() optimization will remove duplicated
rounding mode changes.
Signed-off-by: Samuel Iglesias Gonsálvez
---
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;
+ 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);
--
2.19.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
