nv50/ir: avoid asserts when the state tracker feeds us bogus inputs
authorIlia Mirkin <imirkin@alum.mit.edu>
Fri, 13 May 2016 03:42:47 +0000 (23:42 -0400)
committerIlia Mirkin <imirkin@alum.mit.edu>
Sun, 15 May 2016 18:12:56 +0000 (14:12 -0400)
INTERP is defined (by me) to have to have a INPUT source. However the
state tracker does not always obey this. This happens due to varying
packing logic introducing additional mov's which can't always be undone.
Instead of just giving up, we instead try harder to find the original
input. This won't always be possible, for example with indirect
accesses. There's not much we can (easily) do about that though.

This fixes the remaining interpolateAt* failures in dEQP:

dEQP-GLES31.functional.shaders.multisample_interpolation.interpolate_at*

some of which were asserting due to INTERP_* being passed a non-input.

Signed-off-by: Ilia Mirkin <imirkin@alum.mit.edu>
Reviewed-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp

index 69e1a341bc3622729cb1a6027ca18ad75ba590ec..c085e3818ad92dfdcd29330c44361b71d29e2afe 100644 (file)
@@ -2733,24 +2733,60 @@ Converter::handleINTERP(Value *dst[4])
    // Check whether the input is linear. All other attributes ignored.
    Instruction *insn;
    Value *offset = NULL, *ptr = NULL, *w = NULL;
+   Symbol *sym[4] = { NULL };
    bool linear;
    operation op;
    int c, mode;
 
    tgsi::Instruction::SrcRegister src = tgsi.getSrc(0);
-   assert(src.getFile() == TGSI_FILE_INPUT);
 
-   if (src.isIndirect(0))
-      ptr = fetchSrc(src.getIndirect(0), 0, NULL);
-
-   // XXX: no way to know interp mode if we don't know the index
-   linear = info->in[ptr ? 0 : src.getIndex(0)].linear;
-   if (linear) {
-      op = OP_LINTERP;
-      mode = NV50_IR_INTERP_LINEAR;
+   // In some odd cases, in large part due to varying packing, the source
+   // might not actually be an input. This is illegal TGSI, but it's easier to
+   // account for it here than it is to fix it where the TGSI is being
+   // generated. In that case, it's going to be a straight up mov (or sequence
+   // of mov's) from the input in question. We follow the mov chain to see
+   // which input we need to use.
+   if (src.getFile() != TGSI_FILE_INPUT) {
+      if (src.isIndirect(0)) {
+         ERROR("Ignoring indirect input interpolation\n");
+         return;
+      }
+      FOR_EACH_DST_ENABLED_CHANNEL(0, c, tgsi) {
+         Value *val = fetchSrc(0, c);
+         assert(val->defs.size() == 1);
+         insn = val->getInsn();
+         while (insn->op == OP_MOV) {
+            assert(insn->getSrc(0)->defs.size() == 1);
+            insn = insn->getSrc(0)->getInsn();
+            if (!insn) {
+               ERROR("Miscompiling shader due to unhandled INTERP\n");
+               return;
+            }
+         }
+         if (insn->op != OP_LINTERP && insn->op != OP_PINTERP) {
+            ERROR("Trying to interpolate non-input, this is not allowed.\n");
+            return;
+         }
+         sym[c] = insn->getSrc(0)->asSym();
+         assert(sym[c]);
+         op = insn->op;
+         mode = insn->ipa;
+      }
    } else {
-      op = OP_PINTERP;
-      mode = NV50_IR_INTERP_PERSPECTIVE;
+      if (src.isIndirect(0))
+         ptr = fetchSrc(src.getIndirect(0), 0, NULL);
+
+      // We can assume that the fixed index will point to an input of the same
+      // interpolation type in case of an indirect.
+      // TODO: Make use of ArrayID.
+      linear = info->in[src.getIndex(0)].linear;
+      if (linear) {
+         op = OP_LINTERP;
+         mode = NV50_IR_INTERP_LINEAR;
+      } else {
+         op = OP_PINTERP;
+         mode = NV50_IR_INTERP_PERSPECTIVE;
+      }
    }
 
    switch (tgsi.getOpcode()) {
@@ -2793,7 +2829,7 @@ Converter::handleINTERP(Value *dst[4])
 
 
    FOR_EACH_DST_ENABLED_CHANNEL(0, c, tgsi) {
-      insn = mkOp1(op, TYPE_F32, dst[c], srcToSym(src, c));
+      insn = mkOp1(op, TYPE_F32, dst[c], sym[c] ? sym[c] : srcToSym(src, c));
       if (op == OP_PINTERP)
          insn->setSrc(1, w);
       if (ptr)