[Mesa-dev] [PATCH] glsl_to_tgsi: allocate and enlarge arrays for temporaries on demand
Marek Olšák
maraeo at gmail.com
Sun Aug 24 14:51:30 PDT 2014
On Sun, Aug 24, 2014 at 6:15 PM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
> On Sun, Aug 24, 2014 at 7:44 AM, Marek Olšák <maraeo at gmail.com> wrote:
>> From: Marek Olšák <marek.olsak at amd.com>
>>
>> This fixes crashes if the number of temporaries is greater than 4096.
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=66184
>> Cc: 10.2 10.3 mesa-stable at lists.freedesktop.org
>> ---
>> src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 45 ++++++++++++++++++------------
>> 1 file changed, 27 insertions(+), 18 deletions(-)
>>
>> diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>> index 84bdc4f..9beecf8 100644
>> --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>> +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>> @@ -75,14 +75,6 @@ extern "C" {
>> (1 << PROGRAM_UNIFORM))
>>
>> /**
>> - * Maximum number of temporary registers.
>> - *
>> - * It is too big for stack allocated arrays -- it will cause stack overflow on
>> - * Windows and likely Mac OS X.
>> - */
>> -#define MAX_TEMPS 4096
>> -
>> -/**
>> * Maximum number of arrays
>> */
>> #define MAX_ARRAYS 256
>> @@ -3301,14 +3293,10 @@ get_src_arg_mask(st_dst_reg dst, st_src_reg src)
>> void
>> glsl_to_tgsi_visitor::simplify_cmp(void)
>> {
>> - unsigned *tempWrites;
>> + int tempWritesSize = 0;
>> + unsigned *tempWrites = NULL;
>> unsigned outputWrites[MAX_PROGRAM_OUTPUTS];
>>
>> - tempWrites = new unsigned[MAX_TEMPS];
>> - if (!tempWrites) {
>> - return;
>> - }
>> - memset(tempWrites, 0, sizeof(unsigned) * MAX_TEMPS);
>> memset(outputWrites, 0, sizeof(outputWrites));
>>
>> foreach_in_list(glsl_to_tgsi_instruction, inst, &this->instructions) {
>> @@ -3330,7 +3318,16 @@ glsl_to_tgsi_visitor::simplify_cmp(void)
>> prevWriteMask = outputWrites[inst->dst.index];
>> outputWrites[inst->dst.index] |= inst->dst.writemask;
>> } else if (inst->dst.file == PROGRAM_TEMPORARY) {
>> - assert(inst->dst.index < MAX_TEMPS);
>> + if (inst->dst.index >= tempWritesSize) {
>> + const int inc = 4096;
>> +
>> + tempWrites = (unsigned*)
>> + realloc(tempWrites,
>> + (tempWritesSize + inc) * sizeof(unsigned));
>
> This can fail, the old code dealt with new returning NULL. Not sure
> what the general policy for that is in mesa, but I have seen a lot of
> allocation checks. Probably makes sense here too.
>
>> + memset(tempWrites + tempWritesSize, 0, inc * sizeof(unsigned));
>> + tempWritesSize += inc;
>> + }
>> +
>> prevWriteMask = tempWrites[inst->dst.index];
>> tempWrites[inst->dst.index] |= inst->dst.writemask;
>> } else
>> @@ -3349,7 +3346,7 @@ glsl_to_tgsi_visitor::simplify_cmp(void)
>> }
>> }
>>
>> - delete [] tempWrites;
>> + free(tempWrites);
>> }
>>
>> /* Replaces all references to a temporary register index with another index. */
>> @@ -4158,7 +4155,9 @@ struct label {
>> struct st_translate {
>> struct ureg_program *ureg;
>>
>> - struct ureg_dst temps[MAX_TEMPS];
>> + unsigned temps_size;
>> + struct ureg_dst *temps;
>> +
>> struct ureg_dst arrays[MAX_ARRAYS];
>> struct ureg_src *constants;
>> struct ureg_src *immediates;
>> @@ -4299,7 +4298,16 @@ dst_register(struct st_translate *t,
>> return ureg_dst_undef();
>>
>> case PROGRAM_TEMPORARY:
>> - assert(index < Elements(t->temps));
>> + /* Allocate space for temporaries on demand. */
>> + if (index >= t->temps_size) {
>> + const int inc = 4096;
>> +
>> + t->temps = (struct ureg_dst*)
>> + realloc(t->temps,
>> + (t->temps_size + inc) * sizeof(struct ureg_dst));
>
> Check alloc?
>
> With that fixed,
>
> Reviewed-by: Ilia Mirkin <imirkin at alum.mit.edu>
>
> BTW, should this in any way be related to PIPE_SHADER_CAP_MAX_TEMPS?
> Looking at what drivers set this to, it's fairly small values. e.g.
> nvc0 returns 128. I guess there's limited spilling space. OTOH, it can
> do a *lot* better than TGSI at storing stuff due to optimizations,
> register reuse, etc.
I think the limit only matters for ARB program queries. It should be
ignored by GLSL, because most drivers do register allocation anyway.
Marek
More information about the mesa-dev
mailing list