[Mesa-dev] [PATCH 2/2] panfrost: Implement custom block/instruction iterators
Boris Brezillon
boris.brezillon at collabora.com
Tue Aug 13 19:25:45 UTC 2019
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.brezillon at 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
More information about the mesa-dev
mailing list