On 04/08/2017 02:49 PM, Boyan Ding wrote:
2017-04-08 20:42 GMT+08:00 Samuel Pitoiset <samuel.pitoi...@gmail.com>:


On 04/08/2017 11:51 AM, Boyan Ding wrote:

---
   .../drivers/nouveau/codegen/nv50_ir_emit_gm107.cpp | 23
++++++++++++++++++----
   1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_gm107.cpp
b/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_gm107.cpp
index 6903132efa..4a741bf45b 100644
--- a/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_gm107.cpp
+++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_gm107.cpp
@@ -966,11 +966,26 @@ CodeEmitterGM107::emitSHFL()
         break;
      }
   -   /*XXX: what is this arg? hardcode immediate for now */
-   emitField(0x22, 13, 0x1c03);
-   type |= 2;
+   switch (insn->src(2).getFile()) {
+   case FILE_GPR:
+      emitGPR(0x27, insn->src(2));
+      break;
+   case FILE_IMMEDIATE:
+      emitIMMD(0x22, 13, insn->src(2));
+      type |= 2;
+      break;
+   default:
+      assert(!"invalid src2 file");
+      break;
+   }


This looks wrong to me. I think you need to check that src(2) exists because
you might hit the assert for DFDX (and friends) which uses SHFL.

I wasn't aware that this was used in gm107's lowering. That explains
the 0x1c03 magic number here.

Do you think I shall leave 0x1c03 as default value or add the constant
in nv50_ir_lowering_gm107.cpp?

Yeah, you probably need to emit src(2) there and it would be nice to figure out the magic number. :)


Thanks for pointing it out.

Cheers,
Boyan Ding



+
+   if (!insn->defExists(1))
+      emitPRED(0x30);
+   else {
+      assert(insn->def(1).getFile() == FILE_PREDICATE);
+      emitPRED(0x30, insn->def(1));
+   }
   -   emitPRED (0x30);
      emitField(0x1e, 2, insn->subOp);
      emitField(0x1c, 2, type);
      emitGPR  (0x08, insn->src(0));


_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to