Re: [Mesa-dev] [PATCH v2 3/3] glsl: disable array splitting for AoA
I sent a replacement for patch 1. Patches 2 and 3 are
Reviewed-by: Jason Ekstrand
We should CC stable on my replacement for 1 and 2 because these affect
Vulkan even without the AoA splitting patch.
--Jason
On Sat, Jul 15, 2017 at 12:48 AM, Timothy Arceri
wrote:
> On Mon, Jul 3, 2017, at 10:16 AM, Jason Ekstrand wrote:
> > I'd like the chance to look at these, please. I'm on vacation today and
> > tomorrow though.
>
> Ping!
>
> >
> >
> > On July 3, 2017 7:19:04 AM funfunc...@folklore1984.net wrote:
> >
> > > On 2017-07-03 08:47, Timothy Arceri wrote:
> > >> While it produces functioning code the pass creates worse code
> > >> for arrays of arrays. See the comment added in this patch for more
> > >> detail.
> > >>
> > >> V2: skip splitting of AoA of matrices too.
> > >
> > > Reviewed-by: Edward O'Callaghan
> > >
> > >> ---
> > >> src/compiler/glsl/opt_array_splitting.cpp | 23
> +++
> > >> 1 file changed, 23 insertions(+)
> > >>
> > >> diff --git a/src/compiler/glsl/opt_array_splitting.cpp
> > >> b/src/compiler/glsl/opt_array_splitting.cpp
> > >> index fb6d77b..d2e81665 100644
> > >> --- a/src/compiler/glsl/opt_array_splitting.cpp
> > >> +++ b/src/compiler/glsl/opt_array_splitting.cpp
> > >> @@ -140,6 +140,29 @@
> > >> ir_array_reference_visitor::get_variable_entry(ir_variable *var)
> > >> if (var->type->is_unsized_array())
> > >>return NULL;
> > >>
> > >> + /* FIXME: arrays of arrays are not handled correctly by this pass
> > >> so we
> > >> +* skip it for now. While the pass will create functioning code it
> > >> actually
> > >> +* produces worse code.
> > >> +*
> > >> +* For example the array:
> > >> +*
> > >> +*int[3][2] a;
> > >> +*
> > >> +* ends up being split up into:
> > >> +*
> > >> +*int[3][2] a_0;
> > >> +*int[3][2] a_1;
> > >> +*int[3][2] a_2;
> > >> +*
> > >> +* And we end up referencing each of these new arrays for example:
> > >> +*
> > >> +*a[0][1] will be turned into a_0[0][1]
> > >> +*a[1][0] will be turned into a_1[1][0]
> > >> +*a[2][0] will be turned into a_2[2][0]
> > >> +*/
> > >> + if (var->type->is_array() && var->type->fields.array->is_array())
> > >> + return NULL;
> > >> +
> > >> foreach_in_list(variable_entry, entry, &this->variable_list) {
> > >>if (entry->var == var)
> > >> return entry;
> > >
> > > ___
> > > 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 v2 3/3] glsl: disable array splitting for AoA
On Mon, Jul 3, 2017, at 10:16 AM, Jason Ekstrand wrote:
> I'd like the chance to look at these, please. I'm on vacation today and
> tomorrow though.
Ping!
>
>
> On July 3, 2017 7:19:04 AM funfunc...@folklore1984.net wrote:
>
> > On 2017-07-03 08:47, Timothy Arceri wrote:
> >> While it produces functioning code the pass creates worse code
> >> for arrays of arrays. See the comment added in this patch for more
> >> detail.
> >>
> >> V2: skip splitting of AoA of matrices too.
> >
> > Reviewed-by: Edward O'Callaghan
> >
> >> ---
> >> src/compiler/glsl/opt_array_splitting.cpp | 23 +++
> >> 1 file changed, 23 insertions(+)
> >>
> >> diff --git a/src/compiler/glsl/opt_array_splitting.cpp
> >> b/src/compiler/glsl/opt_array_splitting.cpp
> >> index fb6d77b..d2e81665 100644
> >> --- a/src/compiler/glsl/opt_array_splitting.cpp
> >> +++ b/src/compiler/glsl/opt_array_splitting.cpp
> >> @@ -140,6 +140,29 @@
> >> ir_array_reference_visitor::get_variable_entry(ir_variable *var)
> >> if (var->type->is_unsized_array())
> >>return NULL;
> >>
> >> + /* FIXME: arrays of arrays are not handled correctly by this pass
> >> so we
> >> +* skip it for now. While the pass will create functioning code it
> >> actually
> >> +* produces worse code.
> >> +*
> >> +* For example the array:
> >> +*
> >> +*int[3][2] a;
> >> +*
> >> +* ends up being split up into:
> >> +*
> >> +*int[3][2] a_0;
> >> +*int[3][2] a_1;
> >> +*int[3][2] a_2;
> >> +*
> >> +* And we end up referencing each of these new arrays for example:
> >> +*
> >> +*a[0][1] will be turned into a_0[0][1]
> >> +*a[1][0] will be turned into a_1[1][0]
> >> +*a[2][0] will be turned into a_2[2][0]
> >> +*/
> >> + if (var->type->is_array() && var->type->fields.array->is_array())
> >> + return NULL;
> >> +
> >> foreach_in_list(variable_entry, entry, &this->variable_list) {
> >>if (entry->var == var)
> >> return entry;
> >
> > ___
> > 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 v2 3/3] glsl: disable array splitting for AoA
I'd like the chance to look at these, please. I'm on vacation today and
tomorrow though.
On July 3, 2017 7:19:04 AM funfunc...@folklore1984.net wrote:
On 2017-07-03 08:47, Timothy Arceri wrote:
While it produces functioning code the pass creates worse code
for arrays of arrays. See the comment added in this patch for more
detail.
V2: skip splitting of AoA of matrices too.
Reviewed-by: Edward O'Callaghan
---
src/compiler/glsl/opt_array_splitting.cpp | 23 +++
1 file changed, 23 insertions(+)
diff --git a/src/compiler/glsl/opt_array_splitting.cpp
b/src/compiler/glsl/opt_array_splitting.cpp
index fb6d77b..d2e81665 100644
--- a/src/compiler/glsl/opt_array_splitting.cpp
+++ b/src/compiler/glsl/opt_array_splitting.cpp
@@ -140,6 +140,29 @@
ir_array_reference_visitor::get_variable_entry(ir_variable *var)
if (var->type->is_unsized_array())
return NULL;
+ /* FIXME: arrays of arrays are not handled correctly by this pass
so we
+* skip it for now. While the pass will create functioning code it
actually
+* produces worse code.
+*
+* For example the array:
+*
+*int[3][2] a;
+*
+* ends up being split up into:
+*
+*int[3][2] a_0;
+*int[3][2] a_1;
+*int[3][2] a_2;
+*
+* And we end up referencing each of these new arrays for example:
+*
+*a[0][1] will be turned into a_0[0][1]
+*a[1][0] will be turned into a_1[1][0]
+*a[2][0] will be turned into a_2[2][0]
+*/
+ if (var->type->is_array() && var->type->fields.array->is_array())
+ return NULL;
+
foreach_in_list(variable_entry, entry, &this->variable_list) {
if (entry->var == var)
return entry;
___
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 v2 3/3] glsl: disable array splitting for AoA
On 2017-07-03 08:47, Timothy Arceri wrote:
While it produces functioning code the pass creates worse code
for arrays of arrays. See the comment added in this patch for more
detail.
V2: skip splitting of AoA of matrices too.
Reviewed-by: Edward O'Callaghan
---
src/compiler/glsl/opt_array_splitting.cpp | 23 +++
1 file changed, 23 insertions(+)
diff --git a/src/compiler/glsl/opt_array_splitting.cpp
b/src/compiler/glsl/opt_array_splitting.cpp
index fb6d77b..d2e81665 100644
--- a/src/compiler/glsl/opt_array_splitting.cpp
+++ b/src/compiler/glsl/opt_array_splitting.cpp
@@ -140,6 +140,29 @@
ir_array_reference_visitor::get_variable_entry(ir_variable *var)
if (var->type->is_unsized_array())
return NULL;
+ /* FIXME: arrays of arrays are not handled correctly by this pass
so we
+* skip it for now. While the pass will create functioning code it
actually
+* produces worse code.
+*
+* For example the array:
+*
+*int[3][2] a;
+*
+* ends up being split up into:
+*
+*int[3][2] a_0;
+*int[3][2] a_1;
+*int[3][2] a_2;
+*
+* And we end up referencing each of these new arrays for example:
+*
+*a[0][1] will be turned into a_0[0][1]
+*a[1][0] will be turned into a_1[1][0]
+*a[2][0] will be turned into a_2[2][0]
+*/
+ if (var->type->is_array() && var->type->fields.array->is_array())
+ return NULL;
+
foreach_in_list(variable_entry, entry, &this->variable_list) {
if (entry->var == var)
return entry;
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
