[Beignet] [Patch V2 8/8] SKL: fix skl LD fail.

Zhigang Gong zhigang.gong at linux.intel.com
Thu Jan 29 19:38:17 PST 2015


Thanks for the patchset and review comments.

I just pushed the whole patchset.

On Fri, Jan 30, 2015 at 12:04:38PM +0800, He Junyan wrote:
> Except some format problem, this patchset LGTM and can pass all the
> utest cases on my platform.
> 
> 
> On 2015年01月30日 10:59, Yang Rong wrote:
> >Skl's LD message payload order is changed from u, lod, v, w to u, v, lod, w.
> >Add the Gen9Context and Selection9 to handle it.
> >Skl Still use Gen8Encoder.
> >
> >Signed-off-by: Yang Rong <rong.r.yang at intel.com>
> >---
> >  backend/src/CMakeLists.txt                 |  2 +
> >  backend/src/backend/gen9_context.cpp       | 31 ++++++++++++++
> >  backend/src/backend/gen9_context.hpp       | 50 ++++++++++++++++++++++
> >  backend/src/backend/gen_insn_selection.cpp | 67 ++++++++++++++++++++++++------
> >  backend/src/backend/gen_insn_selection.hpp |  7 ++++
> >  backend/src/backend/gen_program.cpp        |  3 +-
> >  6 files changed, 147 insertions(+), 13 deletions(-)
> >  create mode 100644 backend/src/backend/gen9_context.cpp
> >  create mode 100644 backend/src/backend/gen9_context.hpp
> >
> >diff --git a/backend/src/CMakeLists.txt b/backend/src/CMakeLists.txt
> >index ce83c62..9518888 100644
> >--- a/backend/src/CMakeLists.txt
> >+++ b/backend/src/CMakeLists.txt
> >@@ -103,6 +103,8 @@ set (GBE_SRC
> >      backend/gen75_context.cpp
> >      backend/gen8_context.hpp
> >      backend/gen8_context.cpp
> >+    backend/gen9_context.hpp
> >+    backend/gen9_context.cpp
> >      backend/gen_program.cpp
> >      backend/gen_program.hpp
> >      backend/gen_program.h
> >diff --git a/backend/src/backend/gen9_context.cpp b/backend/src/backend/gen9_context.cpp
> >new file mode 100644
> >index 0000000..79ca275
> >--- /dev/null
> >+++ b/backend/src/backend/gen9_context.cpp
> >@@ -0,0 +1,31 @@
> >+/*
> >+ * Copyright © 2012 Intel Corporation
> >+ *
> >+ * This library is free software; you can redistribute it and/or
> >+ * modify it under the terms of the GNU Lesser General Public
> >+ * License as published by the Free Software Foundation; either
> >+ * version 2.1 of the License, or (at your option) any later version.
> >+ *
> >+ * This library is distributed in the hope that it will be useful,
> >+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> >+ * Lesser General Public License for more details.
> >+ *
> >+ * You should have received a copy of the GNU Lesser General Public
> >+ * License along with this library. If not, see <http://www.gnu.org/licenses/>.
> >+ *
> >+ */
> >+
> >+/**
> >+ * \file gen9_context.cpp
> >+ */
> >+
> >+#include "backend/gen9_context.hpp"
> >+#include "backend/gen_insn_selection.hpp"
> >+
> >+namespace gbe
> >+{
> >+  void Gen9Context::newSelection(void) {
> >+    this->sel = GBE_NEW(Selection9, *this);
> >+  }
> >+}
> >diff --git a/backend/src/backend/gen9_context.hpp b/backend/src/backend/gen9_context.hpp
> >new file mode 100644
> >index 0000000..672b4fc
> >--- /dev/null
> >+++ b/backend/src/backend/gen9_context.hpp
> >@@ -0,0 +1,50 @@
> >+/*
> >+ * Copyright © 2012 Intel Corporation
> >+ *
> >+ * This library is free software; you can redistribute it and/or
> >+ * modify it under the terms of the GNU Lesser General Public
> >+ * License as published by the Free Software Foundation; either
> >+ * version 2.1 of the License, or (at your option) any later version.
> >+ *
> >+ * This library is distributed in the hope that it will be useful,
> >+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> >+ * Lesser General Public License for more details.
> >+ *
> >+ * You should have received a copy of the GNU Lesser General Public
> >+ * License along with this library. If not, see <http://www.gnu.org/licenses/>.
> >+ *
> >+ */
> >+
> >+/**
> >+ * \file gen9_context.hpp
> >+ */
> >+#ifndef __GBE_gen9_CONTEXT_HPP__
> >+#define __GBE_gen9_CONTEXT_HPP__
> >+
> >+#include "backend/gen8_context.hpp"
> >+#include "backend/gen8_encoder.hpp"
> >+
> >+namespace gbe
> >+{
> >+  /* This class is used to implement the HSW
> >+     specific logic for context. */
> >+  class Gen9Context : public Gen8Context
> >+  {
> >+  public:
> >+    virtual ~Gen9Context(void) { };
> >+    Gen9Context(const ir::Unit &unit, const std::string &name, uint32_t deviceID, bool relaxMath = false)
> >+            : Gen8Context(unit, name, deviceID, relaxMath) {
> >+    };
> >+		
> >+	protected:
> >+		virtual GenEncoder* generateEncoder(void) {
> >+			return GBE_NEW(Gen8Encoder, this->simdWidth, 9, deviceID);
> >+		}
> >+
> >+  private:
> >+    virtual void newSelection(void);
> >+  };
> >+}
> >+#endif /* __GBE_GEN9_CONTEXT_HPP__ */
> >+
> >diff --git a/backend/src/backend/gen_insn_selection.cpp b/backend/src/backend/gen_insn_selection.cpp
> >index 65842ff..4d0b979 100644
> >--- a/backend/src/backend/gen_insn_selection.cpp
> >+++ b/backend/src/backend/gen_insn_selection.cpp
> >@@ -249,6 +249,9 @@ namespace gbe
> >      this->vectorList.push_back(vec);
> >    }
> >+#define LD_MSG_ORDER_IVB 7
> >+#define LD_MSG_ORDER_SKL 9
> >+
> >    ///////////////////////////////////////////////////////////////////////////
> >    // Maximal munch selection on DAG
> >    ///////////////////////////////////////////////////////////////////////////
> >@@ -358,6 +361,8 @@ namespace gbe
> >      void setHas32X32Mul(bool b) { bHas32X32Mul = b; }
> >      bool hasLongType() const { return bHasLongType; }
> >      void setHasLongType(bool b) { bHasLongType = b; }
> >+    void setLdMsgOrder(uint32_t type)  { ldMsgOrder = type; }
> >+    uint32_t getLdMsgOrder()  const { return ldMsgOrder; }
> >      /*! indicate whether a register is a scalar/uniform register. */
> >      INLINE bool isScalarReg(const ir::Register &reg) const {
> >        const ir::RegisterData &regData = getRegisterData(reg);
> >@@ -656,6 +661,7 @@ namespace gbe
> >      uint16_t currAuxLabel;
> >      bool bHas32X32Mul;
> >      bool bHasLongType;
> >+    uint32_t ldMsgOrder;
> >      INLINE ir::LabelIndex newAuxLabel()
> >      {
> >        currAuxLabel++;
> >@@ -695,7 +701,7 @@ namespace gbe
> >      curr(ctx.getSimdWidth()), file(ctx.getFunction().getRegisterFile()),
> >      maxInsnNum(ctx.getFunction().getLargestBlockSize()), dagPool(maxInsnNum),
> >      stateNum(0), vectorNum(0), bwdCodeGeneration(false), currAuxLabel(ctx.getFunction().labelNum()),
> >-    bHas32X32Mul(false), bHasLongType(false)
> >+    bHas32X32Mul(false), bHasLongType(false), ldMsgOrder(LD_MSG_ORDER_IVB)
> >    {
> >      const ir::Function &fn = ctx.getFunction();
> >      this->regNum = fn.regNum();
> >@@ -1853,6 +1859,12 @@ namespace gbe
> >      this->opaque->setHasLongType(true);
> >    }
> >+  Selection9::Selection9(GenContext &ctx) : Selection(ctx) {
> >+    this->opaque->setHas32X32Mul(true);
> >+    this->opaque->setHasLongType(true);
> >+    this->opaque->setLdMsgOrder(LD_MSG_ORDER_SKL);
> >+  }
> >+
> >    void Selection::Opaque::TYPED_WRITE(GenRegister *msgs, uint32_t msgNum,
> >                                        uint32_t bti, bool is3D) {
> >      uint32_t elemID = 0;
> >@@ -4299,6 +4311,44 @@ namespace gbe
> >    DECL_PATTERN(SampleInstruction)
> >    {
> >+    INLINE void emitLd_ivb(Selection::Opaque &sel, const ir::SampleInstruction &insn,
> >+                           GenRegister msgPayloads[4], uint32_t &msgLen) const
> >+    {
> >+      // pre SKL: U, lod, [V], [W]
> >+      using namespace ir;
> >+      GBE_ASSERT(insn.getSrcType() != TYPE_FLOAT);
> >+      uint32_t srcNum = insn.getSrcNum();
> >+      msgPayloads[0] = sel.selReg(insn.getSrc(0), insn.getSrcType());
> >+      msgPayloads[1] = sel.selReg(sel.reg(FAMILY_DWORD), TYPE_U32);
> >+      sel.MOV(msgPayloads[1], GenRegister::immud(0));
> >+      if (srcNum > 1)
> >+        msgPayloads[2] = sel.selReg(insn.getSrc(1), insn.getSrcType());
> >+      if (srcNum > 2)
> >+        msgPayloads[3] = sel.selReg(insn.getSrc(2), insn.getSrcType());
> >+      // Clear the lod to zero.
> >+      msgLen = srcNum + 1;
> >+    }
> >+
> >+    INLINE void emitLd_skl(Selection::Opaque &sel, const ir::SampleInstruction &insn,
> >+                           GenRegister msgPayloads[4], uint32_t &msgLen) const
> >+    {
> >+      // SKL: U, [V], [lod], [W]
> >+      using namespace ir;
> >+      GBE_ASSERT(insn.getSrcType() != TYPE_FLOAT);
> >+      uint32_t srcNum = msgLen = insn.getSrcNum();
> >+      msgPayloads[0] = sel.selReg(insn.getSrc(0), insn.getSrcType());
> >+      if (srcNum > 1)
> >+        msgPayloads[1] = sel.selReg(insn.getSrc(1), insn.getSrcType());
> >+      if (srcNum > 2) {
> >+        // Clear the lod to zero.
> >+        msgPayloads[2] = sel.selReg(sel.reg(FAMILY_DWORD), TYPE_U32);
> >+        sel.MOV(msgPayloads[2], GenRegister::immud(0));
> >+        msgLen += 1;
> >+
> >+        msgPayloads[3] = sel.selReg(insn.getSrc(2), insn.getSrcType());
> >+      }
> >+    }
> >+
> >      INLINE bool emitOne(Selection::Opaque &sel, const ir::SampleInstruction &insn, bool &markChildren) const
> >      {
> >        using namespace ir;
> >@@ -4317,17 +4367,10 @@ namespace gbe
> >          srcNum = 2;
> >        if (insn.getSamplerOffset() != 0) {
> >-        // U, lod, [V], [W]
> >-        GBE_ASSERT(insn.getSrcType() != TYPE_FLOAT);
> >-        msgPayloads[0] = sel.selReg(insn.getSrc(0), insn.getSrcType());
> >-        msgPayloads[1] = sel.selReg(sel.reg(FAMILY_DWORD), TYPE_U32);
> >-        if (srcNum > 1)
> >-          msgPayloads[2] = sel.selReg(insn.getSrc(1), insn.getSrcType());
> >-        if (srcNum > 2)
> >-          msgPayloads[3] = sel.selReg(insn.getSrc(2), insn.getSrcType());
> >-        // Clear the lod to zero.
> >-        sel.MOV(msgPayloads[1], GenRegister::immud(0));
> >-        msgLen = srcNum + 1;
> >+        if(sel.getLdMsgOrder() < LD_MSG_ORDER_SKL)
> >+          this->emitLd_ivb(sel, insn, msgPayloads, msgLen);
> >+        else
> >+          this->emitLd_skl(sel, insn, msgPayloads, msgLen);
> >        } else {
> >          // U, V, [W]
> >          GBE_ASSERT(insn.getSrcType() == TYPE_FLOAT);
> >diff --git a/backend/src/backend/gen_insn_selection.hpp b/backend/src/backend/gen_insn_selection.hpp
> >index 8bffb16..6a08180 100644
> >--- a/backend/src/backend/gen_insn_selection.hpp
> >+++ b/backend/src/backend/gen_insn_selection.hpp
> >@@ -294,6 +294,13 @@ namespace gbe
> >        Selection8(GenContext &ctx);
> >    };
> >+  class Selection9: public Selection
> >+  {
> >+    public:
> >+      /*! Initialize internal structures used for the selection */
> >+      Selection9(GenContext &ctx);
> >+  };
> >+
> >  } /* namespace gbe */
> >  #endif /*  __GEN_INSN_SELECTION_HPP__ */
> >diff --git a/backend/src/backend/gen_program.cpp b/backend/src/backend/gen_program.cpp
> >index 01e7ee9..4cfb703 100644
> >--- a/backend/src/backend/gen_program.cpp
> >+++ b/backend/src/backend/gen_program.cpp
> >@@ -54,6 +54,7 @@
> >  #include "backend/gen_context.hpp"
> >  #include "backend/gen75_context.hpp"
> >  #include "backend/gen8_context.hpp"
> >+#include "backend/gen9_context.hpp"
> >  #include "backend/gen_defs.hpp"
> >  #include "backend/gen/gen_mesa_disasm.h"
> >  #include "backend/gen_reg_allocation.hpp"
> >@@ -171,7 +172,7 @@ namespace gbe {
> >      } else if (IS_BROADWELL(deviceID)) {
> >        ctx = GBE_NEW(Gen8Context, unit, name, deviceID, relaxMath);
> >      } else if (IS_SKYLAKE(deviceID)) {
> >-      ctx = GBE_NEW(Gen8Context, unit, name, deviceID, relaxMath);
> >+      ctx = GBE_NEW(Gen9Context, unit, name, deviceID, relaxMath);
> >      }
> >      GBE_ASSERTM(ctx != NULL, "Fail to create the gen context\n");
> 
> 
> 
> _______________________________________________
> Beignet mailing list
> Beignet at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet


More information about the Beignet mailing list