Re: [Mesa-dev] [PATCH v2 3/4] intel/nir: Split IO arrays into elements
On Tuesday, July 31, 2018 8:55:05 PM PDT Jason Ekstrand wrote:
> On Tue, Jul 31, 2018 at 3:00 PM Jason Ekstrand wrote:
>
> > On July 31, 2018 14:54:54 Kenneth Graunke wrote:
> >
> > > On Tuesday, July 31, 2018 12:22:02 PM PDT Jason Ekstrand wrote:
> > >> The NIR nir_lower_io_arrays_to_elements pass attempts to split I/O
> > >> variables which are arrays or matrices into a sequence of separate
> > >> variables. This can help link-time optimization by allowing us to
> > >> remove varyings at a more granular level.
> > >>
> > >> Shader-db results on Kaby Lake:
> > >>
> > >> total instructions in shared programs: 15177645 -> 15168494 (-0.06%)
> > >> instructions in affected programs: 79857 -> 70706 (-11.46%)
> > >> helped: 392
> > >> HURT: 0
> > >> ---
> > >> src/intel/compiler/brw_nir.c | 4
> > >> 1 file changed, 4 insertions(+)
> > >>
> > >> diff --git a/src/intel/compiler/brw_nir.c b/src/intel/compiler/brw_nir.c
> > >> index 17ccfa48af6..29ad68fdb2a 100644
> > >> --- a/src/intel/compiler/brw_nir.c
> > >> +++ b/src/intel/compiler/brw_nir.c
> > >> @@ -709,6 +709,10 @@ void
> > >> brw_nir_link_shaders(const struct brw_compiler *compiler,
> > >> nir_shader **producer, nir_shader **consumer)
> > >> {
> > >> + nir_lower_io_arrays_to_elements(*producer, *consumer);
> > >> + nir_validate_shader(*producer);
> > >> + nir_validate_shader(*consumer);
> > >> +
> > >> NIR_PASS_V(*producer, nir_remove_dead_variables, nir_var_shader_out);
> > >> NIR_PASS_V(*consumer, nir_remove_dead_variables, nir_var_shader_in);
> > >
> > > This actually passes the test suite? That's strange. When I was
> > > playing with this earlier, it was demoting gl_ClipDistance[1] from
> > > a float[1] compact array to a single float variable...but leaving it
> > > marked compact. var->data.compact = true makes no sense for non-arrays.
> > >
> > > I think our clip/cull distance handling assumes that it's an array,
> > > though. So...not sure whether the pass should unset compact, or skip
> > > over compact arrays...
> >
> > Yes, it passes just fine. I'm not sure exactly how it interacts with clip
> > distance lowering though.
> >
>
> Back at my computer finally. I did a bit of digging and the
> lower_io_arrays_to_elements pass ignores everything that isn't
> VARYING_SLOT_VARX. Clip distances are ignored since they're
> VARYING_SLOT_CLIP_DIST0/1. Hope that clears it up. :-)
>
> --Jason
Huh. Not sure what I was seeing then. Probably other local breakage.
Sounds like everything should work, then.
Series is:
Reviewed-by: Kenneth Graunke
signature.asc
Description: This is a digitally signed message part.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 3/4] intel/nir: Split IO arrays into elements
On Tue, Jul 31, 2018 at 3:00 PM Jason Ekstrand wrote:
> On July 31, 2018 14:54:54 Kenneth Graunke wrote:
>
> > On Tuesday, July 31, 2018 12:22:02 PM PDT Jason Ekstrand wrote:
> >> The NIR nir_lower_io_arrays_to_elements pass attempts to split I/O
> >> variables which are arrays or matrices into a sequence of separate
> >> variables. This can help link-time optimization by allowing us to
> >> remove varyings at a more granular level.
> >>
> >> Shader-db results on Kaby Lake:
> >>
> >> total instructions in shared programs: 15177645 -> 15168494 (-0.06%)
> >> instructions in affected programs: 79857 -> 70706 (-11.46%)
> >> helped: 392
> >> HURT: 0
> >> ---
> >> src/intel/compiler/brw_nir.c | 4
> >> 1 file changed, 4 insertions(+)
> >>
> >> diff --git a/src/intel/compiler/brw_nir.c b/src/intel/compiler/brw_nir.c
> >> index 17ccfa48af6..29ad68fdb2a 100644
> >> --- a/src/intel/compiler/brw_nir.c
> >> +++ b/src/intel/compiler/brw_nir.c
> >> @@ -709,6 +709,10 @@ void
> >> brw_nir_link_shaders(const struct brw_compiler *compiler,
> >> nir_shader **producer, nir_shader **consumer)
> >> {
> >> + nir_lower_io_arrays_to_elements(*producer, *consumer);
> >> + nir_validate_shader(*producer);
> >> + nir_validate_shader(*consumer);
> >> +
> >> NIR_PASS_V(*producer, nir_remove_dead_variables, nir_var_shader_out);
> >> NIR_PASS_V(*consumer, nir_remove_dead_variables, nir_var_shader_in);
> >
> > This actually passes the test suite? That's strange. When I was
> > playing with this earlier, it was demoting gl_ClipDistance[1] from
> > a float[1] compact array to a single float variable...but leaving it
> > marked compact. var->data.compact = true makes no sense for non-arrays.
> >
> > I think our clip/cull distance handling assumes that it's an array,
> > though. So...not sure whether the pass should unset compact, or skip
> > over compact arrays...
>
> Yes, it passes just fine. I'm not sure exactly how it interacts with clip
> distance lowering though.
>
Back at my computer finally. I did a bit of digging and the
lower_io_arrays_to_elements pass ignores everything that isn't
VARYING_SLOT_VARX. Clip distances are ignored since they're
VARYING_SLOT_CLIP_DIST0/1. Hope that clears it up. :-)
--Jason
--Jason
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 3/4] intel/nir: Split IO arrays into elements
On July 31, 2018 14:54:54 Kenneth Graunke wrote:
On Tuesday, July 31, 2018 12:22:02 PM PDT Jason Ekstrand wrote:
The NIR nir_lower_io_arrays_to_elements pass attempts to split I/O
variables which are arrays or matrices into a sequence of separate
variables. This can help link-time optimization by allowing us to
remove varyings at a more granular level.
Shader-db results on Kaby Lake:
total instructions in shared programs: 15177645 -> 15168494 (-0.06%)
instructions in affected programs: 79857 -> 70706 (-11.46%)
helped: 392
HURT: 0
---
src/intel/compiler/brw_nir.c | 4
1 file changed, 4 insertions(+)
diff --git a/src/intel/compiler/brw_nir.c b/src/intel/compiler/brw_nir.c
index 17ccfa48af6..29ad68fdb2a 100644
--- a/src/intel/compiler/brw_nir.c
+++ b/src/intel/compiler/brw_nir.c
@@ -709,6 +709,10 @@ void
brw_nir_link_shaders(const struct brw_compiler *compiler,
nir_shader **producer, nir_shader **consumer)
{
+ nir_lower_io_arrays_to_elements(*producer, *consumer);
+ nir_validate_shader(*producer);
+ nir_validate_shader(*consumer);
+
NIR_PASS_V(*producer, nir_remove_dead_variables, nir_var_shader_out);
NIR_PASS_V(*consumer, nir_remove_dead_variables, nir_var_shader_in);
This actually passes the test suite? That's strange. When I was
playing with this earlier, it was demoting gl_ClipDistance[1] from
a float[1] compact array to a single float variable...but leaving it
marked compact. var->data.compact = true makes no sense for non-arrays.
I think our clip/cull distance handling assumes that it's an array,
though. So...not sure whether the pass should unset compact, or skip
over compact arrays...
Yes, it passes just fine. I'm not sure exactly how it interacts with clip
distance lowering though.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 3/4] intel/nir: Split IO arrays into elements
On Tuesday, July 31, 2018 12:22:02 PM PDT Jason Ekstrand wrote:
> The NIR nir_lower_io_arrays_to_elements pass attempts to split I/O
> variables which are arrays or matrices into a sequence of separate
> variables. This can help link-time optimization by allowing us to
> remove varyings at a more granular level.
>
> Shader-db results on Kaby Lake:
>
> total instructions in shared programs: 15177645 -> 15168494 (-0.06%)
> instructions in affected programs: 79857 -> 70706 (-11.46%)
> helped: 392
> HURT: 0
> ---
> src/intel/compiler/brw_nir.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/src/intel/compiler/brw_nir.c b/src/intel/compiler/brw_nir.c
> index 17ccfa48af6..29ad68fdb2a 100644
> --- a/src/intel/compiler/brw_nir.c
> +++ b/src/intel/compiler/brw_nir.c
> @@ -709,6 +709,10 @@ void
> brw_nir_link_shaders(const struct brw_compiler *compiler,
> nir_shader **producer, nir_shader **consumer)
> {
> + nir_lower_io_arrays_to_elements(*producer, *consumer);
> + nir_validate_shader(*producer);
> + nir_validate_shader(*consumer);
> +
> NIR_PASS_V(*producer, nir_remove_dead_variables, nir_var_shader_out);
> NIR_PASS_V(*consumer, nir_remove_dead_variables, nir_var_shader_in);
>
>
This actually passes the test suite? That's strange. When I was
playing with this earlier, it was demoting gl_ClipDistance[1] from
a float[1] compact array to a single float variable...but leaving it
marked compact. var->data.compact = true makes no sense for non-arrays.
I think our clip/cull distance handling assumes that it's an array,
though. So...not sure whether the pass should unset compact, or skip
over compact arrays...
signature.asc
Description: This is a digitally signed message part.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2 3/4] intel/nir: Split IO arrays into elements
The NIR nir_lower_io_arrays_to_elements pass attempts to split I/O
variables which are arrays or matrices into a sequence of separate
variables. This can help link-time optimization by allowing us to
remove varyings at a more granular level.
Shader-db results on Kaby Lake:
total instructions in shared programs: 15177645 -> 15168494 (-0.06%)
instructions in affected programs: 79857 -> 70706 (-11.46%)
helped: 392
HURT: 0
---
src/intel/compiler/brw_nir.c | 4
1 file changed, 4 insertions(+)
diff --git a/src/intel/compiler/brw_nir.c b/src/intel/compiler/brw_nir.c
index 17ccfa48af6..29ad68fdb2a 100644
--- a/src/intel/compiler/brw_nir.c
+++ b/src/intel/compiler/brw_nir.c
@@ -709,6 +709,10 @@ void
brw_nir_link_shaders(const struct brw_compiler *compiler,
nir_shader **producer, nir_shader **consumer)
{
+ nir_lower_io_arrays_to_elements(*producer, *consumer);
+ nir_validate_shader(*producer);
+ nir_validate_shader(*consumer);
+
NIR_PASS_V(*producer, nir_remove_dead_variables, nir_var_shader_out);
NIR_PASS_V(*consumer, nir_remove_dead_variables, nir_var_shader_in);
--
2.17.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
