[Mesa-dev] [PATCH] st/glsl_to_tgsi: fix theoretical memory leak

Gert Wollny gw.fossdev at gmail.com
Thu Sep 14 08:08:39 UTC 2017


Sorry for the lapse and thanks for fixing it. 

Reviewed-By: Gert Wollny <gw.fossdev at gmail.com>  


Am Mittwoch, den 13.09.2017, 19:05 +0200 schrieb Nicolai Hähnle:
> From: Nicolai Hähnle <nicolai.haehnle at amd.com>
> 
> It can't *really* happen since we don't use subroutines.

> CID: 1417491
> ---
>  src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp
> b/src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp
> index 374393bb86e..76c198e165b 100644
> --- a/src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp
> +++ b/src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp
> @@ -610,20 +610,21 @@ static void dump_instruction(int line,
> prog_scope *scope,
>   */
>  bool
>  get_temp_registers_required_lifetimes(void *mem_ctx, exec_list
> *instructions,
>                                        int ntemps, struct lifetime
> *lifetimes)
>  {
>     int line = 0;
>     int loop_id = 0;
>     int if_id = 0;
>     int switch_id = 0;
>     bool is_at_end = false;
> +   bool ok = true;
>     int n_scopes = 1;
>  
>     /* Count scopes to allocate the needed space without the need for
>      * re-allocation
>      */
>     foreach_in_list(glsl_to_tgsi_instruction, inst, instructions) {
>        if (inst->op == TGSI_OPCODE_BGNLOOP ||
>            inst->op == TGSI_OPCODE_SWITCH ||
>            inst->op == TGSI_OPCODE_CASE ||
>            inst->op == TGSI_OPCODE_IF ||
> @@ -751,21 +752,22 @@ get_temp_registers_required_lifetimes(void
> *mem_ctx, exec_list *instructions,
>           break;
>        }
>        case TGSI_OPCODE_CAL:
>        case TGSI_OPCODE_RET:
>           /* These opcodes are not supported and if a subroutine
> would
>            * be called in a shader, then the lifetime tracking would
> have
>            * to follow that call to see which registers are used
> there.
>            * Since this is not done, we have to bail out here and
> signal
>            * that no register merge will take place.
>            */
> -         return false;
> +         ok = false;
> +         goto out;
>        default: {
>           for (unsigned j = 0; j < num_inst_src_regs(inst); j++) {
>              const st_src_reg& src = inst->src[j];
>              if (src.file == PROGRAM_TEMPORARY)
>                 acc[src.index].record_read(line, cur_scope,
> src.swizzle);
>           }
>           for (unsigned j = 0; j < inst->tex_offset_num_offset; j++)
> {
>              const st_src_reg& src = inst->tex_offsets[j];
>              if (src.file == PROGRAM_TEMPORARY)
>                 acc[src.index].record_read(line, cur_scope,
> src.swizzle);
> @@ -790,22 +792,23 @@ get_temp_registers_required_lifetimes(void
> *mem_ctx, exec_list *instructions,
>  
>     RENAME_DEBUG(cerr << "========= lifetimes ==============\n");
>     for(int i = 0; i < ntemps; ++i) {
>        RENAME_DEBUG(cerr << setw(4) << i);
>        lifetimes[i] = acc[i].get_required_lifetime();
>        RENAME_DEBUG(cerr << ": [" << lifetimes[i].begin << ", "
>                          << lifetimes[i].end << "]\n");
>     }
>     RENAME_DEBUG(cerr << "==================================\n\n");
>  
> +out:
>     delete[] acc;
> -   return true;
> +   return ok;
>  }
>  
>  /* Find the next register between [start, end) that has a life time
> starting
>   * at or after bound by using a binary search.
>   * start points at the beginning of the search range,
>   * end points at the element past the end of the search range, and
>   * the array comprising [start, end) must be sorted in ascending
> order.
>   */
>  static access_record*
>  find_next_rename(access_record* start, access_record* end, int
> bound)


More information about the mesa-dev mailing list