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

[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" <idr at freedesktop.org> wrote:
>> From: Ian Romanick <ian.d.romanick at 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 = 0xffff0000;
>> @@ -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-----