[1/2] panfrost: Don't use mir_foreach_instr_in_block_safe() when not needed

Submitted by Boris Brezillon on Aug. 13, 2019, 7:25 p.m.

Details

Message ID 20190813192545.13349-1-boris.brezillon@collabora.com
State New
Headers show
Series "Series without cover letter" ( rev: 2 1 ) in Mesa

Not browsing as part of any series.

Commit Message

Boris Brezillon Aug. 13, 2019, 7:25 p.m.
mir_foreach_instr_in_block_safe() is only needed if the caller intend
to remove the current item from the list. Downgrade to
mir_foreach_instr_in_block() when this is not the case.

Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
---
 src/panfrost/midgard/midgard_compile.c         | 2 +-
 src/panfrost/midgard/midgard_opt_invert.c      | 6 +++---
 src/panfrost/midgard/midgard_opt_perspective.c | 6 +++---
 src/panfrost/midgard/midgard_schedule.c        | 2 +-
 4 files changed, 8 insertions(+), 8 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/panfrost/midgard/midgard_compile.c b/src/panfrost/midgard/midgard_compile.c
index 460847e82660..e261e80c06c5 100644
--- a/src/panfrost/midgard/midgard_compile.c
+++ b/src/panfrost/midgard/midgard_compile.c
@@ -2161,7 +2161,7 @@  midgard_opt_pos_propagate(compiler_context *ctx, midgard_block *block)
 {
         bool progress = false;
 
-        mir_foreach_instr_in_block_safe(block, ins) {
+        mir_foreach_instr_in_block(block, ins) {
                 if (ins->type != TAG_ALU_4) continue;
                 if (ins->alu.op != midgard_alu_op_fmov) continue;
                 if (ins->alu.outmod != midgard_outmod_pos) continue;
diff --git a/src/panfrost/midgard/midgard_opt_invert.c b/src/panfrost/midgard/midgard_opt_invert.c
index b68c98c74dbb..422be6ef03e5 100644
--- a/src/panfrost/midgard/midgard_opt_invert.c
+++ b/src/panfrost/midgard/midgard_opt_invert.c
@@ -69,7 +69,7 @@  midgard_opt_not_propagate(compiler_context *ctx, midgard_block *block)
 {
         bool progress = false;
 
-        mir_foreach_instr_in_block_safe(block, ins) {
+        mir_foreach_instr_in_block(block, ins) {
                 if (ins->type != TAG_ALU_4) continue;
                 if (ins->alu.op != midgard_alu_op_imov) continue;
                 if (!ins->invert) continue;
@@ -162,7 +162,7 @@  midgard_opt_fuse_dest_invert(compiler_context *ctx, midgard_block *block)
 {
         bool progress = false;
 
-        mir_foreach_instr_in_block_safe(block, ins) {
+        mir_foreach_instr_in_block(block, ins) {
                 /* Search for inverted bitwise */
                 if (ins->type != TAG_ALU_4) continue;
                 if (!mir_is_bitwise(ins)) continue;
@@ -213,7 +213,7 @@  midgard_opt_fuse_src_invert(compiler_context *ctx, midgard_block *block)
 {
         bool progress = false;
 
-        mir_foreach_instr_in_block_safe(block, ins) {
+        mir_foreach_instr_in_block(block, ins) {
                 /* Search for inverted bitwise */
                 if (ins->type != TAG_ALU_4) continue;
                 if (!mir_is_bitwise(ins)) continue;
diff --git a/src/panfrost/midgard/midgard_opt_perspective.c b/src/panfrost/midgard/midgard_opt_perspective.c
index f5e483e68d0e..993000923b93 100644
--- a/src/panfrost/midgard/midgard_opt_perspective.c
+++ b/src/panfrost/midgard/midgard_opt_perspective.c
@@ -71,7 +71,7 @@  midgard_opt_combine_projection(compiler_context *ctx, midgard_block *block)
                 unsigned frcp_component = 0;
                 unsigned frcp_from = 0;
 
-                mir_foreach_instr_in_block_safe(block, sub) {
+                mir_foreach_instr_in_block(block, sub) {
                         if (sub->ssa_args.dest != frcp) continue;
 
                         midgard_vector_alu_src s =
@@ -97,7 +97,7 @@  midgard_opt_combine_projection(compiler_context *ctx, midgard_block *block)
                 /* One for frcp and one for fmul */
                 if (mir_use_count(ctx, frcp_from) > 2) continue;
 
-                mir_foreach_instr_in_block_safe(block, v) {
+                mir_foreach_instr_in_block(block, v) {
                         if (v->ssa_args.dest != frcp_from) continue;
                         if (v->type != TAG_LOAD_STORE_4) break;
                         if (!OP_IS_LOAD_VARY_F(v->load_store.op)) break;
@@ -157,7 +157,7 @@  midgard_opt_varying_projection(compiler_context *ctx, midgard_block *block)
 
                 bool rewritten = false;
 
-                mir_foreach_instr_in_block_safe(block, v) {
+                mir_foreach_instr_in_block(block, v) {
                         if (v->ssa_args.dest != vary) continue;
                         if (v->type != TAG_LOAD_STORE_4) break;
                         if (!OP_IS_LOAD_VARY_F(v->load_store.op)) break;
diff --git a/src/panfrost/midgard/midgard_schedule.c b/src/panfrost/midgard/midgard_schedule.c
index 486bb38e0493..7ec0d4428a43 100644
--- a/src/panfrost/midgard/midgard_schedule.c
+++ b/src/panfrost/midgard/midgard_schedule.c
@@ -806,7 +806,7 @@  static void mir_spill_register(
         /* For special reads, figure out how many components we need */
         unsigned read_mask = 0;
 
-        mir_foreach_instr_global_safe(ctx, ins) {
+        mir_foreach_instr_global(ctx, ins) {
                 read_mask |= mir_mask_of_read_components(ins, spill_node);
         }
 

Comments


On Tue, 13 Aug 2019 12:55:57 -0700
Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com> wrote:

> > +++ b/src/panfrost/midgard/midgard_opt_perspective.c
> > @@ -71,7 +71,7 @@ midgard_opt_combine_projection(compiler_context *ctx, midgard_block *block)
> >                  unsigned frcp_component = 0;
> >                  unsigned frcp_from = 0;
> >  
> > -                mir_foreach_instr_in_block_safe(block, sub) {
> > +                mir_foreach_instr_in_block(block, sub) {  
> 
> Should this one be _safe?

I don't see any remove_ins()/insert_before() call in there. Do you see
any reason for using the _safe() variant here?