agx: Put else instructions in the right block

According to Dougall's pseudocode, else_icmp operates as:

  if r0l == 0:
    r0l = n
  elif r0l == 1:
    if cc.compare(A[thread], B[thread]):
      r0l = 0
    else:
      r0l = 1

  exec_mask[thread] = (r0l == 0)

Notice that the comparison only happens when r0l == 1, that is, for threads that
are about to enter the else block. Threads that just executed the if body are
still active (r0l = 0) and skip the comparison. As such, the sources of
else_icmp are only read in the else block, and hence the whole instruction
should be placed in the else block for correctness with respect to live range
splitting.

shader-db is a wash, but shows some improvements due to correctly modelling the
liveness of the condition variable.

   total instructions in shared programs: 1778376 -> 1778390 (<.01%)
   instructions in affected programs: 14753 -> 14767 (0.09%)
   helped: 35
   HURT: 39
   Inconclusive result (value mean confidence interval includes 0).

   total bytes in shared programs: 12185018 -> 12185102 (<.01%)
   bytes in affected programs: 101522 -> 101606 (0.08%)
   helped: 35
   HURT: 39
   Inconclusive result (value mean confidence interval includes 0).

   total halfregs in shared programs: 531174 -> 531032 (-0.03%)
   halfregs in affected programs: 2320 -> 2178 (-6.12%)
   helped: 40
   HURT: 1
   Halfregs are helped.

   total threads in shared programs: 18909184 -> 18909440 (<.01%)
   threads in affected programs: 1792 -> 2048 (14.29%)
   helped: 2
   HURT: 0

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

diff --git a/src/asahi/compiler/agx_compile.c b/src/asahi/compiler/agx_compile.c
index 50807f9..f524060 100644 (file)
--- a/src/asahi/compiler/agx_compile.c
+++ b/src/asahi/compiler/agx_compile.c
@@ -1874,11 +1874,13 @@ emit_if(agx_context *ctx, nir_if *nif)
 
    _b.cursor = agx_after_block(ctx->current_block);
    agx_emit_logical_end(&_b);
-   agx_else_icmp(&_b, cond, agx_zero(), 1, AGX_ICOND_UEQ, false);
 
    agx_block *else_block = emit_cf_list(ctx, &nif->else_list);
    agx_block *end_else = ctx->current_block;
 
+   _b.cursor = agx_before_block(else_block);
+   agx_else_icmp(&_b, cond, agx_zero(), 1, AGX_ICOND_UEQ, false);
+
    ctx->after_block = agx_create_block(ctx);
 
    agx_block_add_successor(first_block, if_block);

diff --git a/src/asahi/compiler/agx_compiler.h b/src/asahi/compiler/agx_compiler.h
index c2c7f3d..47b0a4e 100644 (file)
--- a/src/asahi/compiler/agx_compiler.h
+++ b/src/asahi/compiler/agx_compiler.h
@@ -569,17 +569,14 @@ agx_start_block(agx_context *ctx)
    agx_foreach_dest(ins, v)                                                    \
       if (ins->dest[v].type == AGX_INDEX_NORMAL)
 
-/* Phis only come at the start so we stop as soon as we hit a non-phi */
+/* Phis only come at the start (after else instructions) so we stop as soon as
+ * we hit a non-phi
+ */
 #define agx_foreach_phi_in_block(block, v)                                     \
    agx_foreach_instr_in_block(block, v)                                        \
-      if (v->op != AGX_OPCODE_PHI)                                             \
-         break;                                                                \
-      else
-
-/* Everything else comes after, so we stop as soon as we hit a phi in reverse */
-#define agx_foreach_non_phi_in_block_rev(block, v)                             \
-   agx_foreach_instr_in_block_rev(block, v)                                    \
-      if (v->op == AGX_OPCODE_PHI)                                             \
+      if (v->op == AGX_OPCODE_ELSE_ICMP || v->op == AGX_OPCODE_ELSE_FCMP)      \
+         continue;                                                             \
+      else if (v->op != AGX_OPCODE_PHI)                                        \
          break;                                                                \
       else
 

diff --git a/src/asahi/compiler/agx_liveness.c b/src/asahi/compiler/agx_liveness.c
index 7854659..74d5899 100644 (file)
--- a/src/asahi/compiler/agx_liveness.c
+++ b/src/asahi/compiler/agx_liveness.c
@@ -66,8 +66,10 @@ agx_compute_liveness(agx_context *ctx)
       /* Update its liveness information */
       memcpy(blk->live_in, blk->live_out, words * sizeof(BITSET_WORD));
 
-      agx_foreach_non_phi_in_block_rev(blk, I)
-         agx_liveness_ins_update(blk->live_in, I);
+      agx_foreach_instr_in_block_rev(blk, I) {
+         if (I->op != AGX_OPCODE_PHI)
+            agx_liveness_ins_update(blk->live_in, I);
+      }
 
       /* Propagate the live in of the successor (blk) to the live out of
        * predecessors.

diff --git a/src/asahi/compiler/agx_validate.c b/src/asahi/compiler/agx_validate.c
index d74f29c..7cba4ea 100644 (file)
--- a/src/asahi/compiler/agx_validate.c
+++ b/src/asahi/compiler/agx_validate.c
@@ -20,10 +20,11 @@
  * block contains control flow, it must come after a p_logical_end marker.
  * Therefore the form of a valid block is:
  *
+ *       Control flow instructions (else)
  *       Phi nodes
  *       General instructions
  *       Logical end
- *       Control flow instructions
+ *       Control flow instructions (except else)
  *
  * Validate that this form is satisfied.
  *
@@ -31,20 +32,29 @@
  * that should be deferred though?
  */
 enum agx_block_state {
-   AGX_BLOCK_STATE_PHI = 0,
-   AGX_BLOCK_STATE_BODY = 1,
-   AGX_BLOCK_STATE_CF = 2
+   AGX_BLOCK_STATE_CF_ELSE = 0,
+   AGX_BLOCK_STATE_PHI = 1,
+   AGX_BLOCK_STATE_BODY = 2,
+   AGX_BLOCK_STATE_CF = 3
 };
 
 static bool
 agx_validate_block_form(agx_block *block)
 {
-   enum agx_block_state state = AGX_BLOCK_STATE_PHI;
+   enum agx_block_state state = AGX_BLOCK_STATE_CF_ELSE;
 
    agx_foreach_instr_in_block(block, I) {
       switch (I->op) {
+      case AGX_OPCODE_ELSE_ICMP:
+      case AGX_OPCODE_ELSE_FCMP:
+         agx_validate_assert(state == AGX_BLOCK_STATE_CF_ELSE);
+         break;
+
       case AGX_OPCODE_PHI:
-         agx_validate_assert(state == AGX_BLOCK_STATE_PHI);
+         agx_validate_assert(state == AGX_BLOCK_STATE_CF_ELSE ||
+                             state == AGX_BLOCK_STATE_PHI);
+
+         state = AGX_BLOCK_STATE_PHI;
          break;
 
       default:
@@ -61,10 +71,8 @@ agx_validate_block_form(agx_block *block)
       case AGX_OPCODE_JMP_EXEC_NONE:
       case AGX_OPCODE_POP_EXEC:
       case AGX_OPCODE_IF_ICMP:
-      case AGX_OPCODE_ELSE_ICMP:
       case AGX_OPCODE_WHILE_ICMP:
       case AGX_OPCODE_IF_FCMP:
-      case AGX_OPCODE_ELSE_FCMP:
       case AGX_OPCODE_WHILE_FCMP:
       case AGX_OPCODE_STOP:
          agx_validate_assert(state == AGX_BLOCK_STATE_CF);