[Mesa-dev] [PATCH] etnaviv: fix shader miscompilation with more than 16 labels

Christian Gmeiner christian.gmeiner at gmail.com
Mon Jun 26 12:40:09 UTC 2017


2017-06-26 13:44 GMT+02:00 Lucas Stach <l.stach at pengutronix.de>:
> Am Montag, den 26.06.2017, 13:40 +0200 schrieb Lucas Stach:
>> The labels array may change its virtual address on a reallocation, so
>> it is invalid to cache pointers into the array. Rather than using the
>> pointer directly, remember the array index.
>>
>> Fixes miscompilation of shaders in glmark2 ideas, leading to GPU
>> hangs.
>
> Forgot to add: the change seems straightforward enough to CC it to the
> stable trees.
>

Will add stable trees before pushing it.

>> Signed-off-by: Lucas Stach <l.stach at pengutronix.de>
>> ---
>>  src/gallium/drivers/etnaviv/etnaviv_compiler.c | 57 ++++++++++++++
>> ------------
>>  1 file changed, 30 insertions(+), 27 deletions(-)
>>
>> diff --git a/src/gallium/drivers/etnaviv/etnaviv_compiler.c
>> b/src/gallium/drivers/etnaviv/etnaviv_compiler.c
>> index eafb511bb813..8883e39cf4b3 100644
>> --- a/src/gallium/drivers/etnaviv/etnaviv_compiler.c
>> +++ b/src/gallium/drivers/etnaviv/etnaviv_compiler.c
>> @@ -119,10 +119,10 @@ enum etna_compile_frame_type {
>>   */
>>  struct etna_compile_frame {
>>     enum etna_compile_frame_type type;
>> -   struct etna_compile_label *lbl_else;
>> -   struct etna_compile_label *lbl_endif;
>> -   struct etna_compile_label *lbl_loop_bgn;
>> -   struct etna_compile_label *lbl_loop_end;
>> +   int lbl_else_idx;
>> +   int lbl_endif_idx;
>> +   int lbl_loop_bgn_idx;
>> +   int lbl_loop_end_idx;
>>  };
>>
>>  struct etna_compile_file {
>> @@ -178,7 +178,7 @@ struct etna_compile {
>>     /* Fields for handling nested conditionals */
>>     struct etna_compile_frame frame_stack[ETNA_MAX_DEPTH];
>>     int frame_sp;
>> -   struct etna_compile_label *lbl_usage[ETNA_MAX_INSTRUCTIONS];
>> +   int lbl_usage[ETNA_MAX_INSTRUCTIONS];
>>
>>     unsigned labels_count, labels_sz;
>>     struct etna_compile_label *labels;
>> @@ -990,7 +990,7 @@ etna_src_uniforms_conflict(struct etna_inst_src
>> a, struct etna_inst_src b)
>>  }
>>
>>  /* create a new label */
>> -static struct etna_compile_label *
>> +static unsigned int
>>  alloc_new_label(struct etna_compile *c)
>>  {
>>     struct etna_compile_label label = {
>> @@ -999,7 +999,7 @@ alloc_new_label(struct etna_compile *c)
>>
>>     array_insert(c->labels, label);
>>
>> -   return &c->labels[c->labels_count - 1];
>> +   return c->labels_count - 1;
>>  }
>>
>>  /* place label at current instruction pointer */
>> @@ -1015,10 +1015,10 @@ label_place(struct etna_compile *c, struct
>> etna_compile_label *label)
>>   * as the value becomes known.
>>   */
>>  static void
>> -label_mark_use(struct etna_compile *c, struct etna_compile_label
>> *label)
>> +label_mark_use(struct etna_compile *c, int lbl_idx)
>>  {
>>     assert(c->inst_ptr < ETNA_MAX_INSTRUCTIONS);
>> -   c->lbl_usage[c->inst_ptr] = label;
>> +   c->lbl_usage[c->inst_ptr] = lbl_idx;
>>  }
>>
>>  /* walk the frame stack and return first frame with matching type */
>> @@ -1099,8 +1099,8 @@ trans_if(const struct instr_translater *t,
>> struct etna_compile *c,
>>     /* push IF to stack */
>>     f->type = ETNA_COMPILE_FRAME_IF;
>>     /* create "else" label */
>> -   f->lbl_else = alloc_new_label(c);
>> -   f->lbl_endif = NULL;
>> +   f->lbl_else_idx = alloc_new_label(c);
>> +   f->lbl_endif_idx = -1;
>>
>>     /* We need to avoid the emit_inst() below becoming two
>> instructions */
>>     if (etna_src_uniforms_conflict(src[0], imm_0))
>> @@ -1108,7 +1108,7 @@ trans_if(const struct instr_translater *t,
>> struct etna_compile *c,
>>
>>     /* mark position in instruction stream of label reference so that
>> it can be
>>      * filled in in next pass */
>> -   label_mark_use(c, f->lbl_else);
>> +   label_mark_use(c, f->lbl_else_idx);
>>
>>     /* create conditional branch to label if src0 EQ 0 */
>>     emit_inst(c, &(struct etna_inst){
>> @@ -1129,8 +1129,8 @@ trans_else(const struct instr_translater *t,
>> struct etna_compile *c,
>>     assert(f->type == ETNA_COMPILE_FRAME_IF);
>>
>>     /* create "endif" label, and branch to endif label */
>> -   f->lbl_endif = alloc_new_label(c);
>> -   label_mark_use(c, f->lbl_endif);
>> +   f->lbl_endif_idx = alloc_new_label(c);
>> +   label_mark_use(c, f->lbl_endif_idx);
>>     emit_inst(c, &(struct etna_inst) {
>>        .opcode = INST_OPCODE_BRANCH,
>>        .cond = INST_CONDITION_TRUE,
>> @@ -1138,7 +1138,7 @@ trans_else(const struct instr_translater *t,
>> struct etna_compile *c,
>>     });
>>
>>     /* mark "else" label at this position in instruction stream */
>> -   label_place(c, f->lbl_else);
>> +   label_place(c, &c->labels[f->lbl_else_idx]);
>>  }
>>
>>  static void
>> @@ -1151,10 +1151,10 @@ trans_endif(const struct instr_translater *t,
>> struct etna_compile *c,
>>
>>     /* assign "endif" or "else" (if no ELSE) label to current
>> position in
>>      * instruction stream, pop IF */
>> -   if (f->lbl_endif != NULL)
>> -      label_place(c, f->lbl_endif);
>> +   if (f->lbl_endif_idx != -1)
>> +      label_place(c, &c->labels[f->lbl_endif_idx]);
>>     else
>> -      label_place(c, f->lbl_else);
>> +      label_place(c, &c->labels[f->lbl_else_idx]);
>>  }
>>
>>  static void
>> @@ -1166,10 +1166,10 @@ trans_loop_bgn(const struct instr_translater
>> *t, struct etna_compile *c,
>>
>>     /* push LOOP to stack */
>>     f->type = ETNA_COMPILE_FRAME_LOOP;
>> -   f->lbl_loop_bgn = alloc_new_label(c);
>> -   f->lbl_loop_end = alloc_new_label(c);
>> +   f->lbl_loop_bgn_idx = alloc_new_label(c);
>> +   f->lbl_loop_end_idx = alloc_new_label(c);
>>
>> -   label_place(c, f->lbl_loop_bgn);
>> +   label_place(c, &c->labels[f->lbl_loop_bgn_idx]);
>>
>>     c->num_loops++;
>>  }
>> @@ -1185,7 +1185,7 @@ trans_loop_end(const struct instr_translater
>> *t, struct etna_compile *c,
>>
>>     /* mark position in instruction stream of label reference so that
>> it can be
>>      * filled in in next pass */
>> -   label_mark_use(c, f->lbl_loop_bgn);
>> +   label_mark_use(c, f->lbl_loop_bgn_idx);
>>
>>     /* create branch to loop_bgn label */
>>     emit_inst(c, &(struct etna_inst) {
>> @@ -1195,7 +1195,7 @@ trans_loop_end(const struct instr_translater
>> *t, struct etna_compile *c,
>>        /* imm is filled in later */
>>     });
>>
>> -   label_place(c, f->lbl_loop_end);
>> +   label_place(c, &c->labels[f->lbl_loop_end_idx]);
>>  }
>>
>>  static void
>> @@ -1207,7 +1207,7 @@ trans_brk(const struct instr_translater *t,
>> struct etna_compile *c,
>>
>>     /* mark position in instruction stream of label reference so that
>> it can be
>>      * filled in in next pass */
>> -   label_mark_use(c, f->lbl_loop_end);
>> +   label_mark_use(c, f->lbl_loop_end_idx);
>>
>>     /* create branch to loop_end label */
>>     emit_inst(c, &(struct etna_inst) {
>> @@ -1227,7 +1227,7 @@ trans_cont(const struct instr_translater *t,
>> struct etna_compile *c,
>>
>>     /* mark position in instruction stream of label reference so that
>> it can be
>>      * filled in in next pass */
>> -   label_mark_use(c, f->lbl_loop_bgn);
>> +   label_mark_use(c, f->lbl_loop_bgn_idx);
>>
>>     /* create branch to loop_end label */
>>     emit_inst(c, &(struct etna_inst) {
>> @@ -1998,8 +1998,9 @@ static void
>>  etna_compile_fill_in_labels(struct etna_compile *c)
>>  {
>>     for (int idx = 0; idx < c->inst_ptr; ++idx) {
>> -      if (c->lbl_usage[idx])
>> -         etna_assemble_set_imm(&c->code[idx * 4], c->lbl_usage[idx]-
>> >inst_idx);
>> +      if (c->lbl_usage[idx] != -1)
>> +         etna_assemble_set_imm(&c->code[idx * 4],
>> +                               c->labels[c-
>> >lbl_usage[idx]].inst_idx);
>>     }
>>  }
>>
>> @@ -2301,6 +2302,8 @@ etna_compile_shader(struct etna_shader_variant
>> *v)
>>     if (!c)
>>        return false;
>>
>> +   memset(&c->lbl_usage, -1, ARRAY_SIZE(c->lbl_usage));
>> +
>>     const struct tgsi_token *tokens = v->shader->tokens;
>>
>>     c->specs = specs;
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev

greets
--
Christian Gmeiner, MSc

https://www.youtube.com/user/AloryOFFICIAL
https://soundcloud.com/christian-gmeiner


More information about the mesa-dev mailing list