Re: [Mesa-dev] [PATCH 2/4] glsl: Replace constant-index vector array accesses with swizzles

2013-03-29 Thread Paul Berry
On 27 March 2013 09:30, Ian Romanick  wrote:

> From: Ian Romanick 
>
> Search and replace:
>
> ][0] -> ].x
> ][1] -> ].y
> ][2] -> ].z
> ][3] -> ].w
>
> Fixes piglit tests inverse-mat[234].{vert,frag}.  These tests call the
> inverse function with constant parameters and expect proper constant
> folding to happen.  My suspicion is that this patch papers over some bug
> in constant propagation involving array accesses.
>
> Either way, all of these accesses eventually get lowered to swizzles.
> This cuts out the middle man (saving a trivial amount of CPU).
>
> NOTE: This is a candidate for the 9.1 branch.
>

I've tracked down the constant propagation bug that this patch papers over,
and I just sent out a bug fix ("glsl: Fix array indexing when constant
folding built-in functions.")

Assuming my bug fix is correct, can we NAK this patch?  I think it makes
the source code harder to read, and as you point out the benefit is just to
save a trivial amount of CPU.


>
> Signed-off-by: Ian Romanick 
> Cc: Eric Anholt 
> Cc: Paul Berry 
> ---
>  src/glsl/builtins/glsl/determinant.glsl |  62 +-
>  src/glsl/builtins/glsl/inverse.glsl | 112
> 
>  2 files changed, 87 insertions(+), 87 deletions(-)
>
> diff --git a/src/glsl/builtins/glsl/determinant.glsl
> b/src/glsl/builtins/glsl/determinant.glsl
> index 32695a8..78751a6 100644
> --- a/src/glsl/builtins/glsl/determinant.glsl
> +++ b/src/glsl/builtins/glsl/determinant.glsl
> @@ -24,47 +24,47 @@
>  #version 120
>  float determinant(mat2 m)
>  {
> -   return m[0][0] * m[1][1] - m[1][0] * m[0][1];
> +   return m[0].x * m[1].y - m[1].x * m[0].y;
>  }
>
>  float determinant(mat3 m)
>  {
> -   return (+ m[0][0] * (m[1][1] * m[2][2] - m[1][2] * m[2][1])
> -   - m[0][1] * (m[1][0] * m[2][2] - m[1][2] * m[2][0])
> -   + m[0][2] * (m[1][0] * m[2][1] - m[1][1] * m[2][0]));
> +   return (+ m[0].x * (m[1].y * m[2].z - m[1].z * m[2].y)
> +   - m[0].y * (m[1].x * m[2].z - m[1].z * m[2].x)
> +   + m[0].z * (m[1].x * m[2].y - m[1].y * m[2].x));
>  }
>
>  float determinant(mat4 m)
>  {
> -   float SubFactor00 = m[2][2] * m[3][3] - m[3][2] * m[2][3];
> -   float SubFactor01 = m[2][1] * m[3][3] - m[3][1] * m[2][3];
> -   float SubFactor02 = m[2][1] * m[3][2] - m[3][1] * m[2][2];
> -   float SubFactor03 = m[2][0] * m[3][3] - m[3][0] * m[2][3];
> -   float SubFactor04 = m[2][0] * m[3][2] - m[3][0] * m[2][2];
> -   float SubFactor05 = m[2][0] * m[3][1] - m[3][0] * m[2][1];
> -   float SubFactor06 = m[1][2] * m[3][3] - m[3][2] * m[1][3];
> -   float SubFactor07 = m[1][1] * m[3][3] - m[3][1] * m[1][3];
> -   float SubFactor08 = m[1][1] * m[3][2] - m[3][1] * m[1][2];
> -   float SubFactor09 = m[1][0] * m[3][3] - m[3][0] * m[1][3];
> -   float SubFactor10 = m[1][0] * m[3][2] - m[3][0] * m[1][2];
> -   float SubFactor11 = m[1][1] * m[3][3] - m[3][1] * m[1][3];
> -   float SubFactor12 = m[1][0] * m[3][1] - m[3][0] * m[1][1];
> -   float SubFactor13 = m[1][2] * m[2][3] - m[2][2] * m[1][3];
> -   float SubFactor14 = m[1][1] * m[2][3] - m[2][1] * m[1][3];
> -   float SubFactor15 = m[1][1] * m[2][2] - m[2][1] * m[1][2];
> -   float SubFactor16 = m[1][0] * m[2][3] - m[2][0] * m[1][3];
> -   float SubFactor17 = m[1][0] * m[2][2] - m[2][0] * m[1][2];
> -   float SubFactor18 = m[1][0] * m[2][1] - m[2][0] * m[1][1];
> +   float SubFactor00 = m[2].z * m[3].w - m[3].z * m[2].w;
> +   float SubFactor01 = m[2].y * m[3].w - m[3].y * m[2].w;
> +   float SubFactor02 = m[2].y * m[3].z - m[3].y * m[2].z;
> +   float SubFactor03 = m[2].x * m[3].w - m[3].x * m[2].w;
> +   float SubFactor04 = m[2].x * m[3].z - m[3].x * m[2].z;
> +   float SubFactor05 = m[2].x * m[3].y - m[3].x * m[2].y;
> +   float SubFactor06 = m[1].z * m[3].w - m[3].z * m[1].w;
> +   float SubFactor07 = m[1].y * m[3].w - m[3].y * m[1].w;
> +   float SubFactor08 = m[1].y * m[3].z - m[3].y * m[1].z;
> +   float SubFactor09 = m[1].x * m[3].w - m[3].x * m[1].w;
> +   float SubFactor10 = m[1].x * m[3].z - m[3].x * m[1].z;
> +   float SubFactor11 = m[1].y * m[3].w - m[3].y * m[1].w;
> +   float SubFactor12 = m[1].x * m[3].y - m[3].x * m[1].y;
> +   float SubFactor13 = m[1].z * m[2].w - m[2].z * m[1].w;
> +   float SubFactor14 = m[1].y * m[2].w - m[2].y * m[1].w;
> +   float SubFactor15 = m[1].y * m[2].z - m[2].y * m[1].z;
> +   float SubFactor16 = m[1].x * m[2].w - m[2].x * m[1].w;
> +   float SubFactor17 = m[1].x * m[2].z - m[2].x * m[1].z;
> +   float SubFactor18 = m[1].x * m[2].y - m[2].x * m[1].y;
>
> vec4 adj_0;
>
> -   adj_0[0] = + (m[1][1] * SubFactor00 - m[1][2] * SubFactor01 + m[1][3]
> * SubFactor02);
> -   adj_0[1] = - (m[1][0] * SubFactor00 - m[1][2] * SubFactor03 + m[1][3]
> * SubFactor04);
> -   adj_0[2] = + (m[1][0] * SubFactor01 - m[1][1] * SubFactor03 + m[1][3]
> * SubFactor05);
> -   adj_0[3] = - (m[1][0] * SubFactor02 - m[1][1] * SubFactor04 + m[1][2]
> * SubFactor05);
> +   adj_0.x = + (m[1].y * SubFactor00 - m[1].z * SubFactor01 + m[

[Mesa-dev] [PATCH 2/4] glsl: Replace constant-index vector array accesses with swizzles

2013-03-27 Thread Ian Romanick
From: Ian Romanick 

Search and replace:

][0] -> ].x
][1] -> ].y
][2] -> ].z
][3] -> ].w

Fixes piglit tests inverse-mat[234].{vert,frag}.  These tests call the
inverse function with constant parameters and expect proper constant
folding to happen.  My suspicion is that this patch papers over some bug
in constant propagation involving array accesses.

Either way, all of these accesses eventually get lowered to swizzles.
This cuts out the middle man (saving a trivial amount of CPU).

NOTE: This is a candidate for the 9.1 branch.

Signed-off-by: Ian Romanick 
Cc: Eric Anholt 
Cc: Paul Berry 
---
 src/glsl/builtins/glsl/determinant.glsl |  62 +-
 src/glsl/builtins/glsl/inverse.glsl | 112 
 2 files changed, 87 insertions(+), 87 deletions(-)

diff --git a/src/glsl/builtins/glsl/determinant.glsl 
b/src/glsl/builtins/glsl/determinant.glsl
index 32695a8..78751a6 100644
--- a/src/glsl/builtins/glsl/determinant.glsl
+++ b/src/glsl/builtins/glsl/determinant.glsl
@@ -24,47 +24,47 @@
 #version 120
 float determinant(mat2 m)
 {
-   return m[0][0] * m[1][1] - m[1][0] * m[0][1];
+   return m[0].x * m[1].y - m[1].x * m[0].y;
 }
 
 float determinant(mat3 m)
 {
-   return (+ m[0][0] * (m[1][1] * m[2][2] - m[1][2] * m[2][1])
-   - m[0][1] * (m[1][0] * m[2][2] - m[1][2] * m[2][0])
-   + m[0][2] * (m[1][0] * m[2][1] - m[1][1] * m[2][0]));
+   return (+ m[0].x * (m[1].y * m[2].z - m[1].z * m[2].y)
+   - m[0].y * (m[1].x * m[2].z - m[1].z * m[2].x)
+   + m[0].z * (m[1].x * m[2].y - m[1].y * m[2].x));
 }
 
 float determinant(mat4 m)
 {
-   float SubFactor00 = m[2][2] * m[3][3] - m[3][2] * m[2][3];
-   float SubFactor01 = m[2][1] * m[3][3] - m[3][1] * m[2][3];
-   float SubFactor02 = m[2][1] * m[3][2] - m[3][1] * m[2][2];
-   float SubFactor03 = m[2][0] * m[3][3] - m[3][0] * m[2][3];
-   float SubFactor04 = m[2][0] * m[3][2] - m[3][0] * m[2][2];
-   float SubFactor05 = m[2][0] * m[3][1] - m[3][0] * m[2][1];
-   float SubFactor06 = m[1][2] * m[3][3] - m[3][2] * m[1][3];
-   float SubFactor07 = m[1][1] * m[3][3] - m[3][1] * m[1][3];
-   float SubFactor08 = m[1][1] * m[3][2] - m[3][1] * m[1][2];
-   float SubFactor09 = m[1][0] * m[3][3] - m[3][0] * m[1][3];
-   float SubFactor10 = m[1][0] * m[3][2] - m[3][0] * m[1][2];
-   float SubFactor11 = m[1][1] * m[3][3] - m[3][1] * m[1][3];
-   float SubFactor12 = m[1][0] * m[3][1] - m[3][0] * m[1][1];
-   float SubFactor13 = m[1][2] * m[2][3] - m[2][2] * m[1][3];
-   float SubFactor14 = m[1][1] * m[2][3] - m[2][1] * m[1][3];
-   float SubFactor15 = m[1][1] * m[2][2] - m[2][1] * m[1][2];
-   float SubFactor16 = m[1][0] * m[2][3] - m[2][0] * m[1][3];
-   float SubFactor17 = m[1][0] * m[2][2] - m[2][0] * m[1][2];
-   float SubFactor18 = m[1][0] * m[2][1] - m[2][0] * m[1][1];
+   float SubFactor00 = m[2].z * m[3].w - m[3].z * m[2].w;
+   float SubFactor01 = m[2].y * m[3].w - m[3].y * m[2].w;
+   float SubFactor02 = m[2].y * m[3].z - m[3].y * m[2].z;
+   float SubFactor03 = m[2].x * m[3].w - m[3].x * m[2].w;
+   float SubFactor04 = m[2].x * m[3].z - m[3].x * m[2].z;
+   float SubFactor05 = m[2].x * m[3].y - m[3].x * m[2].y;
+   float SubFactor06 = m[1].z * m[3].w - m[3].z * m[1].w;
+   float SubFactor07 = m[1].y * m[3].w - m[3].y * m[1].w;
+   float SubFactor08 = m[1].y * m[3].z - m[3].y * m[1].z;
+   float SubFactor09 = m[1].x * m[3].w - m[3].x * m[1].w;
+   float SubFactor10 = m[1].x * m[3].z - m[3].x * m[1].z;
+   float SubFactor11 = m[1].y * m[3].w - m[3].y * m[1].w;
+   float SubFactor12 = m[1].x * m[3].y - m[3].x * m[1].y;
+   float SubFactor13 = m[1].z * m[2].w - m[2].z * m[1].w;
+   float SubFactor14 = m[1].y * m[2].w - m[2].y * m[1].w;
+   float SubFactor15 = m[1].y * m[2].z - m[2].y * m[1].z;
+   float SubFactor16 = m[1].x * m[2].w - m[2].x * m[1].w;
+   float SubFactor17 = m[1].x * m[2].z - m[2].x * m[1].z;
+   float SubFactor18 = m[1].x * m[2].y - m[2].x * m[1].y;
 
vec4 adj_0;
 
-   adj_0[0] = + (m[1][1] * SubFactor00 - m[1][2] * SubFactor01 + m[1][3] * 
SubFactor02);
-   adj_0[1] = - (m[1][0] * SubFactor00 - m[1][2] * SubFactor03 + m[1][3] * 
SubFactor04);
-   adj_0[2] = + (m[1][0] * SubFactor01 - m[1][1] * SubFactor03 + m[1][3] * 
SubFactor05);
-   adj_0[3] = - (m[1][0] * SubFactor02 - m[1][1] * SubFactor04 + m[1][2] * 
SubFactor05);
+   adj_0.x = + (m[1].y * SubFactor00 - m[1].z * SubFactor01 + m[1].w * 
SubFactor02);
+   adj_0.y = - (m[1].x * SubFactor00 - m[1].z * SubFactor03 + m[1].w * 
SubFactor04);
+   adj_0.z = + (m[1].x * SubFactor01 - m[1].y * SubFactor03 + m[1].w * 
SubFactor05);
+   adj_0.w = - (m[1].x * SubFactor02 - m[1].y * SubFactor04 + m[1].z * 
SubFactor05);
 
-   return (+ m[0][0] * adj_0[0]
-   + m[0][1] * adj_0[1]
-   + m[0][2] * adj_0[2]
-   + m[0][3] * adj_0[3]);
+   return (+ m[0].x * adj_0.x
+   + m[0].y * adj_0.y
+   + m[0].z * adj_0.z
+   + m[0].w * adj_0.w);
 }
diff --git a/src/glsl/builtins/glsl/inverse.gl