intel: compiler: remove dead local variables at optimization pass

Submitted by Lionel Landwerlin on Aug. 23, 2018, 1:37 p.m.

Details

Message ID 20180823133722.32549-1-lionel.g.landwerlin@intel.com
State New
Series "intel: compiler: remove dead local variables at optimization pass"
Headers show

Commit Message

Lionel Landwerlin Aug. 23, 2018, 1:37 p.m.
We're hitting an assert in gfxbench because one of the local variable
is a sampler (according to Jason this isn't valid) :

testfw_app: ../src/compiler/nir_types.cpp:551: void glsl_get_natural_size_align_bytes(const glsl_type*, unsigned int*, unsigned int*): Assertion `!"type does not have a natural size"' failed.

Since this particular variable isn't used and it can be eliminated by
removing unused local variables in the optimization pass. This makes
sense also for valid local variables.

Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
---
 src/intel/compiler/brw_nir.c | 1 +
 1 file changed, 1 insertion(+)

Patch hide | download patch | download mbox

diff --git a/src/intel/compiler/brw_nir.c b/src/intel/compiler/brw_nir.c
index 29ad68fdb2a..ede341c8f3a 100644
--- a/src/intel/compiler/brw_nir.c
+++ b/src/intel/compiler/brw_nir.c
@@ -586,6 +586,7 @@  brw_nir_optimize(nir_shader *nir, const struct brw_compiler *compiler,
                              nir_lower_dround_even |
                              nir_lower_dmod);
       OPT(nir_lower_pack);
+      OPT(nir_remove_dead_variables, nir_var_local);
    } while (progress);
 
    return nir;

Comments

Jason Ekstrand Sept. 3, 2018, 2:09 p.m.
On Thu, Aug 23, 2018 at 8:38 AM Lionel Landwerlin <
lionel.g.landwerlin@intel.com> wrote:

> We're hitting an assert in gfxbench because one of the local variable
> is a sampler (according to Jason this isn't valid) :
>
> testfw_app: ../src/compiler/nir_types.cpp:551: void
> glsl_get_natural_size_align_bytes(const glsl_type*, unsigned int*, unsigned
> int*): Assertion `!"type does not have a natural size"' failed.
>
> Since this particular variable isn't used and it can be eliminated by
> removing unused local variables in the optimization pass. This makes
> sense also for valid local variables.
>
> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> ---
>  src/intel/compiler/brw_nir.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/src/intel/compiler/brw_nir.c b/src/intel/compiler/brw_nir.c
> index 29ad68fdb2a..ede341c8f3a 100644
> --- a/src/intel/compiler/brw_nir.c
> +++ b/src/intel/compiler/brw_nir.c
> @@ -586,6 +586,7 @@ brw_nir_optimize(nir_shader *nir, const struct
> brw_compiler *compiler,
>                               nir_lower_dround_even |
>                               nir_lower_dmod);
>        OPT(nir_lower_pack);
> +      OPT(nir_remove_dead_variables, nir_var_local);
>

I'm not sure how much it actually helps to have this in the optimization
loop and it is an extra (though fairly cheap) pass over the IR.  Maybe we
should just put it right before opt_large_constants instead?  Or, better
yet, just put it the end of brw_nir_optimize after the loop and then we can
remove the other call to it that happens at the end of brw_preprocess_nir.

--Jason