Revert "panfrost: Rework midgard_pair_load_store() to kill the nested foreach loop"
authorBoris Brezillon <boris.brezillon@collabora.com>
Thu, 19 Sep 2019 18:59:46 +0000 (20:59 +0200)
committerBoris Brezillon <boris.brezillon@collabora.com>
Thu, 19 Sep 2019 19:01:27 +0000 (21:01 +0200)
There's a missing prev_ldst = NULL; assignment in the new logic,
but even with this fixed it seems to regress some applications,
so let's revert the change until we find the real problem.

This reverts commit c9bebae2877e55cdcd94f9f9f3f6805238caeb28.

src/panfrost/midgard/midgard_schedule.c

index feb988a66badb46eea46f7e5bf3d0176ff0ca789..b8d9b5ec9be1538cf919e4f1e191c79bae38dc4d 100644 (file)
@@ -657,41 +657,46 @@ schedule_block(compiler_context *ctx, midgard_block *block)
 static void
 midgard_pair_load_store(compiler_context *ctx, midgard_block *block)
 {
-        midgard_instruction *prev_ldst = NULL;
-        int search_distance;
-
         mir_foreach_instr_in_block_safe(block, ins) {
-                if (ins->type != TAG_LOAD_STORE_4 && !prev_ldst) continue;
+                if (ins->type != TAG_LOAD_STORE_4) continue;
 
-                /* We've found a load/store op. Start searching for another one.
-                 * Maximum search distance to pair, to avoid register pressure disasters
-                 */
-                if (!prev_ldst) {
-                        search_distance = 8;
-                        prev_ldst = ins;
-                        continue;
-                }
+                /* We've found a load/store op. Check if next is also load/store. */
+                midgard_instruction *next_op = mir_next_op(ins);
+                if (&next_op->link != &block->instructions) {
+                        if (next_op->type == TAG_LOAD_STORE_4) {
+                                /* If so, we're done since we're a pair */
+                                ins = mir_next_op(ins);
+                                continue;
+                        }
 
-                /* Already paired. */
-                if (mir_prev_op(ins) == prev_ldst) {
-                        prev_ldst = NULL;
-                        continue;
-                }
+                        /* Maximum search distance to pair, to avoid register pressure disasters */
+                        int search_distance = 8;
 
-                /* We can only reorder if there are no sources */
-                bool deps = false;
-                for (unsigned s = 0; s < ARRAY_SIZE(ins->src); ++s)
-                        deps |= (ins->src[s] != ~0);
+                        /* Otherwise, we have an orphaned load/store -- search for another load */
+                        mir_foreach_instr_in_block_from(block, c, mir_next_op(ins)) {
+                                /* Terminate search if necessary */
+                                if (!(search_distance--)) break;
 
-                /* We found one! Move it up to pair */
-                if (!deps) {
-                        list_del(&ins->link);
-                        list_add(&ins->link, &prev_ldst->link);
-                        continue;
-                }
+                                if (c->type != TAG_LOAD_STORE_4) continue;
+
+                                /* We can only reorder if there are no sources */
+
+                                bool deps = false;
+
+                                for (unsigned s = 0; s < ARRAY_SIZE(ins->src); ++s)
+                                        deps |= (c->src[s] != ~0);
+
+                                if (deps)
+                                        continue;
 
-                if (!(search_distance--))
-                        prev_ldst = NULL;
+                                /* We found one! Move it up to pair and remove it from the old location */
+
+                                mir_insert_instruction_before(ctx, ins, *c);
+                                mir_remove_instruction(c);
+
+                                break;
+                        }
+                }
         }
 }