Re: [Mesa-dev] [PATCH] intel: compiler: remove dead local variables at optimization pass
Thanks for the prompt reply, Jason. I will skip this patch, then. On Tue, 2018-09-04 at 15:51 -0500, Jason Ekstrand wrote: > Doesn't matter too much > > On Tue, Sep 4, 2018 at 3:45 PM Andres Gomez wrote: > > Lionel, should we also include this in the stable queues ? > > > > > > On Mon, 2018-09-03 at 16:47 +0100, Lionel Landwerlin 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. > > > > > > v2: Move additional local variable removal out of optimization loop, > > > but before large constant removal (Jason/Lionel) > > > > > > Signed-off-by: Lionel Landwerlin > > > --- > > > src/intel/compiler/brw_nir.c | 5 + > > > 1 file changed, 5 insertions(+) > > > > > > diff --git a/src/intel/compiler/brw_nir.c b/src/intel/compiler/brw_nir.c > > > index ef5034d1e1e..8c1bcb99f8c 100644 > > > --- a/src/intel/compiler/brw_nir.c > > > +++ b/src/intel/compiler/brw_nir.c > > > @@ -673,6 +673,11 @@ brw_preprocess_nir(const struct brw_compiler > > > *compiler, nir_shader *nir) > > > > > > nir = brw_nir_optimize(nir, compiler, is_scalar, true); > > > > > > + /* Workaround Gfxbench unused local sampler variable which will > > > trigger an > > > +* assert in the opt_large_constants pass. > > > +*/ > > > + OPT(nir_remove_dead_variables, nir_var_local); > > > + > > > /* This needs to be run after the first optimization pass but before > > > we > > > * lower indirect derefs away > > > */ -- Br, Andres ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] intel: compiler: remove dead local variables at optimization pass
Doesn't matter too much On Tue, Sep 4, 2018 at 3:45 PM Andres Gomez wrote: > Lionel, should we also include this in the stable queues ? > > > On Mon, 2018-09-03 at 16:47 +0100, Lionel Landwerlin 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. > > > > v2: Move additional local variable removal out of optimization loop, > > but before large constant removal (Jason/Lionel) > > > > Signed-off-by: Lionel Landwerlin > > --- > > src/intel/compiler/brw_nir.c | 5 + > > 1 file changed, 5 insertions(+) > > > > diff --git a/src/intel/compiler/brw_nir.c b/src/intel/compiler/brw_nir.c > > index ef5034d1e1e..8c1bcb99f8c 100644 > > --- a/src/intel/compiler/brw_nir.c > > +++ b/src/intel/compiler/brw_nir.c > > @@ -673,6 +673,11 @@ brw_preprocess_nir(const struct brw_compiler > *compiler, nir_shader *nir) > > > > nir = brw_nir_optimize(nir, compiler, is_scalar, true); > > > > + /* Workaround Gfxbench unused local sampler variable which will > trigger an > > +* assert in the opt_large_constants pass. > > +*/ > > + OPT(nir_remove_dead_variables, nir_var_local); > > + > > /* This needs to be run after the first optimization pass but before > we > > * lower indirect derefs away > > */ > -- > Br, > > Andres > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] intel: compiler: remove dead local variables at optimization pass
Lionel, should we also include this in the stable queues ? On Mon, 2018-09-03 at 16:47 +0100, Lionel Landwerlin 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. > > v2: Move additional local variable removal out of optimization loop, > but before large constant removal (Jason/Lionel) > > Signed-off-by: Lionel Landwerlin > --- > src/intel/compiler/brw_nir.c | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/src/intel/compiler/brw_nir.c b/src/intel/compiler/brw_nir.c > index ef5034d1e1e..8c1bcb99f8c 100644 > --- a/src/intel/compiler/brw_nir.c > +++ b/src/intel/compiler/brw_nir.c > @@ -673,6 +673,11 @@ brw_preprocess_nir(const struct brw_compiler *compiler, > nir_shader *nir) > > nir = brw_nir_optimize(nir, compiler, is_scalar, true); > > + /* Workaround Gfxbench unused local sampler variable which will trigger an > +* assert in the opt_large_constants pass. > +*/ > + OPT(nir_remove_dead_variables, nir_var_local); > + > /* This needs to be run after the first optimization pass but before we > * lower indirect derefs away > */ -- Br, Andres ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] intel: compiler: remove dead local variables at optimization pass
On 03/09/2018 17:15, Jason Ekstrand wrote: On September 3, 2018 10:47:11 Lionel Landwerlin 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. v2: Move additional local variable removal out of optimization loop, but before large constant removal (Jason/Lionel) Oh, I meant to make it the last thing brw_optimize_nir does because the other call to this pass is also right after brw_optimize_nir. This is also fine. Reviewed-by: Jason Ekstrand Oh right, I'll update. Signed-off-by: Lionel Landwerlin --- src/intel/compiler/brw_nir.c | 5 + 1 file changed, 5 insertions(+) diff --git a/src/intel/compiler/brw_nir.c b/src/intel/compiler/brw_nir.c index ef5034d1e1e..8c1bcb99f8c 100644 --- a/src/intel/compiler/brw_nir.c +++ b/src/intel/compiler/brw_nir.c @@ -673,6 +673,11 @@ brw_preprocess_nir(const struct brw_compiler *compiler, nir_shader *nir) nir = brw_nir_optimize(nir, compiler, is_scalar, true); + /* Workaround Gfxbench unused local sampler variable which will trigger an + * assert in the opt_large_constants pass. + */ + OPT(nir_remove_dead_variables, nir_var_local); + /* This needs to be run after the first optimization pass but before we * lower indirect derefs away */ -- 2.19.0.rc1 ___ 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] intel: compiler: remove dead local variables at optimization pass
On September 3, 2018 10:47:11 Lionel Landwerlin 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. v2: Move additional local variable removal out of optimization loop, but before large constant removal (Jason/Lionel) Oh, I meant to make it the last thing brw_optimize_nir does because the other call to this pass is also right after brw_optimize_nir. This is also fine. Reviewed-by: Jason Ekstrand Signed-off-by: Lionel Landwerlin --- src/intel/compiler/brw_nir.c | 5 + 1 file changed, 5 insertions(+) diff --git a/src/intel/compiler/brw_nir.c b/src/intel/compiler/brw_nir.c index ef5034d1e1e..8c1bcb99f8c 100644 --- a/src/intel/compiler/brw_nir.c +++ b/src/intel/compiler/brw_nir.c @@ -673,6 +673,11 @@ brw_preprocess_nir(const struct brw_compiler *compiler, nir_shader *nir) nir = brw_nir_optimize(nir, compiler, is_scalar, true); + /* Workaround Gfxbench unused local sampler variable which will trigger an +* assert in the opt_large_constants pass. +*/ + OPT(nir_remove_dead_variables, nir_var_local); + /* This needs to be run after the first optimization pass but before we * lower indirect derefs away */ -- 2.19.0.rc1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] intel: compiler: remove dead local variables at optimization pass
On 03/09/2018 16:47, Lionel Landwerlin 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. v2: Move additional local variable removal out of optimization loop, but before large constant removal (Jason/Lionel) Signed-off-by: Lionel Landwerlin Forgot to add Eero's bug : https://bugs.freedesktop.org/show_bug.cgi?id=107806 --- src/intel/compiler/brw_nir.c | 5 + 1 file changed, 5 insertions(+) diff --git a/src/intel/compiler/brw_nir.c b/src/intel/compiler/brw_nir.c index ef5034d1e1e..8c1bcb99f8c 100644 --- a/src/intel/compiler/brw_nir.c +++ b/src/intel/compiler/brw_nir.c @@ -673,6 +673,11 @@ brw_preprocess_nir(const struct brw_compiler *compiler, nir_shader *nir) nir = brw_nir_optimize(nir, compiler, is_scalar, true); + /* Workaround Gfxbench unused local sampler variable which will trigger an +* assert in the opt_large_constants pass. +*/ + OPT(nir_remove_dead_variables, nir_var_local); + /* This needs to be run after the first optimization pass but before we * lower indirect derefs away */ ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] intel: compiler: remove dead local variables at optimization pass
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. v2: Move additional local variable removal out of optimization loop, but before large constant removal (Jason/Lionel) Signed-off-by: Lionel Landwerlin --- src/intel/compiler/brw_nir.c | 5 + 1 file changed, 5 insertions(+) diff --git a/src/intel/compiler/brw_nir.c b/src/intel/compiler/brw_nir.c index ef5034d1e1e..8c1bcb99f8c 100644 --- a/src/intel/compiler/brw_nir.c +++ b/src/intel/compiler/brw_nir.c @@ -673,6 +673,11 @@ brw_preprocess_nir(const struct brw_compiler *compiler, nir_shader *nir) nir = brw_nir_optimize(nir, compiler, is_scalar, true); + /* Workaround Gfxbench unused local sampler variable which will trigger an +* assert in the opt_large_constants pass. +*/ + OPT(nir_remove_dead_variables, nir_var_local); + /* This needs to be run after the first optimization pass but before we * lower indirect derefs away */ -- 2.19.0.rc1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] intel: compiler: remove dead local variables at optimization pass
On Thu, Aug 23, 2018 at 8:38 AM Lionel Landwerlin < lionel.g.landwer...@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 > --- > 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 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] intel: compiler: remove dead local variables at optimization pass
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 --- 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); } while (progress); return nir; -- 2.18.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
