[Beignet] [PATCH] GBE: fix an ACC register related instruction scheduling bug

[Yang, Rong R]

LGTM, thanks.

> -----Original Message-----
> From: Beignet [mailto:beignet-bounces at lists.freedesktop.org] On Behalf Of
> Zhigang Gong
> Sent: Tuesday, January 20, 2015 14:41
> To: beignet at lists.freedesktop.org
> 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 <zhigang.gong at intel.com>
> ---
>  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
> Beignet at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet