Re: [Mesa-dev] [PATCH] linker: Reject shaders that use too many varyings

2011-06-27 Thread Ian Romanick
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 06/23/2011 07:56 AM, Alex Deucher wrote:
 On Thu, Jun 23, 2011 at 10:38 AM, Roland Scheidegger srol...@vmware.com 
 wrote:
 Am 23.06.2011 16:09, schrieb Jerome Glisse:
 On Wed, Jun 22, 2011 at 10:49 PM, Alex Deucher alexdeuc...@gmail.com 
 wrote:
 On Wed, Jun 22, 2011 at 10:12 PM, Roland Scheidegger srol...@vmware.com 
 wrote:
 Am 21.06.2011 20:59, schrieb Sven Arvidsson:
 This change broke a whole lot of stuff on r600g, for example Unigine
 Heaven:

   shader uses too many varying components (36  32)

 It looks like the r600g driver claims to only support 10 varyings, which
 the state tracker reduces to 8 (as it subtracts the supposedly included
 color varyings).
 At first sight I can't quite see why it's limited to 10, all r600 chips
 should be able to handle 32 (dx10 requirement) but of course the driver
 might not (mesa itself is limited to 16 it seems). If it worked just
 fine before that suggests it indeed works just fine with more...
 Someone more familiar with the driver should be able to tell if it's
 safe to increase the limit to 32 (the state tracker will cap it to 16).

 The hardware definitely supports 32.  I'm not sure why it's currently
 set to 10; I don't see any limitations in the code off hand.

 Alex

 IIRC it's just cut  paste from r300g it can be safely bump

 Ok Marek bumped it to 34. That seems to be lying too I don't think it
 could handle 32 generic inputs and 2 colors. But there's no way to
 really express that right now.
 
 In addition to the routed inputs, you can also enable a bunch of hw
 generated inputs, but I think both generated and routed inputs
 combined are limited to 32 input slots.

Was there any resolution to this issue?  I'd like to cherry pick the
linker patch to the 7.10 branch, but I don't want to regress r600g.
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAk4JAF8ACgkQX1gOwKyEAw/uZgCgmfWOXlfVQqW29DEajJzILWNm
5wsAniwJv88TkY4IjtUGt6tQu9/SRK6C
=y29u
-END PGP SIGNATURE-
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] linker: Reject shaders that use too many varyings

2011-06-27 Thread Roland Scheidegger
Am 28.06.2011 00:12, schrieb Ian Romanick:
 On 06/23/2011 07:56 AM, Alex Deucher wrote:
 On Thu, Jun 23, 2011 at 10:38 AM, Roland Scheidegger srol...@vmware.com 
 wrote:
 Am 23.06.2011 16:09, schrieb Jerome Glisse:
 On Wed, Jun 22, 2011 at 10:49 PM, Alex Deucher alexdeuc...@gmail.com 
 wrote:
 On Wed, Jun 22, 2011 at 10:12 PM, Roland Scheidegger srol...@vmware.com 
 wrote:
 Am 21.06.2011 20:59, schrieb Sven Arvidsson:
 This change broke a whole lot of stuff on r600g, for example Unigine
 Heaven:

   shader uses too many varying components (36  32)

 It looks like the r600g driver claims to only support 10 varyings, which
 the state tracker reduces to 8 (as it subtracts the supposedly included
 color varyings).
 At first sight I can't quite see why it's limited to 10, all r600 chips
 should be able to handle 32 (dx10 requirement) but of course the driver
 might not (mesa itself is limited to 16 it seems). If it worked just
 fine before that suggests it indeed works just fine with more...
 Someone more familiar with the driver should be able to tell if it's
 safe to increase the limit to 32 (the state tracker will cap it to 16).

 The hardware definitely supports 32.  I'm not sure why it's currently
 set to 10; I don't see any limitations in the code off hand.

 Alex

 IIRC it's just cut  paste from r300g it can be safely bump

 Ok Marek bumped it to 34. That seems to be lying too I don't think it
 could handle 32 generic inputs and 2 colors. But there's no way to
 really express that right now.
 
 In addition to the routed inputs, you can also enable a bunch of hw
 generated inputs, but I think both generated and routed inputs
 combined are limited to 32 input slots.
 
 Was there any resolution to this issue?  I'd like to cherry pick the
 linker patch to the 7.10 branch, but I don't want to regress r600g.

Yes, 1e5cef96d184b00eb588b48ecd02386998077d82 bumped the limits. It
wasn't pushed to 7.10 though I believe it should be safe to do so.

Roland
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] linker: Reject shaders that use too many varyings

2011-06-23 Thread Jerome Glisse
On Wed, Jun 22, 2011 at 10:49 PM, Alex Deucher alexdeuc...@gmail.com wrote:
 On Wed, Jun 22, 2011 at 10:12 PM, Roland Scheidegger srol...@vmware.com 
 wrote:
 Am 21.06.2011 20:59, schrieb Sven Arvidsson:
 This change broke a whole lot of stuff on r600g, for example Unigine
 Heaven:

       shader uses too many varying components (36  32)

 It looks like the r600g driver claims to only support 10 varyings, which
 the state tracker reduces to 8 (as it subtracts the supposedly included
 color varyings).
 At first sight I can't quite see why it's limited to 10, all r600 chips
 should be able to handle 32 (dx10 requirement) but of course the driver
 might not (mesa itself is limited to 16 it seems). If it worked just
 fine before that suggests it indeed works just fine with more...
 Someone more familiar with the driver should be able to tell if it's
 safe to increase the limit to 32 (the state tracker will cap it to 16).

 The hardware definitely supports 32.  I'm not sure why it's currently
 set to 10; I don't see any limitations in the code off hand.

 Alex

IIRC it's just cut  paste from r300g it can be safely bump

Cheers,
Jerome
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] linker: Reject shaders that use too many varyings

2011-06-23 Thread Roland Scheidegger
Am 23.06.2011 16:09, schrieb Jerome Glisse:
 On Wed, Jun 22, 2011 at 10:49 PM, Alex Deucher alexdeuc...@gmail.com wrote:
 On Wed, Jun 22, 2011 at 10:12 PM, Roland Scheidegger srol...@vmware.com 
 wrote:
 Am 21.06.2011 20:59, schrieb Sven Arvidsson:
 This change broke a whole lot of stuff on r600g, for example Unigine
 Heaven:

   shader uses too many varying components (36  32)

 It looks like the r600g driver claims to only support 10 varyings, which
 the state tracker reduces to 8 (as it subtracts the supposedly included
 color varyings).
 At first sight I can't quite see why it's limited to 10, all r600 chips
 should be able to handle 32 (dx10 requirement) but of course the driver
 might not (mesa itself is limited to 16 it seems). If it worked just
 fine before that suggests it indeed works just fine with more...
 Someone more familiar with the driver should be able to tell if it's
 safe to increase the limit to 32 (the state tracker will cap it to 16).

 The hardware definitely supports 32.  I'm not sure why it's currently
 set to 10; I don't see any limitations in the code off hand.

 Alex
 
 IIRC it's just cut  paste from r300g it can be safely bump

Ok Marek bumped it to 34. That seems to be lying too I don't think it
could handle 32 generic inputs and 2 colors. But there's no way to
really express that right now.

Roland
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] linker: Reject shaders that use too many varyings

2011-06-23 Thread Jerome Glisse
On Thu, Jun 23, 2011 at 10:38 AM, Roland Scheidegger srol...@vmware.com wrote:
 Am 23.06.2011 16:09, schrieb Jerome Glisse:
 On Wed, Jun 22, 2011 at 10:49 PM, Alex Deucher alexdeuc...@gmail.com wrote:
 On Wed, Jun 22, 2011 at 10:12 PM, Roland Scheidegger srol...@vmware.com 
 wrote:
 Am 21.06.2011 20:59, schrieb Sven Arvidsson:
 This change broke a whole lot of stuff on r600g, for example Unigine
 Heaven:

       shader uses too many varying components (36  32)

 It looks like the r600g driver claims to only support 10 varyings, which
 the state tracker reduces to 8 (as it subtracts the supposedly included
 color varyings).
 At first sight I can't quite see why it's limited to 10, all r600 chips
 should be able to handle 32 (dx10 requirement) but of course the driver
 might not (mesa itself is limited to 16 it seems). If it worked just
 fine before that suggests it indeed works just fine with more...
 Someone more familiar with the driver should be able to tell if it's
 safe to increase the limit to 32 (the state tracker will cap it to 16).

 The hardware definitely supports 32.  I'm not sure why it's currently
 set to 10; I don't see any limitations in the code off hand.

 Alex

 IIRC it's just cut  paste from r300g it can be safely bump

 Ok Marek bumped it to 34. That seems to be lying too I don't think it
 could handle 32 generic inputs and 2 colors. But there's no way to
 really express that right now.

 Roland


Also iirc r6xx/r7xx needs special code for handling varying over 16,
can't remember if we had proper code for that.

Cheers,
Jerome
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] linker: Reject shaders that use too many varyings

2011-06-23 Thread Alex Deucher
On Thu, Jun 23, 2011 at 10:38 AM, Roland Scheidegger srol...@vmware.com wrote:
 Am 23.06.2011 16:09, schrieb Jerome Glisse:
 On Wed, Jun 22, 2011 at 10:49 PM, Alex Deucher alexdeuc...@gmail.com wrote:
 On Wed, Jun 22, 2011 at 10:12 PM, Roland Scheidegger srol...@vmware.com 
 wrote:
 Am 21.06.2011 20:59, schrieb Sven Arvidsson:
 This change broke a whole lot of stuff on r600g, for example Unigine
 Heaven:

       shader uses too many varying components (36  32)

 It looks like the r600g driver claims to only support 10 varyings, which
 the state tracker reduces to 8 (as it subtracts the supposedly included
 color varyings).
 At first sight I can't quite see why it's limited to 10, all r600 chips
 should be able to handle 32 (dx10 requirement) but of course the driver
 might not (mesa itself is limited to 16 it seems). If it worked just
 fine before that suggests it indeed works just fine with more...
 Someone more familiar with the driver should be able to tell if it's
 safe to increase the limit to 32 (the state tracker will cap it to 16).

 The hardware definitely supports 32.  I'm not sure why it's currently
 set to 10; I don't see any limitations in the code off hand.

 Alex

 IIRC it's just cut  paste from r300g it can be safely bump

 Ok Marek bumped it to 34. That seems to be lying too I don't think it
 could handle 32 generic inputs and 2 colors. But there's no way to
 really express that right now.

In addition to the routed inputs, you can also enable a bunch of hw
generated inputs, but I think both generated and routed inputs
combined are limited to 32 input slots.

Alex


 Roland

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] linker: Reject shaders that use too many varyings

2011-06-23 Thread Christoph Bumiller
On 23.06.2011 04:12, Roland Scheidegger wrote:
 Am 21.06.2011 20:59, schrieb Sven Arvidsson:
 This change broke a whole lot of stuff on r600g, for example Unigine
 Heaven:

  shader uses too many varying components (36  32)
 It looks like the r600g driver claims to only support 10 varyings, which
 the state tracker reduces to 8 (as it subtracts the supposedly included
 color varyings).
 At first sight I can't quite see why it's limited to 10, all r600 chips
 should be able to handle 32 (dx10 requirement) but of course the driver
Just a side note, the requirement of 32 outputs (and thus FP inputs)
only applies to geometry shaders, vertex shaders are allowed to be
limited to 16 (and that's the case on nv50).

 might not (mesa itself is limited to 16 it seems). If it worked just
 fine before that suggests it indeed works just fine with more...
 Someone more familiar with the driver should be able to tell if it's
 safe to increase the limit to 32 (the state tracker will cap it to 16).

 Roland
 ___
 mesa-dev mailing list
 mesa-dev@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/mesa-dev

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] linker: Reject shaders that use too many varyings

2011-06-22 Thread Alex Deucher
On Wed, Jun 22, 2011 at 10:12 PM, Roland Scheidegger srol...@vmware.com wrote:
 Am 21.06.2011 20:59, schrieb Sven Arvidsson:
 This change broke a whole lot of stuff on r600g, for example Unigine
 Heaven:

       shader uses too many varying components (36  32)

 It looks like the r600g driver claims to only support 10 varyings, which
 the state tracker reduces to 8 (as it subtracts the supposedly included
 color varyings).
 At first sight I can't quite see why it's limited to 10, all r600 chips
 should be able to handle 32 (dx10 requirement) but of course the driver
 might not (mesa itself is limited to 16 it seems). If it worked just
 fine before that suggests it indeed works just fine with more...
 Someone more familiar with the driver should be able to tell if it's
 safe to increase the limit to 32 (the state tracker will cap it to 16).

The hardware definitely supports 32.  I'm not sure why it's currently
set to 10; I don't see any limitations in the code off hand.

Alex


 Roland
 ___
 mesa-dev mailing list
 mesa-dev@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/mesa-dev

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH] linker: Reject shaders that use too many varyings

2011-06-13 Thread Ian Romanick
From: Ian Romanick ian.d.roman...@intel.com

Previously it was up to the driver or later code generator to reject
these shaders.  It turns out that nobody did this.

This will need changes to support geometry shaders.

NOTE: This is a candidate for the stable branches.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=37743
---
 src/glsl/linker.cpp |   55 ++
 1 files changed, 50 insertions(+), 5 deletions(-)

diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
index 255edc6..eaa6412 100644
--- a/src/glsl/linker.cpp
+++ b/src/glsl/linker.cpp
@@ -1405,8 +1405,9 @@ demote_shader_inputs_and_outputs(gl_shader *sh, enum 
ir_variable_mode mode)
 }
 
 
-void
-assign_varying_locations(struct gl_shader_program *prog,
+bool
+assign_varying_locations(struct gl_context *ctx,
+struct gl_shader_program *prog,
 gl_shader *producer, gl_shader *consumer)
 {
/* FINISHME: Set dynamically when geometry shader support is added. */
@@ -1462,6 +1463,9 @@ assign_varying_locations(struct gl_shader_program *prog,
   }
}
 
+   unsigned float_components = 0;
+   unsigned varying_vectors = 0;
+
foreach_list(node, consumer-ir) {
   ir_variable *const var = ((ir_instruction *) node)-as_variable();
 
@@ -1492,8 +1496,45 @@ assign_varying_locations(struct gl_shader_program *prog,
  * value is written by the previous stage.
  */
 var-mode = ir_var_auto;
+  } else {
+if (var-type-is_array()) {
+   if (var-type-is_matrix()) {
+  varying_vectors += var-type-array_size()
+ * var-type-matrix_columns;
+  float_components += var-type-array_size()
+ * var-type-matrix_columns * 4;
+   } else {
+  varying_vectors += var-type-array_size();
+  float_components += var-type-array_size() * 4;
+   }
+} else if (var-type-is_matrix()) {
+   varying_vectors += var-type-matrix_columns;
+   float_components += var-type-matrix_columns * 4;
+} else {
+   varying_vectors++;
+   float_components += var-type-vector_elements;
+}
+  }
+   }
+
+   printf(varyings = %u, %u\n, varying_vectors, float_components);
+   if (ctx-API == API_OPENGLES2 || prog-Version == 100) {
+  if (varying_vectors  ctx-Const.MaxVarying) {
+linker_error_printf(prog, shader uses too many varying vectors 
+(%u  %u)\n,
+varying_vectors, ctx-Const.MaxVarying);
+return false;
+  }
+   } else {
+  if (float_components  ctx-Const.MaxVarying * 4) {
+linker_error_printf(prog, shader uses too many varying components 
+(%u  %u)\n,
+float_components, ctx-Const.MaxVarying * 4);
+return false;
   }
}
+
+   return true;
 }
 
 
@@ -1666,9 +1707,13 @@ link_shaders(struct gl_context *ctx, struct 
gl_shader_program *prog)
   if (prog-_LinkedShaders[i] == NULL)
 continue;
 
-  assign_varying_locations(prog,
-  prog-_LinkedShaders[prev],
-  prog-_LinkedShaders[i]);
+  if (!assign_varying_locations(ctx, prog,
+   prog-_LinkedShaders[prev],
+   prog-_LinkedShaders[i])) {
+prog-LinkStatus = false;
+goto done;
+  }
+
   prev = i;
}
 
-- 
1.7.5.2

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] linker: Reject shaders that use too many varyings

2011-06-13 Thread Tilman Sauerbeck
Ian Romanick [2011-06-13 12:09]:

 [...]
 +  } else if (var-type-is_matrix()) {
 + varying_vectors += var-type-matrix_columns;
 + float_components += var-type-matrix_columns * 4;
 +  } else {
 + varying_vectors++;
 + float_components += var-type-vector_elements;
 +  }
 +  }
 +   }
 +
 +   printf(varyings = %u, %u\n, varying_vectors, float_components);

Did you intend to leave this printf there?

Regards,
Tilman

-- 
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing on usenet and in e-mail?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] linker: Reject shaders that use too many varyings

2011-06-13 Thread Eric Anholt
On Mon, 13 Jun 2011 12:09:04 -0700, Ian Romanick i...@freedesktop.org wrote:
 From: Ian Romanick ian.d.roman...@intel.com
 
 Previously it was up to the driver or later code generator to reject
 these shaders.  It turns out that nobody did this.
 
 This will need changes to support geometry shaders.
 
 NOTE: This is a candidate for the stable branches.
 
 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=37743

I find it odd that arrays/matrices are treated as increments of 4
components while plain vectors aren't.  Plain vectors end up occupying 4
components in our current implementation (you won't get more than 16
varying vectors working, since we run out of bits), and I think treating
it that way for limits would be more intelligible for users than leaving
it to the driver to catch later.


pgpwexdP0428a.pgp
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev