vc4: Update copy propagation for control flow.

Previously, we could assume that a MOV from a temp was always an available
copy, because all temps were SSA in NIR, and their non-SSA state in QIR
was just due to the fact that they were from a bcsel or pack_unorm_4x8, so
we could use the current value of the temp after that series of QIR
instructions to define it.

However, this is no longer the case with control flow.  Instead, we track
a new array of MOVs defined within the block that haven't had their source
or dest killed yet, and use that primarily.  We fall back to looking
through the QIR defs array to handle across-block MOVs, but now require
that copies from the SSA defs have an SSA src as well.

diff --git a/src/gallium/drivers/vc4/vc4_opt_copy_propagation.c b/src/gallium/drivers/vc4/vc4_opt_copy_propagation.c
index a180f040b54c92a30b0ef248404ca5280836658f..f8f1365f6589b5d26de24956670e9c075574083a 100644 (file)
--- a/src/gallium/drivers/vc4/vc4_opt_copy_propagation.c
+++ b/src/gallium/drivers/vc4/vc4_opt_copy_propagation.c
@@ -34,85 +34,160 @@
 
 #include "vc4_qir.h"
 
-bool
-qir_opt_copy_propagation(struct vc4_compile *c)
+static bool
+is_copy_mov(struct qinst *inst)
+{
+        if (!inst)
+                return false;
+
+        if (inst->op != QOP_MOV &&
+            inst->op != QOP_FMOV &&
+            inst->op != QOP_MMOV) {
+                return false;
+        }
+
+        if (inst->dst.file != QFILE_TEMP)
+                return false;
+
+        if (inst->src[0].file != QFILE_TEMP &&
+            inst->src[0].file != QFILE_UNIF) {
+                return false;
+        }
+
+        if (inst->dst.pack || inst->cond != QPU_COND_ALWAYS)
+                return false;
+
+        return true;
+
+}
+
+static bool
+try_copy_prop(struct vc4_compile *c, struct qinst *inst, struct qinst **movs)
 {
-        bool progress = false;
         bool debug = false;
+        bool progress = false;
+
+       for (int i = 0; i < qir_get_op_nsrc(inst->op); i++) {
+                if (inst->src[i].file != QFILE_TEMP)
+                        continue;
 
-        qir_for_each_inst_inorder(inst, c) {
-                int nsrc = qir_get_op_nsrc(inst->op);
-                for (int i = 0; i < nsrc; i++) {
-                        if (inst->src[i].file != QFILE_TEMP)
+                /* We have two ways of finding MOVs we can copy propagate
+                 * from.  One is if it's an SSA def: then we can reuse it from
+                 * any block in the program, as long as its source is also an
+                 * SSA def.  Alternatively, if it's in the "movs" array
+                 * tracked within the block, then we know the sources for it
+                 * haven't been changed since we saw the instruction within
+                 * our block.
+                 */
+                struct qinst *mov = movs[inst->src[i].index];
+                if (!mov) {
+                        if (!is_copy_mov(c->defs[inst->src[i].index]))
                                 continue;
+                        mov = c->defs[inst->src[i].index];
 
-                        struct qinst *mov = c->defs[inst->src[i].index];
-                        if (!mov ||
-                            (mov->op != QOP_MOV &&
-                             mov->op != QOP_FMOV &&
-                             mov->op != QOP_MMOV)) {
+                        if (mov->src[0].file == QFILE_TEMP &&
+                            !c->defs[mov->src[0].index])
                                 continue;
-                        }
+                }
 
-                        if (mov->src[0].file != QFILE_TEMP &&
-                            mov->src[0].file != QFILE_UNIF) {
+                uint8_t unpack;
+                if (mov->src[0].pack) {
+                        /* Make sure that the meaning of the unpack
+                         * would be the same between the two
+                         * instructions.
+                         */
+                        if (qir_is_float_input(inst) !=
+                            qir_is_float_input(mov)) {
                                 continue;
                         }
 
-                        if (mov->dst.pack)
+                        /* There's only one unpack field, so make sure
+                         * this instruction doesn't already use it.
+                         */
+                        bool already_has_unpack = false;
+                        for (int j = 0; j < qir_get_op_nsrc(inst->op); j++) {
+                                if (inst->src[j].pack)
+                                        already_has_unpack = true;
+                        }
+                        if (already_has_unpack)
                                 continue;
 
-                        uint8_t unpack;
-                        if (mov->src[0].pack) {
-                                /* Make sure that the meaning of the unpack
-                                 * would be the same between the two
-                                 * instructions.
-                                 */
-                                if (qir_is_float_input(inst) !=
-                                    qir_is_float_input(mov)) {
-                                        continue;
-                                }
-
-                                /* There's only one unpack field, so make sure
-                                 * this instruction doesn't already use it.
-                                 */
-                                bool already_has_unpack = false;
-                                for (int j = 0; j < nsrc; j++) {
-                                        if (inst->src[j].pack)
-                                                already_has_unpack = true;
-                                }
-                                if (already_has_unpack)
-                                        continue;
-
-                                /* A destination pack requires the PM bit to
-                                 * be set to a specific value already, which
-                                 * may be different from ours.
-                                 */
-                                if (inst->dst.pack)
-                                        continue;
-
-                                unpack = mov->src[0].pack;
-                        } else {
-                                unpack = inst->src[i].pack;
-                        }
+                        /* A destination pack requires the PM bit to
+                         * be set to a specific value already, which
+                         * may be different from ours.
+                         */
+                        if (inst->dst.pack)
+                                continue;
 
-                        if (debug) {
-                                fprintf(stderr, "Copy propagate: ");
-                                qir_dump_inst(c, inst);
-                                fprintf(stderr, "\n");
-                        }
+                        unpack = mov->src[0].pack;
+                } else {
+                        unpack = inst->src[i].pack;
+                }
 
-                        inst->src[i] = mov->src[0];
-                        inst->src[i].pack = unpack;
+                if (debug) {
+                        fprintf(stderr, "Copy propagate: ");
+                        qir_dump_inst(c, inst);
+                        fprintf(stderr, "\n");
+                }
 
-                        if (debug) {
-                                fprintf(stderr, "to: ");
-                                qir_dump_inst(c, inst);
-                                fprintf(stderr, "\n");
-                        }
+                inst->src[i] = mov->src[0];
+                inst->src[i].pack = unpack;
 
-                        progress = true;
+                if (debug) {
+                        fprintf(stderr, "to: ");
+                        qir_dump_inst(c, inst);
+                        fprintf(stderr, "\n");
+                }
+
+                progress = true;
+        }
+
+        return progress;
+}
+
+static void
+apply_kills(struct vc4_compile *c, struct qinst **movs, struct qinst *inst)
+{
+        if (inst->dst.file != QFILE_TEMP)
+                return;
+
+        for (int i = 0; i < c->num_temps; i++) {
+                if (movs[i] &&
+                    (movs[i]->dst.index == inst->dst.index ||
+                     (movs[i]->src[0].file == QFILE_TEMP &&
+                      movs[i]->src[0].index == inst->dst.index))) {
+                        movs[i] = NULL;
+                }
+        }
+}
+
+bool
+qir_opt_copy_propagation(struct vc4_compile *c)
+{
+        bool progress = false;
+        struct qinst **movs;
+
+        movs = ralloc_array(c, struct qinst *, c->num_temps);
+        if (!movs)
+                return false;
+
+        qir_for_each_block(block, c) {
+                /* The MOVs array tracks only available movs within the
+                 * block.
+                 */
+                memset(movs, 0, sizeof(struct qinst *) * c->num_temps);
+
+                qir_for_each_inst(inst, block) {
+                        progress = try_copy_prop(c, inst, movs) || progress;
+
+                        apply_kills(c, movs, inst);
+
+                        if (is_copy_mov(inst))
+                                movs[inst->dst.index] = inst;
                 }
         }
+
+        ralloc_free(movs);
+
         return progress;
 }