LGTM, it will solve the overlap register problem. -----Original Message----- From: Beignet [mailto:[email protected]] On Behalf Of Yang Rong Sent: Friday, December 30, 2016 12:39 PM To: [email protected] Cc: Yang, Rong R <[email protected]> Subject: [Beignet] [Patch V2] GBE: fix a src/dst register reuse bug.
From: "Yang, Rong R" <[email protected]> For case: mad(8) g4<1>:F g4.1<0,1,0>:F g127.7<0,1,0>:F g46:F mad(8) g5<1>:F g4.1<0,1,0>:F g127.7<0,1,0>:F g47:F src0 is uniform, dst is non-uniform, dst can't reuse the src0. V2: check the all region field. Signed-off-by: Yang Rong <[email protected]> --- backend/src/backend/gen_insn_selection.cpp | 13 +++++++++++++ backend/src/backend/gen_insn_selection.hpp | 2 ++ backend/src/backend/gen_reg_allocation.cpp | 4 +++- backend/src/backend/gen_register.hpp | 8 ++++++++ 4 files changed, 26 insertions(+), 1 deletion(-) diff --git a/backend/src/backend/gen_insn_selection.cpp b/backend/src/backend/gen_insn_selection.cpp index dd21960..53a9d97 100644 --- a/backend/src/backend/gen_insn_selection.cpp +++ b/backend/src/backend/gen_insn_selection.cpp @@ -234,6 +234,19 @@ namespace gbe return this->opcode == SEL_OP_LABEL; } + bool SelectionInstruction::sameAsDstRegion(uint32_t srcID) { + assert(srcID < srcNum); + if (dstNum == 0) + return true; + GenRegister &srcReg = this->src(srcID); + for (uint32_t dstID = 0; dstID < dstNum; ++dstID) { + const GenRegister &dstReg = this->dst(dstID); + if (!dstReg.isSameRegion(srcReg)) + return false; + } + return true; + } + bool SelectionInstruction::isNative(void) const { return this->opcode == SEL_OP_NOT || /* ALU1 */ this->opcode == SEL_OP_LZD || diff --git a/backend/src/backend/gen_insn_selection.hpp b/backend/src/backend/gen_insn_selection.hpp index 01999a2..89d28f2 100644 --- a/backend/src/backend/gen_insn_selection.hpp +++ b/backend/src/backend/gen_insn_selection.hpp @@ -82,6 +82,8 @@ namespace gbe bool isBranch(void) const; /*! Is it a label instruction (i.e. change the implicit mask) */ bool isLabel(void) const; + /*! Is the src's gen register region is same as all dest regs' region */ + bool sameAsDstRegion(uint32_t srcID); /*! Is it a simple navtive instruction (i.e. will be one simple ISA) */ bool isNative(void) const; /*! Get the destination register */ diff --git a/backend/src/backend/gen_reg_allocation.cpp b/backend/src/backend/gen_reg_allocation.cpp index 2b76eee..6dd570d 100644 --- a/backend/src/backend/gen_reg_allocation.cpp +++ b/backend/src/backend/gen_reg_allocation.cpp @@ -1267,7 +1267,9 @@ namespace gbe this->intervals[reg].conflictReg = conflictReg; int insnsrcID = insnID; // If instruction is simple, src and dst can be reused and they will have different IDs - if (insn.isNative()) + // insn may be split in the encoder, if register region are not same, can't be reused. + // Because hard to check split or not here, so only check register regio. + if (insn.isNative() && insn.sameAsDstRegion(srcID)) insnsrcID -= 1; this->intervals[reg].minID = std::min(this->intervals[reg].minID, insnsrcID); this->intervals[reg].maxID = std::max(this->intervals[reg].maxID, insnsrcID); diff --git a/backend/src/backend/gen_register.hpp b/backend/src/backend/gen_register.hpp index d9798cf..6c73f5e 100644 --- a/backend/src/backend/gen_register.hpp +++ b/backend/src/backend/gen_register.hpp @@ -293,6 +293,14 @@ namespace gbe return r; } + INLINE bool isSameRegion(GenRegister reg) const { + return reg.file == file && + typeSize(reg.type) == typeSize(type) && + reg.vstride == vstride && + reg.width == width && + reg.hstride == hstride; + } + static INLINE uint32_t grfOffset(GenRegister reg) { return reg.nr * GEN_REG_SIZE + reg.subnr; } -- 2.1.4 _______________________________________________ Beignet mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/beignet _______________________________________________ Beignet mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/beignet
