Re: [Mesa-dev] [PATCH 6/7] i915: Fail without crashing if a Mesa IR program uses too many registers

2011-08-02 Thread Kenneth Graunke 
On 08/01/2011 06:18 PM, Ian Romanick wrote:
 On 08/01/2011 11:39 AM, Eric Anholt wrote:
 On Mon,  1 Aug 2011 10:20:22 -0700, Ian Romanick i...@freedesktop.org 
 wrote:
 From: Ian Romanick ian.d.roman...@intel.com

 So far this can only happen in GLSL shaders that contain flow-control
 that could not be lowered.  These programs would have failed to run on
 hardware anyway.
 
 This looks reasonable, but I don't understand why this can only happen
 with flow control.  Couldn't we just have something with too many temps
 involved without flow control?
 
 I suppose the more correct comment would be I've only seen this occur
 in GLSL shaders that contain flow-control that could not be lowered.  I
 don't think it can happen outside GLSL because the assembly extensions
 can reject (at the core Mesa level) programs that use too many temps.
 
 How does this sound:
 
 This can only happen in GLSL shaders because assembly shaders that use
 too many temps are rejected by core Mesa.  It is easiest to make this
 happen with shaders that contain flow-control that could not be lowered.

This makes more sense.  Looks good.

Reviewed-by: Kenneth Graunke kenn...@whitecape.org

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

[Mesa-dev] [PATCH 6/7] i915: Fail without crashing if a Mesa IR program uses too many registers

2011-08-01 Thread Ian Romanick 
From: Ian Romanick ian.d.roman...@intel.com

So far this can only happen in GLSL shaders that contain flow-control
that could not be lowered.  These programs would have failed to run on
hardware anyway.
---
 src/mesa/drivers/dri/i915/i915_fragprog.c |   15 +--
 1 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/src/mesa/drivers/dri/i915/i915_fragprog.c 
b/src/mesa/drivers/dri/i915/i915_fragprog.c
index 6e1d709..32050ce 100644
--- a/src/mesa/drivers/dri/i915/i915_fragprog.c
+++ b/src/mesa/drivers/dri/i915/i915_fragprog.c
@@ -303,7 +303,7 @@ do {
\
 /* 
  * TODO: consider moving this into core 
  */
-static void calc_live_regs( struct i915_fragment_program *p )
+static bool calc_live_regs( struct i915_fragment_program *p )
 {
 const struct gl_fragment_program *program = p-FragProg;
 GLuint regsUsed = 0x;
@@ -317,6 +317,9 @@ static void calc_live_regs( struct i915_fragment_program *p 
)
 
 /* Register is written to: unmark as live for this and preceeding ops 
*/ 
 if (inst-DstReg.File == PROGRAM_TEMPORARY) {
+   if (inst-DstReg.Index  16)
+  return false;
+
 live_components[inst-DstReg.Index] = ~inst-DstReg.WriteMask;
 if (live_components[inst-DstReg.Index] == 0)
 regsUsed = ~(1  inst-DstReg.Index);
@@ -327,6 +330,9 @@ static void calc_live_regs( struct i915_fragment_program *p 
)
 if (inst-SrcReg[a].File == PROGRAM_TEMPORARY) {
 unsigned c;
 
+   if (inst-SrcReg[a].Index  16)
+  return false;
+
 regsUsed |= 1  inst-SrcReg[a].Index;
 
 for (c = 0; c  4; c++) {
@@ -340,6 +346,8 @@ static void calc_live_regs( struct i915_fragment_program *p 
)
 
 p-usedRegs[i] = regsUsed;
 }
+
+return true;
 }
 
 static GLuint get_live_regs( struct i915_fragment_program *p, 
@@ -394,7 +402,10 @@ upload_program(struct i915_fragment_program *p)
 
/* Not always needed:
 */
-   calc_live_regs(p);
+   if (!calc_live_regs(p)) {
+  i915_program_error(p, Could not allocate registers);
+  return;
+   }
 
while (1) {
   GLuint src0, src1, src2, flags;
-- 
1.7.4.4

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

Re: [Mesa-dev] [PATCH 6/7] i915: Fail without crashing if a Mesa IR program uses too many registers

2011-08-01 Thread Eric Anholt 
On Mon,  1 Aug 2011 10:20:22 -0700, Ian Romanick i...@freedesktop.org wrote:
 From: Ian Romanick ian.d.roman...@intel.com
 
 So far this can only happen in GLSL shaders that contain flow-control
 that could not be lowered.  These programs would have failed to run on
 hardware anyway.

This looks reasonable, but I don't understand why this can only happen
with flow control.  Couldn't we just have something with too many temps
involved without flow control?

 ---
  src/mesa/drivers/dri/i915/i915_fragprog.c |   15 +--
  1 files changed, 13 insertions(+), 2 deletions(-)
 
 diff --git a/src/mesa/drivers/dri/i915/i915_fragprog.c 
 b/src/mesa/drivers/dri/i915/i915_fragprog.c
 index 6e1d709..32050ce 100644
 --- a/src/mesa/drivers/dri/i915/i915_fragprog.c
 +++ b/src/mesa/drivers/dri/i915/i915_fragprog.c
 @@ -303,7 +303,7 @@ do {  
 \
  /* 
   * TODO: consider moving this into core 
   */
 -static void calc_live_regs( struct i915_fragment_program *p )
 +static bool calc_live_regs( struct i915_fragment_program *p )
  {
  const struct gl_fragment_program *program = p-FragProg;
  GLuint regsUsed = 0x;
 @@ -317,6 +317,9 @@ static void calc_live_regs( struct i915_fragment_program 
 *p )
  
  /* Register is written to: unmark as live for this and preceeding 
 ops */ 
  if (inst-DstReg.File == PROGRAM_TEMPORARY) {
 + if (inst-DstReg.Index  16)
 +return false;
 +
  live_components[inst-DstReg.Index] = ~inst-DstReg.WriteMask;
  if (live_components[inst-DstReg.Index] == 0)
  regsUsed = ~(1  inst-DstReg.Index);
 @@ -327,6 +330,9 @@ static void calc_live_regs( struct i915_fragment_program 
 *p )
  if (inst-SrcReg[a].File == PROGRAM_TEMPORARY) {
  unsigned c;
  
 + if (inst-SrcReg[a].Index  16)
 +return false;
 +
  regsUsed |= 1  inst-SrcReg[a].Index;
  
  for (c = 0; c  4; c++) {
 @@ -340,6 +346,8 @@ static void calc_live_regs( struct i915_fragment_program 
 *p )
  
  p-usedRegs[i] = regsUsed;
  }
 +
 +return true;
  }
  
  static GLuint get_live_regs( struct i915_fragment_program *p, 
 @@ -394,7 +402,10 @@ upload_program(struct i915_fragment_program *p)
  
 /* Not always needed:
  */
 -   calc_live_regs(p);
 +   if (!calc_live_regs(p)) {
 +  i915_program_error(p, Could not allocate registers);
 +  return;
 +   }
  
 while (1) {
GLuint src0, src1, src2, flags;
 -- 
 1.7.4.4
 
 ___
 mesa-dev mailing list
 mesa-dev@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

Re: [Mesa-dev] [PATCH 6/7] i915: Fail without crashing if a Mesa IR program uses too many registers

2011-08-01 Thread Ian Romanick 
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 08/01/2011 11:39 AM, Eric Anholt wrote:
 On Mon,  1 Aug 2011 10:20:22 -0700, Ian Romanick i...@freedesktop.org 
 wrote:
 From: Ian Romanick ian.d.roman...@intel.com

 So far this can only happen in GLSL shaders that contain flow-control
 that could not be lowered.  These programs would have failed to run on
 hardware anyway.
 
 This looks reasonable, but I don't understand why this can only happen
 with flow control.  Couldn't we just have something with too many temps
 involved without flow control?

I suppose the more correct comment would be I've only seen this occur
in GLSL shaders that contain flow-control that could not be lowered.  I
don't think it can happen outside GLSL because the assembly extensions
can reject (at the core Mesa level) programs that use too many temps.

How does this sound:

This can only happen in GLSL shaders because assembly shaders that use
too many temps are rejected by core Mesa.  It is easiest to make this
happen with shaders that contain flow-control that could not be lowered.

 ---
  src/mesa/drivers/dri/i915/i915_fragprog.c |   15 +--
  1 files changed, 13 insertions(+), 2 deletions(-)

 diff --git a/src/mesa/drivers/dri/i915/i915_fragprog.c 
 b/src/mesa/drivers/dri/i915/i915_fragprog.c
 index 6e1d709..32050ce 100644
 --- a/src/mesa/drivers/dri/i915/i915_fragprog.c
 +++ b/src/mesa/drivers/dri/i915/i915_fragprog.c
 @@ -303,7 +303,7 @@ do { 
 \
  /* 
   * TODO: consider moving this into core 
   */
 -static void calc_live_regs( struct i915_fragment_program *p )
 +static bool calc_live_regs( struct i915_fragment_program *p )
  {
  const struct gl_fragment_program *program = p-FragProg;
  GLuint regsUsed = 0x;
 @@ -317,6 +317,9 @@ static void calc_live_regs( struct i915_fragment_program 
 *p )
  
  /* Register is written to: unmark as live for this and preceeding 
 ops */ 
  if (inst-DstReg.File == PROGRAM_TEMPORARY) {
 +if (inst-DstReg.Index  16)
 +   return false;
 +
  live_components[inst-DstReg.Index] = ~inst-DstReg.WriteMask;
  if (live_components[inst-DstReg.Index] == 0)
  regsUsed = ~(1  inst-DstReg.Index);
 @@ -327,6 +330,9 @@ static void calc_live_regs( struct i915_fragment_program 
 *p )
  if (inst-SrcReg[a].File == PROGRAM_TEMPORARY) {
  unsigned c;
  
 +if (inst-SrcReg[a].Index  16)
 +   return false;
 +
  regsUsed |= 1  inst-SrcReg[a].Index;
  
  for (c = 0; c  4; c++) {
 @@ -340,6 +346,8 @@ static void calc_live_regs( struct i915_fragment_program 
 *p )
  
  p-usedRegs[i] = regsUsed;
  }
 +
 +return true;
  }
  
  static GLuint get_live_regs( struct i915_fragment_program *p, 
 @@ -394,7 +402,10 @@ upload_program(struct i915_fragment_program *p)
  
 /* Not always needed:
  */
 -   calc_live_regs(p);
 +   if (!calc_live_regs(p)) {
 +  i915_program_error(p, Could not allocate registers);
 +  return;
 +   }
  
 while (1) {
GLuint src0, src1, src2, flags;
 -- 
 1.7.4.4
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAk43UHYACgkQX1gOwKyEAw8PqwCcCh5n7385MCiXciB2kfbxMhoF
ruAAn1ezsWe0t0VYrKz7IDJ/MgW3F79K
=dExl
-END PGP SIGNATURE-
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev