LGTM, thanks.
> -----Original Message----- > From: Beignet [mailto:[email protected]] On Behalf Of > Zhigang Gong > Sent: Tuesday, January 20, 2015 14:41 > To: [email protected] > Cc: Gong, Zhigang > Subject: [Beignet] [PATCH] GBE: fix an ACC register related instruction > scheduling bug > > Some instructions modify the ACC register in the gen_context stage which's > not regonized by current instruction scheduling algorithm. This patch fix this > bug by checking all the possible SEL_OPs which may change the ACC implicitly. > > The corresponding bugzilla link is as below: > https://bugs.freedesktop.org/show_bug.cgi?id=88587 > > Signed-off-by: Zhigang Gong <[email protected]> > --- > backend/src/backend/gen_insn_scheduling.cpp | 4 ++-- > backend/src/backend/gen_insn_selection.cpp | 14 ++++++++++++++ > backend/src/backend/gen_insn_selection.hpp | 2 ++ > 3 files changed, 18 insertions(+), 2 deletions(-) > > diff --git a/backend/src/backend/gen_insn_scheduling.cpp > b/backend/src/backend/gen_insn_scheduling.cpp > index 5538a74..f849c52 100644 > --- a/backend/src/backend/gen_insn_scheduling.cpp > +++ b/backend/src/backend/gen_insn_scheduling.cpp > @@ -379,7 +379,7 @@ namespace gbe > } > > // Track writes in accumulators > - if (insn.state.accWrEnable) { > + if (insn.modAcc()) { > const uint32_t index = this->getIndex(GenRegister::acc()); > this->nodes[index] = node; > } > @@ -517,7 +517,7 @@ namespace gbe > tracker.addDependency(node, getFlag(insn), WRITE_AFTER_WRITE); > > // write-after-write for accumulators > - if (insn.state.accWrEnable) > + if (insn.modAcc()) > tracker.addDependency(node, GenRegister::acc(), > WRITE_AFTER_WRITE); > > // write-after-write in memory > diff --git a/backend/src/backend/gen_insn_selection.cpp > b/backend/src/backend/gen_insn_selection.cpp > index f83edf5..bf33386 100644 > --- a/backend/src/backend/gen_insn_selection.cpp > +++ b/backend/src/backend/gen_insn_selection.cpp > @@ -189,6 +189,20 @@ namespace gbe > this->opcode == SEL_OP_DWORD_GATHER; > } > > + bool SelectionInstruction::modAcc(void) const { > + return this->opcode == SEL_OP_I64SUB || > + this->opcode == SEL_OP_I64ADD || > + this->opcode == SEL_OP_MUL_HI || > + this->opcode == SEL_OP_HADD || > + this->opcode == SEL_OP_RHADD || > + this->opcode == SEL_OP_I64MUL || > + this->opcode == SEL_OP_I64_MUL_HI || > + this->opcode == SEL_OP_I64MADSAT || > + this->opcode == SEL_OP_I64DIV || > + this->opcode == SEL_OP_I64REM || > + this->opcode == SEL_OP_MACH; } > + > bool SelectionInstruction::isWrite(void) const { > return this->opcode == SEL_OP_UNTYPED_WRITE || > this->opcode == SEL_OP_WRITE64 || > diff --git a/backend/src/backend/gen_insn_selection.hpp > b/backend/src/backend/gen_insn_selection.hpp > index 7fef11f..8bffb16 100644 > --- a/backend/src/backend/gen_insn_selection.hpp > +++ b/backend/src/backend/gen_insn_selection.hpp > @@ -77,6 +77,8 @@ namespace gbe > bool isRead(void) const; > /*! Does it write memory? */ > bool isWrite(void) const; > + /*! Does it modify the acc register. */ > + bool modAcc(void) const; > /*! Is it a branch instruction (i.e. modify control flow) */ > bool isBranch(void) const; > /*! Is it a label instruction (i.e. change the implicit mask) */ > -- > 1.8.3.2 > > _______________________________________________ > Beignet mailing list > [email protected] > http://lists.freedesktop.org/mailman/listinfo/beignet _______________________________________________ Beignet mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/beignet
