Re: [Mesa-dev] [PATCH 1/2] radv/nir: call opt_remove_phis after trivial continues.

[Timothy Arceri]

On 13/09/17 13:52, Timothy Arceri wrote:



On 13/09/17 13:48, Dave Airlie wrote:
On 13 September 2017 at 13:42, Timothy Arceri  
wrote:


On 13/09/17 12:57, Dave Airlie wrote:


From: Dave Airlie 

With the shaders in the ssao demo, the nir_opt_if wasn't
working properly without this, after this the if gets optimised
so that loop unrolling gets called.

(loop unrolling fails due to instruction count, but at least
it gets to do that.)

Signed-off-by: Dave Airlie 
---
   src/amd/vulkan/radv_shader.c | 1 +
   1 file changed, 1 insertion(+)

diff --git a/src/amd/vulkan/radv_shader.c 
b/src/amd/vulkan/radv_shader.c

index 1e25ea3..87deb7c 100644
--- a/src/amd/vulkan/radv_shader.c
+++ b/src/amd/vulkan/radv_shader.c
@@ -129,6 +129,7 @@ radv_optimize_nir(struct nir_shader *shader)
   if (nir_opt_trivial_continues(shader)) {
   progress = true;
   NIR_PASS(progress, shader, nir_copy_prop);
+   NIR_PASS(progress, shader, 
nir_opt_remove_phis);



Any reason for not just putting this in the main nir opt loop rather 
than

inside this if?


It's already in there.

This is adding it after the second copy_prop.



Right I just noticed that. I seems i965 just calls it once but later on, 
I wonder if radv should just do the same.


Or do we need to do this before something else kicks in? If thats the case:

Reviewed-by: Timothy Arceri 




Dave.




   NIR_PASS(progress, shader, nir_opt_dce);
   }
   NIR_PASS(progress, shader, nir_opt_if);




___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 1/2] radv/nir: call opt_remove_phis after trivial continues.

[Timothy Arceri]

On 13/09/17 13:48, Dave Airlie wrote:

On 13 September 2017 at 13:42, Timothy Arceri  wrote:


On 13/09/17 12:57, Dave Airlie wrote:


From: Dave Airlie 

With the shaders in the ssao demo, the nir_opt_if wasn't
working properly without this, after this the if gets optimised
so that loop unrolling gets called.

(loop unrolling fails due to instruction count, but at least
it gets to do that.)

Signed-off-by: Dave Airlie 
---
   src/amd/vulkan/radv_shader.c | 1 +
   1 file changed, 1 insertion(+)

diff --git a/src/amd/vulkan/radv_shader.c b/src/amd/vulkan/radv_shader.c
index 1e25ea3..87deb7c 100644
--- a/src/amd/vulkan/radv_shader.c
+++ b/src/amd/vulkan/radv_shader.c
@@ -129,6 +129,7 @@ radv_optimize_nir(struct nir_shader *shader)
   if (nir_opt_trivial_continues(shader)) {
   progress = true;
   NIR_PASS(progress, shader, nir_copy_prop);
+   NIR_PASS(progress, shader, nir_opt_remove_phis);



Any reason for not just putting this in the main nir opt loop rather than
inside this if?


It's already in there.

This is adding it after the second copy_prop.



Right I just noticed that. I seems i965 just calls it once but later on, 
I wonder if radv should just do the same.



Dave.




   NIR_PASS(progress, shader, nir_opt_dce);
   }
   NIR_PASS(progress, shader, nir_opt_if);




___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 1/2] radv/nir: call opt_remove_phis after trivial continues.

[Dave Airlie]

On 13 September 2017 at 13:42, Timothy Arceri  wrote:
>
> On 13/09/17 12:57, Dave Airlie wrote:
>>
>> From: Dave Airlie 
>>
>> With the shaders in the ssao demo, the nir_opt_if wasn't
>> working properly without this, after this the if gets optimised
>> so that loop unrolling gets called.
>>
>> (loop unrolling fails due to instruction count, but at least
>> it gets to do that.)
>>
>> Signed-off-by: Dave Airlie 
>> ---
>>   src/amd/vulkan/radv_shader.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/src/amd/vulkan/radv_shader.c b/src/amd/vulkan/radv_shader.c
>> index 1e25ea3..87deb7c 100644
>> --- a/src/amd/vulkan/radv_shader.c
>> +++ b/src/amd/vulkan/radv_shader.c
>> @@ -129,6 +129,7 @@ radv_optimize_nir(struct nir_shader *shader)
>>   if (nir_opt_trivial_continues(shader)) {
>>   progress = true;
>>   NIR_PASS(progress, shader, nir_copy_prop);
>> +   NIR_PASS(progress, shader, nir_opt_remove_phis);
>
>
> Any reason for not just putting this in the main nir opt loop rather than
> inside this if?

It's already in there.

This is adding it after the second copy_prop.

Dave.
>
>
>>   NIR_PASS(progress, shader, nir_opt_dce);
>>   }
>>   NIR_PASS(progress, shader, nir_opt_if);
>>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 1/2] radv/nir: call opt_remove_phis after trivial continues.

[Timothy Arceri]

On 13/09/17 12:57, Dave Airlie wrote:

From: Dave Airlie 

With the shaders in the ssao demo, the nir_opt_if wasn't
working properly without this, after this the if gets optimised
so that loop unrolling gets called.

(loop unrolling fails due to instruction count, but at least
it gets to do that.)

Signed-off-by: Dave Airlie 
---
  src/amd/vulkan/radv_shader.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/src/amd/vulkan/radv_shader.c b/src/amd/vulkan/radv_shader.c
index 1e25ea3..87deb7c 100644
--- a/src/amd/vulkan/radv_shader.c
+++ b/src/amd/vulkan/radv_shader.c
@@ -129,6 +129,7 @@ radv_optimize_nir(struct nir_shader *shader)
  if (nir_opt_trivial_continues(shader)) {
  progress = true;
  NIR_PASS(progress, shader, nir_copy_prop);
+   NIR_PASS(progress, shader, nir_opt_remove_phis);


Any reason for not just putting this in the main nir opt loop rather 
than inside this if?



  NIR_PASS(progress, shader, nir_opt_dce);
  }
  NIR_PASS(progress, shader, nir_opt_if);


___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev