[Beignet] [PATCH] GBE: don't allocate/modify flag if it is not used in current BB.

[Song, Ruiling]

The patch looks good to me.

-----Original Message-----
From: Beignet [mailto:beignet-bounces at lists.freedesktop.org] On Behalf Of Zhigang Gong
Sent: Friday, May 23, 2014 9:19 AM
To: Gong, Zhigang
Cc: beignet at lists.freedesktop.org
Subject: Re: [Beignet] [PATCH] GBE: don't allocate/modify flag if it is not used in current BB.

Ping for review.

On Tue, May 13, 2014 at 05:56:01PM +0800, Zhigang Gong wrote:
> If a flag is not used in current BB, we don't need to set the modFlag 
> bit on that instruction. Thus the register allocation stage will not 
> allocate a flag register for it.
> 
> No performance impact, as the previous implementation will expire that 
> flag register immediately.
> 
> Signed-off-by: Zhigang Gong <zhigang.gong at intel.com>
> ---
>  backend/src/backend/gen_insn_selection.cpp | 15 +++++++++++----
>  1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/backend/src/backend/gen_insn_selection.cpp 
> b/backend/src/backend/gen_insn_selection.cpp
> index 0cb633f..417354f 100644
> --- a/backend/src/backend/gen_insn_selection.cpp
> +++ b/backend/src/backend/gen_insn_selection.cpp
> @@ -228,6 +228,7 @@ namespace gbe
>        for (uint32_t childID = 0; childID < childNum; ++childID)
>          this->child[childID] = NULL;
>        computeBool = false;
> +      isUsed = false;
>      }
>      /*! Mergeable are non-root instructions with valid sources */
>      INLINE void setAsMergeable(uint32_t which) { 
> mergeable|=(1<<which); } @@ -245,6 +246,8 @@ namespace gbe
>      uint32_t isRoot:1;
>      /*! A bool register is used as normal computing sources. */
>      bool computeBool;
> +    /*! is used in this block */
> +    bool isUsed;
>    };
>  
>    /*! A pattern is a tree to match. This is the general interface for 
> them. For @@ -1453,6 +1456,7 @@ namespace gbe
>                     (insn.getOpcode() == OP_SEL && srcID != 0)))
>                child->computeBool = true;
>            }
> +          child->isUsed = true;
>          } else
>            dag->child[srcID] = NULL;
>        }
> @@ -1747,8 +1751,10 @@ namespace gbe
>                sel.MOV_DF(dst, src, sel.selReg(r));
>              } else {
>                sel.push();
> -                if (sel.getRegisterFamily(insn.getDst(0)) == ir::FAMILY_BOOL) {
> -                  sel.curr.physicalFlag = 0;
> +                auto dag = sel.regDAG[insn.getDst(0)];
> +                if (sel.getRegisterFamily(insn.getDst(0)) == ir::FAMILY_BOOL &&
> +                    dag->isUsed) {
> +                sel.curr.physicalFlag = 0;
>                  sel.curr.flagIndex = (uint16_t)(insn.getDst(0));
>                  sel.curr.modFlag = 1;
>                }
> @@ -1976,7 +1982,8 @@ namespace gbe
>        }
>  
>        // Output the binary instruction
> -      if (sel.getRegisterFamily(insn.getDst(0)) == ir::FAMILY_BOOL) {
> +      if (sel.getRegisterFamily(insn.getDst(0)) == ir::FAMILY_BOOL &&
> +          dag.isUsed) {
>          GBE_ASSERT(insn.getOpcode() == OP_AND ||
>                     insn.getOpcode() == OP_OR ||
>                     insn.getOpcode() == OP_XOR); @@ -2513,7 +2520,7 @@ 
> namespace gbe
>  
>        switch (type) {
>          case TYPE_BOOL:
> -          if (!sel.isScalarReg(insn.getDst(0))) {
> +          if (!sel.isScalarReg(insn.getDst(0)) && 
> + sel.regDAG[insn.getDst(0)]->isUsed) {
>              sel.curr.modFlag = 1;
>              sel.curr.physicalFlag = 0;
>              sel.curr.flagIndex = (uint16_t) insn.getDst(0);
> --
> 1.8.3.2
> 
> _______________________________________________
> Beignet mailing list
> Beignet at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet
_______________________________________________
Beignet mailing list
Beignet at lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/beignet