[Mesa-dev] [PATCH v7 02/35] nv50/ir/ra: Fix copying compound for moves

[Karol Herbst]

From: Connor Abbott <cwabbott0 at gmail.com>

In order to reduce moves when coalescing multiple registers into a
larger register, RA will try to coalesce MERGE instructions with their
definitions. For example, for something like this in GLSL:

uint a = ...;
uint b = ...;
uint64 x = packUint2x32(a, b);

The compiler will try to coalesce x with a and b, in the same way as
something like:

uint a = ...;
uint b = ...;
...
uint x = phi(a, b);

with the crucial difference that the definitions of a and b only clobber
part of the register, instead of the whole thing. This information is
carried through the compound flag and compMask bitmask. If compound is
set, then the value has been coalesced in such a way that not all the
defs clobber the entire register. The compMask bitmask describes which
subregister each def clobbers, although it does it in a slightly
convoluted way. It's an invariant that once compound is set on one def,
it must be set for all the defs in a given coalesced value.

In more detail, the constraints pass will first create extra moves:

uint a = ...;
uint b = ...;
uint a' = a;
uint b' = b;
uint64 x = packUint2x32(a', b');

and then RA will merge values involved in MERGE/SPLIT instructions,
merging x with a' and b' and making the combined value compound -- this
is relatively simple, and will always succeed since we just created a'
and b', so they never interfere with x, and x has no other definitions,
since we haven't started coalescing moves yet. Basically, we just replaced
the MERGE instruction with an equivalent sequence of partial writes to the
destination. The tricky part comes when we try to merge a' with a
and b' with b. We need to transfer the compound information from a' to a
and b' to b, which copyCompound() does, but we also need to transfer it
to any defs coalesced with a and b, which the code failed to do. Similarly,
if x is the argument to a phi instruction, then when we try to merge it
with other arguments to the same phi by coalescing moves, we'd have
problems guaranteeing that all the other merged defs stay up-to-date.

One tricky part of fixing this is that in order to properly propagate
the information from a' to a, we need to do it before the defs for a and
a' are merged in coalesceValues(), since we need to know which defs are
merged with a but not a' -- after coalesceValues() returns, all the defs
have been combined, so we don't know which is which. I took the approach
of calling copyCompound() inside coalesceValues(), instead of
afterwards.

Cc: Ilia Mirkin <imirkin at alum.mit.edu>
Cc: Karol Herbst <kherbst at redhat.com>
Tested-by: Karol Herbst <kherbst at redhat.com>
Signed-off-by: Karol Herbst <kherbst at redhat.com>
---
 src/gallium/drivers/nouveau/codegen/nv50_ir_ra.cpp | 60 ++++++++++++++--------
 1 file changed, 39 insertions(+), 21 deletions(-)

diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_ra.cpp b/src/gallium/drivers/nouveau/codegen/nv50_ir_ra.cpp
index 3a0e56e1385..df3116a6d73 100644
--- a/src/gallium/drivers/nouveau/codegen/nv50_ir_ra.cpp
+++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_ra.cpp
@@ -890,6 +890,35 @@ GCRA::RIG_Node::init(const RegisterSet& regs, LValue *lval)
    livei.insert(lval->livei);
 }
 
+// Used when coalescing moves. The non-compound value will become one, e.g.:
+// mov b32 $r0 $r2            / merge b64 $r0d { $r0 $r1 }
+// split b64 { $r0 $r1 } $r0d / mov b64 $r0d f64 $r2d
+static inline void copyCompound(Value *dst, Value *src)
+{
+   LValue *ldst = dst->asLValue();
+   LValue *lsrc = src->asLValue();
+
+   if (ldst->compound && !lsrc->compound) {
+      LValue *swap = lsrc;
+      lsrc = ldst;
+      ldst = swap;
+   }
+
+   assert(!ldst->compound);
+
+   if (lsrc->compound) {
+      Value *dstRep = ldst->join;
+      for (Value::DefIterator d = dstRep->defs.begin(); d != dstRep->defs.end();
+           ++d) {
+         LValue *ldst = (*d)->get()->asLValue();
+         if (!ldst->compound)
+            ldst->compMask = 0xff;
+         ldst->compound = 1;
+         ldst->compMask &= lsrc->compMask;
+      }
+   }
+}
+
 bool
 GCRA::coalesceValues(Value *dst, Value *src, bool force)
 {
@@ -932,9 +961,16 @@ GCRA::coalesceValues(Value *dst, Value *src, bool force)
    if (!force && nRep->livei.overlaps(nVal->livei))
       return false;
 
+   // TODO: Handle this case properly.
+   if (!force && rep->compound && val->compound)
+      return false;
+
    INFO_DBG(prog->dbgFlags, REG_ALLOC, "joining %%%i($%i) <- %%%i\n",
             rep->id, rep->reg.data.id, val->id);
 
+   if (!force)
+      copyCompound(dst, src);
+
    // set join pointer of all values joined with val
    for (Value::DefIterator def = val->defs.begin(); def != val->defs.end();
         ++def)
@@ -997,24 +1033,6 @@ static inline uint8_t makeCompMask(int compSize, int base, int size)
    }
 }
 
-// Used when coalescing moves. The non-compound value will become one, e.g.:
-// mov b32 $r0 $r2            / merge b64 $r0d { $r0 $r1 }
-// split b64 { $r0 $r1 } $r0d / mov b64 $r0d f64 $r2d
-static inline void copyCompound(Value *dst, Value *src)
-{
-   LValue *ldst = dst->asLValue();
-   LValue *lsrc = src->asLValue();
-
-   if (ldst->compound && !lsrc->compound) {
-      LValue *swap = lsrc;
-      lsrc = ldst;
-      ldst = swap;
-   }
-
-   ldst->compound = lsrc->compound;
-   ldst->compMask = lsrc->compMask;
-}
-
 void
 GCRA::makeCompound(Instruction *insn, bool split)
 {
@@ -1100,8 +1118,7 @@ GCRA::doCoalesce(ArrayList& insns, unsigned int mask)
             break;
          i = insn->getSrc(0)->getUniqueInsn();
          if (i && !i->constrainedDefs()) {
-            if (coalesceValues(insn->getDef(0), insn->getSrc(0), false))
-               copyCompound(insn->getSrc(0), insn->getDef(0));
+            coalesceValues(insn->getDef(0), insn->getSrc(0), false);
          }
          break;
       case OP_TEX:
@@ -2337,7 +2354,8 @@ RegAlloc::InsertConstraintsPass::insertConstraintMoves()
                defi->src(0).getFile() == FILE_MEMORY_CONST &&
                !defi->src(0).isIndirect(0);
             // catch some cases where don't really need MOVs
-            if (cst->getSrc(s)->refCount() == 1 && !defi->constrainedDefs()) {
+            if (cst->getSrc(s)->refCount() == 1 && !defi->constrainedDefs()
+                && defi->op != OP_MERGE && defi->op != OP_SPLIT) {
                if (imm || load) {
                   // Move the defi right before the cst. No point in expanding
                   // the range.
-- 
2.14.3