agx: Remove logical_end instructions
authorAlyssa Rosenzweig <alyssa@rosenzweig.io>
Tue, 29 Aug 2023 18:40:25 +0000 (14:40 -0400)
committerMarge Bot <emma+marge@anholt.net>
Tue, 5 Sep 2023 18:50:34 +0000 (18:50 +0000)
They're more trouble than they're worth for us. They were originally lifted
unthinkingly from ACO, where I assume they're necessary for software CF
lowering, but they're just an inconvenient convenience for us. Remove em.

Signed-off-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/25052>

src/asahi/compiler/agx_compile.c
src/asahi/compiler/agx_compiler.h
src/asahi/compiler/agx_opcodes.py
src/asahi/compiler/agx_opt_empty_else.c
src/asahi/compiler/agx_pack.c
src/asahi/compiler/agx_pressure_schedule.c
src/asahi/compiler/agx_validate.c

index f0807d6..1bec5f1 100644 (file)
@@ -1719,20 +1719,6 @@ agx_emit_tex(agx_builder *b, nir_tex_instr *instr)
 }
 
 /*
- * Mark the logical end of the current block by emitting a p_logical_end marker.
- * Note if an unconditional jump is emitted (for instance, to break out of a
- * loop from inside an if), the block has already reached its logical end so we
- * don't re-emit p_logical_end. The validator checks this, and correct register
- * allocation depends on it.
- */
-static void
-agx_emit_logical_end(agx_builder *b)
-{
-   if (!b->shader->current_block->unconditional_jumps)
-      agx_logical_end(b);
-}
-
-/*
  * NIR loops are treated as a pair of AGX loops:
  *
  *    do {
@@ -1768,9 +1754,6 @@ agx_emit_jump(agx_builder *b, nir_jump_instr *instr)
 
    /* Update the counter and flush */
    agx_nest(b, nestings);
-
-   /* Jumps must come at the end of a block */
-   agx_emit_logical_end(b);
    agx_pop_exec(b, 0);
 
    ctx->current_block->unconditional_jumps = true;
@@ -1925,7 +1908,6 @@ emit_if(agx_context *ctx, nir_if *nif)
    agx_builder _b = agx_init_builder(ctx, agx_after_block(first_block));
    agx_index cond = agx_src_index(&nif->condition);
 
-   agx_emit_logical_end(&_b);
    agx_if_icmp(&_b, cond, agx_zero(), 1, AGX_ICOND_UEQ, true);
    ctx->loop_nesting++;
 
@@ -1934,7 +1916,6 @@ emit_if(agx_context *ctx, nir_if *nif)
    agx_block *end_then = ctx->current_block;
 
    _b.cursor = agx_after_block(ctx->current_block);
-   agx_emit_logical_end(&_b);
 
    agx_block *else_block = emit_cf_list(ctx, &nif->else_list);
    agx_block *end_else = ctx->current_block;
@@ -1953,7 +1934,6 @@ emit_if(agx_context *ctx, nir_if *nif)
    agx_block_add_successor(end_else, ctx->after_block);
 
    _b.cursor = agx_after_block(ctx->current_block);
-   agx_emit_logical_end(&_b);
    agx_pop_exec(&_b, 1);
    ctx->loop_nesting--;
 }
@@ -1974,7 +1954,6 @@ emit_loop(agx_context *ctx, nir_loop *nloop)
 
    /* Make room for break/continue nesting (TODO: skip if no divergent CF) */
    agx_builder _b = agx_init_builder(ctx, agx_after_block(ctx->current_block));
-   agx_emit_logical_end(&_b);
    agx_push_exec(&_b, 2);
 
    /* Fallthrough to body */
@@ -1988,7 +1967,6 @@ emit_loop(agx_context *ctx, nir_loop *nloop)
    /* Fix up the nesting counter via an always true while_icmp, and branch back
     * to start of loop if any lanes are active */
    _b.cursor = agx_after_block(ctx->current_block);
-   agx_emit_logical_end(&_b);
    agx_while_icmp(&_b, agx_zero(), agx_zero(), 2, AGX_ICOND_UEQ, false);
    agx_jmp_exec_any(&_b, start_block);
    agx_pop_exec(&_b, 2);
@@ -2526,7 +2504,6 @@ agx_compile_function_nir(nir_shader *nir, nir_function_impl *impl,
     */
    agx_block *last_block = list_last_entry(&ctx->blocks, agx_block, link);
    agx_builder _b = agx_init_builder(ctx, agx_after_block(last_block));
-   agx_logical_end(&_b);
    agx_stop(&_b);
 
    /* Index blocks now that we're done emitting so the order is consistent */
index 3246819..eca198d 100644 (file)
@@ -670,24 +670,6 @@ agx_after_instr(agx_instr *instr)
    };
 }
 
-/*
- * Get a cursor inserting at the logical end of the block. In particular, this
- * is before branches or control flow instructions, which occur after the
- * logical end but before the physical end.
- */
-static inline agx_cursor
-agx_after_block_logical(agx_block *block)
-{
-   /* Search for a p_logical_end */
-   agx_foreach_instr_in_block_rev(block, I) {
-      if (I->op == AGX_OPCODE_LOGICAL_END)
-         return agx_before_instr(I);
-   }
-
-   /* If there's no p_logical_end, use the physical end */
-   return agx_after_block(block);
-}
-
 static inline agx_cursor
 agx_before_nonempty_block(agx_block *block)
 {
@@ -706,6 +688,42 @@ agx_before_block(agx_block *block)
       return agx_before_nonempty_block(block);
 }
 
+static inline bool
+instr_after_logical_end(const agx_instr *I)
+{
+   switch (I->op) {
+   case AGX_OPCODE_JMP_EXEC_ANY:
+   case AGX_OPCODE_JMP_EXEC_NONE:
+   case AGX_OPCODE_POP_EXEC:
+   case AGX_OPCODE_IF_ICMP:
+   case AGX_OPCODE_WHILE_ICMP:
+   case AGX_OPCODE_IF_FCMP:
+   case AGX_OPCODE_WHILE_FCMP:
+   case AGX_OPCODE_STOP:
+      return true;
+   default:
+      return false;
+   }
+}
+
+/*
+ * Get a cursor inserting at the logical end of the block. In particular, this
+ * is before branches or control flow instructions, which occur after the
+ * logical end but before the physical end.
+ */
+static inline agx_cursor
+agx_after_block_logical(agx_block *block)
+{
+   /* Search for the first instruction that's not past the logical end */
+   agx_foreach_instr_in_block_rev(block, I) {
+      if (!instr_after_logical_end(I))
+         return agx_after_instr(I);
+   }
+
+   /* If we got here, the block is either empty or entirely control flow */
+   return agx_before_block(block);
+}
+
 /* IR builder in terms of cursor infrastructure */
 
 typedef struct {
index 6c81d2c..71a9a45 100644 (file)
@@ -410,10 +410,6 @@ op("xor", _, srcs = 2)
 op("and", _, srcs = 2)
 op("or", _, srcs = 2)
 
-# Indicates the logical end of the block, before final branches/control flow
-op("logical_end", _, dests = 0, srcs = 0, can_eliminate = False,
-   schedule_class = "invalid")
-
 op("collect", _, srcs = VARIABLE)
 op("split", _, srcs = 1, dests = VARIABLE)
 op("phi", _, srcs = VARIABLE, schedule_class = "preload")
index eb22651..653782b 100644 (file)
@@ -12,7 +12,6 @@
  * Detect blocks with the sole contents:
  *
  *    else n=1
- *    logical_end
  *    pop_exec n=1
  *
  * The else instruction is a no-op. To see that, consider the pseudocode for the
@@ -50,7 +49,6 @@
 
 enum block_state {
    STATE_ELSE = 0,
-   STATE_LOGICAL_END,
    STATE_POP_EXEC,
    STATE_DONE,
 
@@ -66,9 +64,6 @@ state_for_instr(const agx_instr *I)
    case AGX_OPCODE_ELSE_FCMP:
       return (I->nest == 1) ? STATE_ELSE : STATE_NONE;
 
-   case AGX_OPCODE_LOGICAL_END:
-      return STATE_LOGICAL_END;
-
    case AGX_OPCODE_POP_EXEC:
       return (I->nest == 1) ? STATE_POP_EXEC : STATE_NONE;
 
index 68c1f9e..6cf796a 100644 (file)
@@ -985,15 +985,6 @@ agx_pack_binary(agx_context *ctx, struct util_dynarray *emission)
    util_dynarray_init(&fixups, ctx);
 
    agx_foreach_block(ctx, block) {
-      agx_foreach_instr_in_block_rev(block, I) {
-         if (I->op == AGX_OPCODE_LOGICAL_END) {
-            agx_remove_instruction(I);
-            break;
-         }
-      }
-   }
-
-   agx_foreach_block(ctx, block) {
       /* Relative to the start of the binary, the block begins at the current
        * number of bytes emitted */
       block->offset = emission->size;
index 4ae25f1..78e0b44 100644 (file)
@@ -59,7 +59,7 @@ create_dag(agx_context *ctx, agx_block *block, void *memctx)
 
    agx_foreach_instr_in_block(block, I) {
       /* Don't touch control flow */
-      if (I->op == AGX_OPCODE_LOGICAL_END)
+      if (instr_after_logical_end(I))
          break;
 
       struct sched_node *node = rzalloc(memctx, struct sched_node);
index a19a794..5acd950 100644 (file)
 
 /*
  * If a block contains phi nodes, they must come at the start of the block. If a
- * block contains control flow, it must come after a p_logical_end marker.
+ * block contains control flow, it must come at the beginning/end as applicable.
  * Therefore the form of a valid block is:
  *
  *       Control flow instructions (else)
  *       Phi nodes
  *       General instructions
- *       Logical end
  *       Control flow instructions (except else)
  *
  * Validate that this form is satisfied.
@@ -58,24 +57,12 @@ agx_validate_block_form(agx_block *block)
          break;
 
       default:
-         agx_validate_assert(state != AGX_BLOCK_STATE_CF);
-         state = AGX_BLOCK_STATE_BODY;
-         break;
-
-      case AGX_OPCODE_LOGICAL_END:
-         agx_validate_assert(state != AGX_BLOCK_STATE_CF);
-         state = AGX_BLOCK_STATE_CF;
-         break;
-
-      case AGX_OPCODE_JMP_EXEC_ANY:
-      case AGX_OPCODE_JMP_EXEC_NONE:
-      case AGX_OPCODE_POP_EXEC:
-      case AGX_OPCODE_IF_ICMP:
-      case AGX_OPCODE_WHILE_ICMP:
-      case AGX_OPCODE_IF_FCMP:
-      case AGX_OPCODE_WHILE_FCMP:
-      case AGX_OPCODE_STOP:
-         agx_validate_assert(state == AGX_BLOCK_STATE_CF);
+         if (instr_after_logical_end(I)) {
+            state = AGX_BLOCK_STATE_CF;
+         } else {
+            agx_validate_assert(state != AGX_BLOCK_STATE_CF);
+            state = AGX_BLOCK_STATE_BODY;
+         }
          break;
       }
    }