lima/ppir: rework select conditions
authorErico Nunes <nunes.erico@gmail.com>
Tue, 14 Apr 2020 00:54:56 +0000 (02:54 +0200)
committerErico Nunes <nunes.erico@gmail.com>
Sat, 9 May 2020 12:40:40 +0000 (14:40 +0200)
This is yet another simple optimization that attemts to save the
insertion of an unnecessary mov for a large number of cases.
If the node outputting the condition for select satisfies a few
requirements (which are common in the case of comparison conditions),
it can just be changed to pipeline output and used directly.
In case of difficult corner cases, just fall back to the mov as before.

The sel_cond op is removed as the scheduler can be smart enough to place
nodes that output to ^fmul in the ALU_SCL_MUL slot, and as there can be
alu ops other than just mov.

Signed-off-by: Erico Nunes <nunes.erico@gmail.com>
Reviewed-by: Vasily Khoruzhick <anarsoul@gmail.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4632>

src/gallium/drivers/lima/ir/pp/codegen.c
src/gallium/drivers/lima/ir/pp/instr.c
src/gallium/drivers/lima/ir/pp/lower.c
src/gallium/drivers/lima/ir/pp/node.c
src/gallium/drivers/lima/ir/pp/ppir.h

index 4420ee30e05931774e6ee8f5a78f177ff643dc3d..55d3489c51c9c29d0a426421c307a2b83bb9fe67 100644 (file)
@@ -276,9 +276,6 @@ static void ppir_codegen_encode_scl_mul(ppir_node *node, void *code)
    case ppir_op_mov:
       f->op = ppir_codegen_float_mul_op_mov;
       break;
-   case ppir_op_sel_cond:
-      f->op = ppir_codegen_float_mul_op_mov;
-      break;
    case ppir_op_max:
       f->op = ppir_codegen_float_mul_op_max;
       break;
index 74b6e5fbaea6761dd70a8eb99d2bcab42884c3c6..8e1bc95158d64afdb46364263f2b7c308527b268 100644 (file)
@@ -221,10 +221,19 @@ bool ppir_instr_insert_node(ppir_instr *instr, ppir_node *node)
                continue;
          }
 
+         /* ^fmul dests (e.g. condition for select) can only be
+          * scheduled to ALU_SCL_MUL */
+         if (pos == PPIR_INSTR_SLOT_ALU_SCL_ADD) {
+            ppir_dest *dest = ppir_node_get_dest(node);
+            if (dest && dest->type == ppir_target_pipeline &&
+                dest->pipeline == ppir_pipeline_reg_fmul)
+            continue;
+         }
+
          if (pos == PPIR_INSTR_SLOT_ALU_SCL_MUL ||
              pos == PPIR_INSTR_SLOT_ALU_SCL_ADD) {
             ppir_dest *dest = ppir_node_get_dest(node);
-            if (!ppir_target_is_scaler(dest))
+            if (!ppir_target_is_scalar(dest))
                continue;
          }
 
index b5b7c34c25f56756a9b92a168fc8cbd886c12fc3..6a088c3328d93038d21830ff0fca61e5c769d575 100644 (file)
@@ -212,23 +212,56 @@ static bool ppir_lower_texture(ppir_block *block, ppir_node *node)
    return true;
 }
 
-/* insert a move as the select condition to make sure it can
- * be inserted to select instr float mul slot
- */
+/* Check if the select condition and ensure it can be inserted to
+ * the scalar mul slot */
 static bool ppir_lower_select(ppir_block *block, ppir_node *node)
 {
    ppir_alu_node *alu = ppir_node_to_alu(node);
+   ppir_src *src0 = &alu->src[0];
+   ppir_src *src1 = &alu->src[1];
+   ppir_src *src2 = &alu->src[2];
+
+   /* If the condition is already an alu scalar whose only successor
+    * is the select node, just turn it into pipeline output. */
+   /* The (src2->node == cond) case is a tricky exception.
+    * The reason is that we must force cond to output to ^fmul -- but
+    * then it no longer writes to a register and it is impossible to
+    * reference ^fmul in src2. So in that exceptional case, also fall
+    * back to the mov. */
+   ppir_node *cond = src0->node;
+   if (cond &&
+       cond->type == ppir_node_type_alu &&
+       ppir_node_has_single_succ(cond) &&
+       ppir_target_is_scalar(ppir_node_get_dest(cond)) &&
+       ppir_node_schedulable_slot(cond, PPIR_INSTR_SLOT_ALU_SCL_MUL) &&
+       src2->node != cond) {
+
+      ppir_dest *cond_dest = ppir_node_get_dest(cond);
+      cond_dest->type = ppir_target_pipeline;
+      cond_dest->pipeline = ppir_pipeline_reg_fmul;
+
+      ppir_node_target_assign(src0, cond);
+
+      /* src1 could also be a reference from the same node as
+       * the condition, so update it in that case. */
+      if (src1->node && src1->node == cond)
+         ppir_node_target_assign(src1, cond);
 
-   ppir_node *move = ppir_node_create(block, ppir_op_sel_cond, -1, 0);
+      return true;
+   }
+
+   /* If the condition can't be used for any reason, insert a mov
+    * so that the condition can end up in ^fmul */
+   ppir_node *move = ppir_node_create(block, ppir_op_mov, -1, 0);
    if (!move)
       return false;
    list_addtail(&move->list, &node->list);
 
    ppir_alu_node *move_alu = ppir_node_to_alu(move);
-   ppir_src *move_src = move_alu->src, *src = alu->src;
-   move_src->type = src->type;
-   move_src->ssa = src->ssa;
-   move_src->swizzle[0] = src->swizzle[0];
+   ppir_src *move_src = move_alu->src;
+   move_src->type = src0->type;
+   move_src->ssa = src0->ssa;
+   move_src->swizzle[0] = src0->swizzle[0];
    move_alu->num_src = 1;
 
    ppir_dest *move_dest = &move_alu->dest;
@@ -236,7 +269,7 @@ static bool ppir_lower_select(ppir_block *block, ppir_node *node)
    move_dest->pipeline = ppir_pipeline_reg_fmul;
    move_dest->write_mask = 1;
 
-   ppir_node *pred = alu->src[0].node;
+   ppir_node *pred = src0->node;
    ppir_dep *dep = ppir_dep_for_pred(node, pred);
    if (dep)
       ppir_node_replace_pred(dep, move);
@@ -247,8 +280,12 @@ static bool ppir_lower_select(ppir_block *block, ppir_node *node)
    if (pred)
       ppir_node_add_dep(move, pred, ppir_dep_src);
 
-   src->swizzle[0] = 0;
-   ppir_node_target_assign(alu->src, move);
+   ppir_node_target_assign(src0, move);
+
+   /* src1 could also be a reference from the same node as
+    * the condition, so update it in that case. */
+   if (src1->node && src1->node == pred)
+      ppir_node_target_assign(src1, move);
 
    return true;
 }
index b71cbc3763e607834f6e54ba8a2e141f6def2a22..d0e97616f3e39f17a277773aee03ff38aaad394a 100644 (file)
@@ -225,14 +225,6 @@ const ppir_op_info ppir_op_infos[] = {
          PPIR_INSTR_SLOT_END
       },
    },
-   [ppir_op_sel_cond] = {
-      /* effectively mov, but must be scheduled only to
-       * PPIR_INSTR_SLOT_ALU_SCL_MUL */
-      .name = "sel_cond",
-      .slots = (int []) {
-         PPIR_INSTR_SLOT_ALU_SCL_MUL, PPIR_INSTR_SLOT_END
-      },
-   },
    [ppir_op_select] = {
       .name = "select",
       .slots = (int []) {
index 59887a70ec96b6acf0c14b1bb9404113a3f0bc67..821107302d71fd99ea76c2460cbf4e1c46d18013 100644 (file)
@@ -54,7 +54,6 @@ typedef enum {
    ppir_op_normalize3,
    ppir_op_normalize4,
 
-   ppir_op_sel_cond,
    ppir_op_select,
 
    ppir_op_sin,
@@ -630,7 +629,7 @@ static inline int ppir_src_get_mask(ppir_src *src)
    return mask;
 }
 
-static inline bool ppir_target_is_scaler(ppir_dest *dest)
+static inline bool ppir_target_is_scalar(ppir_dest *dest)
 {
    switch (dest->type) {
    case ppir_target_ssa:
@@ -656,6 +655,17 @@ static inline bool ppir_target_is_scaler(ppir_dest *dest)
    }
 }
 
+static inline bool ppir_node_schedulable_slot(ppir_node *node,
+                                              enum ppir_instr_slot slot)
+{
+   int *slots = ppir_op_infos[node->op].slots;
+   for (int i = 0; slots[i] != PPIR_INSTR_SLOT_END; i++)
+      if (slots[i] == slot)
+         return true;
+
+   return false;
+}
+
 ppir_instr *ppir_instr_create(ppir_block *block);
 bool ppir_instr_insert_node(ppir_instr *instr, ppir_node *node);
 void ppir_instr_add_dep(ppir_instr *succ, ppir_instr *pred);