The generic list helpers are too restrictive for us: we want to be able to update the instruction pointer within the foreach body, and the list_assert() check done in list_for_each_entry() prevents it. Sometimes we also want to update the next_ins pointer (in case we delete/replace the next instruction by something else).
Let's implement our own iterators (still based on the existing list helpers) to address this limitation. Signed-off-by: Boris Brezillon <boris.brezil...@collabora.com> --- src/panfrost/midgard/compiler.h | 75 ++++++++++++++----- src/panfrost/midgard/midgard_compile.c | 2 +- src/panfrost/midgard/midgard_derivatives.c | 4 +- src/panfrost/midgard/midgard_liveness.c | 1 + src/panfrost/midgard/midgard_opt_copy_prop.c | 2 +- src/panfrost/midgard/midgard_opt_dce.c | 6 +- src/panfrost/midgard/midgard_opt_invert.c | 4 +- .../midgard/midgard_opt_perspective.c | 4 +- src/panfrost/midgard/midgard_ra.c | 4 +- src/panfrost/midgard/midgard_schedule.c | 18 +++-- src/panfrost/midgard/mir_promote_uniforms.c | 2 +- 11 files changed, 84 insertions(+), 38 deletions(-) diff --git a/src/panfrost/midgard/compiler.h b/src/panfrost/midgard/compiler.h index d62157f3be4f..19f65ddc96ec 100644 --- a/src/panfrost/midgard/compiler.h +++ b/src/panfrost/midgard/compiler.h @@ -305,44 +305,83 @@ mir_remove_instruction(struct midgard_instruction *ins) list_del(&ins->link); } -static inline midgard_instruction* -mir_prev_op(struct midgard_instruction *ins) +static inline midgard_instruction * +mir_first_op(struct midgard_block *block) { - return list_last_entry(&(ins->link), midgard_instruction, link); + assert(block); + return list_first_entry(&block->instructions, struct midgard_instruction, link); } -static inline midgard_instruction* +static inline midgard_instruction * +mir_last_op(struct midgard_block *block) +{ + assert(block); + return list_last_entry(&block->instructions, struct midgard_instruction, link); +} + +static inline midgard_instruction * +mir_prev_op(struct midgard_instruction *ins) +{ + assert(ins); + return list_last_entry(&ins->link, struct midgard_instruction, link); +} + +static inline midgard_instruction * mir_next_op(struct midgard_instruction *ins) { - return list_first_entry(&(ins->link), midgard_instruction, link); + assert(ins); + return list_first_entry(&ins->link, struct midgard_instruction, link); +} + +static inline struct midgard_block *mir_first_block(compiler_context *ctx) +{ + assert(ctx); + return list_first_entry(&ctx->blocks, struct midgard_block, link); +} + +static inline struct midgard_block *mir_next_block(struct midgard_block *b) +{ + assert(b); + return list_first_entry(&b->link, struct midgard_block, link); } #define mir_foreach_block(ctx, v) \ - list_for_each_entry(struct midgard_block, v, &ctx->blocks, link) + for (struct midgard_block *v = mir_first_block(ctx); \ + &v->link != &ctx->blocks; v = mir_next_block(v)) #define mir_foreach_block_from(ctx, from, v) \ - list_for_each_entry_from(struct midgard_block, v, from, &ctx->blocks, link) + for (struct midgard_block *v = from; &v->link != &ctx->blocks; \ + v = mir_next_block(v)) #define mir_foreach_instr(ctx, v) \ - list_for_each_entry(struct midgard_instruction, v, &ctx->current_block->instructions, link) + for (struct midgard_instruction *v = mir_first_op(ctx->current_block); \ + &v->link != &ctx->current_block->instructions; v = mir_next_op(v)) -#define mir_foreach_instr_safe(ctx, v) \ - list_for_each_entry_safe(struct midgard_instruction, v, &ctx->current_block->instructions, link) +#define mir_foreach_instr_safe(ctx, v, n) \ + for (struct midgard_instruction *v = mir_first_op(ctx->current_block), \ + struct midgard_instruction *n = mir_next_op(v); \ + &v->link != &ctx->current_block->instructions; \ + v = n, n = mir_next_op(v)) #define mir_foreach_instr_in_block(block, v) \ - list_for_each_entry(struct midgard_instruction, v, &block->instructions, link) + for (struct midgard_instruction *v = mir_first_op(block); \ + &v->link != &block->instructions; v = mir_next_op(v)) -#define mir_foreach_instr_in_block_safe(block, v) \ - list_for_each_entry_safe(struct midgard_instruction, v, &block->instructions, link) +#define mir_foreach_instr_in_block_safe(block, v, n) \ + for (struct midgard_instruction *v = mir_first_op(block), *n = mir_next_op(v); \ + &v->link != &block->instructions; v = n, n = mir_next_op(v)) #define mir_foreach_instr_in_block_safe_rev(block, v) \ - list_for_each_entry_safe_rev(struct midgard_instruction, v, &block->instructions, link) + for (struct midgard_instruction *v = mir_last_op(block), *p = mir_prev_op(v); \ + &v->link != &block->instructions; v = p, p = mir_prev_op(v)) #define mir_foreach_instr_in_block_from(block, v, from) \ - list_for_each_entry_from(struct midgard_instruction, v, from, &block->instructions, link) + for (struct midgard_instruction *v = from; \ + &v->link != &block->instructions; v = mir_next_op(v)) #define mir_foreach_instr_in_block_from_rev(block, v, from) \ - list_for_each_entry_from_rev(struct midgard_instruction, v, from, &block->instructions, link) + for (struct midgard_instruction *v = from, *p = mir_prev_op(v); \ + &v->link != &block->instructions; v = p, p = mir_prev_op(v)) #define mir_foreach_bundle_in_block(block, v) \ util_dynarray_foreach(&block->bundles, midgard_bundle, v) @@ -351,9 +390,9 @@ mir_next_op(struct midgard_instruction *ins) mir_foreach_block(ctx, v_block) \ mir_foreach_instr_in_block(v_block, v) -#define mir_foreach_instr_global_safe(ctx, v) \ +#define mir_foreach_instr_global_safe(ctx, v, n) \ mir_foreach_block(ctx, v_block) \ - mir_foreach_instr_in_block_safe(v_block, v) + mir_foreach_instr_in_block_safe(v_block, v, n) static inline midgard_instruction * mir_last_in_block(struct midgard_block *block) diff --git a/src/panfrost/midgard/midgard_compile.c b/src/panfrost/midgard/midgard_compile.c index e261e80c06c5..aa97a2ac6b8e 100644 --- a/src/panfrost/midgard/midgard_compile.c +++ b/src/panfrost/midgard/midgard_compile.c @@ -2116,7 +2116,7 @@ midgard_opt_cull_dead_branch(compiler_context *ctx, midgard_block *block) { bool branched = false; - mir_foreach_instr_in_block_safe(block, ins) { + mir_foreach_instr_in_block_safe(block, ins, next_ins) { if (!midgard_is_branch_unit(ins->unit)) continue; /* We ignore prepacked branches since the fragment epilogue is diff --git a/src/panfrost/midgard/midgard_derivatives.c b/src/panfrost/midgard/midgard_derivatives.c index 0f15af3db427..6717063aae3d 100644 --- a/src/panfrost/midgard/midgard_derivatives.c +++ b/src/panfrost/midgard/midgard_derivatives.c @@ -122,7 +122,7 @@ midgard_emit_derivatives(compiler_context *ctx, nir_alu_instr *instr) void midgard_lower_derivatives(compiler_context *ctx, midgard_block *block) { - mir_foreach_instr_in_block_safe(block, ins) { + mir_foreach_instr_in_block_safe(block, ins, next_ins) { if (ins->type != TAG_TEXTURE_4) continue; if (!OP_IS_DERIVATIVE(ins->texture.op)) continue; @@ -150,7 +150,7 @@ midgard_lower_derivatives(compiler_context *ctx, midgard_block *block) dup.texture.in_reg_swizzle = SWIZZLE_ZWWW; /* Insert the new instruction */ - mir_insert_instruction_before(mir_next_op(ins), dup); + mir_insert_instruction_before(next_ins, dup); /* TODO: Set .cont/.last automatically via dataflow analysis */ ctx->texture_op_count++; diff --git a/src/panfrost/midgard/midgard_liveness.c b/src/panfrost/midgard/midgard_liveness.c index 8ecb22ee2739..9ae47ac4f513 100644 --- a/src/panfrost/midgard/midgard_liveness.c +++ b/src/panfrost/midgard/midgard_liveness.c @@ -75,6 +75,7 @@ mir_is_live_after(compiler_context *ctx, midgard_block *block, midgard_instructi { /* Check the rest of the block for liveness */ + assert(&start->link != &block->instructions); mir_foreach_instr_in_block_from(block, ins, mir_next_op(start)) { if (mir_has_arg(ins, src)) return true; diff --git a/src/panfrost/midgard/midgard_opt_copy_prop.c b/src/panfrost/midgard/midgard_opt_copy_prop.c index dc5579c4d463..183e3d3e4460 100644 --- a/src/panfrost/midgard/midgard_opt_copy_prop.c +++ b/src/panfrost/midgard/midgard_opt_copy_prop.c @@ -30,7 +30,7 @@ midgard_opt_copy_prop(compiler_context *ctx, midgard_block *block) { bool progress = false; - mir_foreach_instr_in_block_safe(block, ins) { + mir_foreach_instr_in_block_safe(block, ins, next_ins) { if (ins->type != TAG_ALU_4) continue; if (!OP_IS_MOVE(ins->alu.op)) continue; diff --git a/src/panfrost/midgard/midgard_opt_dce.c b/src/panfrost/midgard/midgard_opt_dce.c index 9964675763c2..caf3ad45134b 100644 --- a/src/panfrost/midgard/midgard_opt_dce.c +++ b/src/panfrost/midgard/midgard_opt_dce.c @@ -31,7 +31,7 @@ midgard_opt_dead_code_eliminate(compiler_context *ctx, midgard_block *block) { bool progress = false; - mir_foreach_instr_in_block_safe(block, ins) { + mir_foreach_instr_in_block_safe(block, ins, next_ins) { if (ins->type != TAG_ALU_4) continue; if (ins->compact_branch) continue; @@ -54,7 +54,7 @@ midgard_opt_dead_move_eliminate(compiler_context *ctx, midgard_block *block) { bool progress = false; - mir_foreach_instr_in_block_safe(block, ins) { + mir_foreach_instr_in_block_safe(block, ins, next_ins) { if (ins->type != TAG_ALU_4) continue; if (ins->compact_branch) continue; if (!OP_IS_MOVE(ins->alu.op)) continue; @@ -93,7 +93,7 @@ midgard_opt_dead_move_eliminate(compiler_context *ctx, midgard_block *block) void midgard_opt_post_move_eliminate(compiler_context *ctx, midgard_block *block, struct ra_graph *g) { - mir_foreach_instr_in_block_safe(block, ins) { + mir_foreach_instr_in_block_safe(block, ins, next_ins) { if (ins->type != TAG_ALU_4) continue; if (ins->compact_branch) continue; if (!OP_IS_MOVE(ins->alu.op)) continue; diff --git a/src/panfrost/midgard/midgard_opt_invert.c b/src/panfrost/midgard/midgard_opt_invert.c index 422be6ef03e5..e46b44ec06a3 100644 --- a/src/panfrost/midgard/midgard_opt_invert.c +++ b/src/panfrost/midgard/midgard_opt_invert.c @@ -31,7 +31,7 @@ void midgard_lower_invert(compiler_context *ctx, midgard_block *block) { - mir_foreach_instr_in_block_safe(block, ins) { + mir_foreach_instr_in_block_safe(block, ins, next_ins) { if (ins->type != TAG_ALU_4) continue; if (!ins->invert) continue; @@ -58,7 +58,7 @@ midgard_lower_invert(compiler_context *ctx, midgard_block *block) ins->ssa_args.dest = temp; ins->invert = false; - mir_insert_instruction_before(mir_next_op(ins), not); + mir_insert_instruction_before(next_ins, not); } } diff --git a/src/panfrost/midgard/midgard_opt_perspective.c b/src/panfrost/midgard/midgard_opt_perspective.c index 993000923b93..8d2f2a235590 100644 --- a/src/panfrost/midgard/midgard_opt_perspective.c +++ b/src/panfrost/midgard/midgard_opt_perspective.c @@ -42,7 +42,7 @@ midgard_opt_combine_projection(compiler_context *ctx, midgard_block *block) { bool progress = false; - mir_foreach_instr_in_block_safe(block, ins) { + mir_foreach_instr_in_block_safe(block, ins, next_ins) { /* First search for fmul */ if (ins->type != TAG_ALU_4) continue; if (ins->alu.op != midgard_alu_op_fmul) continue; @@ -141,7 +141,7 @@ midgard_opt_varying_projection(compiler_context *ctx, midgard_block *block) { bool progress = false; - mir_foreach_instr_in_block_safe(block, ins) { + mir_foreach_instr_in_block_safe(block, ins, next_ins) { /* Search for a projection */ if (ins->type != TAG_LOAD_STORE_4) continue; if (!OP_IS_PROJECTION(ins->load_store.op)) continue; diff --git a/src/panfrost/midgard/midgard_ra.c b/src/panfrost/midgard/midgard_ra.c index 4754971acb2a..46fc5263e395 100644 --- a/src/panfrost/midgard/midgard_ra.c +++ b/src/panfrost/midgard/midgard_ra.c @@ -481,7 +481,7 @@ mir_lower_special_reads(compiler_context *ctx) /* Insert move before each read/write, depending on the * hazard we're trying to account for */ - mir_foreach_instr_global_safe(ctx, pre_use) { + mir_foreach_instr_global_safe(ctx, pre_use, use) { if (pre_use->type != classes[j]) continue; @@ -494,8 +494,6 @@ mir_lower_special_reads(compiler_context *ctx) } if (hazard_write) { - midgard_instruction *use = mir_next_op(pre_use); - assert(use); mir_insert_instruction_before(use, m); mir_rewrite_index_dst_tag(ctx, i, idx, classes[j]); } else { diff --git a/src/panfrost/midgard/midgard_schedule.c b/src/panfrost/midgard/midgard_schedule.c index 7ec0d4428a43..97b7ce7936f8 100644 --- a/src/panfrost/midgard/midgard_schedule.c +++ b/src/panfrost/midgard/midgard_schedule.c @@ -556,6 +556,7 @@ schedule_bundle(compiler_context *ctx, midgard_block *block, midgard_instruction midgard_instruction *uins = ins; for (; packed_idx < bundle.instruction_count; ++packed_idx) { + assert(&uins->link != &block->instructions); bundle.instructions[packed_idx] = uins; uins = mir_next_op(uins); } @@ -586,8 +587,10 @@ schedule_block(compiler_context *ctx, midgard_block *block) ctx->blend_constant_offset = quadwords_within_block * 0x10; } - while(skip--) + while(skip--) { + assert(&ins->link != &block->instructions); ins = mir_next_op(ins); + } block->quadword_count += quadword_size(bundle.tag); } @@ -600,7 +603,7 @@ schedule_block(compiler_context *ctx, midgard_block *block) static void midgard_pair_load_store(compiler_context *ctx, midgard_block *block) { - mir_foreach_instr_in_block_safe(block, ins) { + mir_foreach_instr_in_block_safe(block, ins, next_ins) { if (ins->type != TAG_LOAD_STORE_4) continue; /* We've found a load/store op. Check if next is also load/store. */ @@ -634,6 +637,9 @@ midgard_pair_load_store(compiler_context *ctx, midgard_block *block) /* We found one! Move it up to pair and remove it from the old location */ + if (c == next_ins) + next_ins = mir_next_op(c); + mir_insert_instruction_before(ins, *c); mir_remove_instruction(c); @@ -782,7 +788,7 @@ static void mir_spill_register( * implicitly. For special writes, spill to a work register */ if (!is_special || is_special_w) { - mir_foreach_instr_global_safe(ctx, ins) { + mir_foreach_instr_global_safe(ctx, ins, next_ins) { if (ins->ssa_args.dest != spill_node) continue; midgard_instruction st; @@ -796,7 +802,7 @@ static void mir_spill_register( st = v_load_store_scratch(ins->ssa_args.dest, spill_slot, true, ins->mask); } - spill_move = mir_insert_instruction_before(mir_next_op(ins), st); + spill_move = mir_insert_instruction_before(next_ins, st); if (!is_special) ctx->spills++; @@ -844,8 +850,10 @@ static void mir_spill_register( midgard_instruction *before = ins; /* For a csel, go back one more not to break up the bundle */ - if (ins->type == TAG_ALU_4 && OP_IS_CSEL(ins->alu.op)) + if (ins->type == TAG_ALU_4 && OP_IS_CSEL(ins->alu.op)) { before = mir_prev_op(before); + assert(&before->link != &block->instructions); + } midgard_instruction st; diff --git a/src/panfrost/midgard/mir_promote_uniforms.c b/src/panfrost/midgard/mir_promote_uniforms.c index 9f5be37be2cf..7dc09fcdf147 100644 --- a/src/panfrost/midgard/mir_promote_uniforms.c +++ b/src/panfrost/midgard/mir_promote_uniforms.c @@ -39,7 +39,7 @@ void midgard_promote_uniforms(compiler_context *ctx, unsigned promoted_count) { - mir_foreach_instr_global_safe(ctx, ins) { + mir_foreach_instr_global_safe(ctx, ins, next_ins) { if (ins->type != TAG_LOAD_STORE_4) continue; if (!OP_IS_UBO_READ(ins->load_store.op)) continue; -- 2.21.0 _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev