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>
This commit is contained in:
Alyssa Rosenzweig 2023-07-29 20:08:43 -04:00 committed by Marge Bot
parent 5196558204
commit 41b7891673
4 changed files with 29 additions and 20 deletions

View file

@ -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);

View file

@ -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

View file

@ -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.

View file

@ -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);