[Beignet] [PATCH] GBE: Fix a bug in phicopy coaleasing.

[Yang, Rong R]

This opt is independent of the original PhiCopy opt, maybe can use a separate phiCopyDef loop for clear.
Anyway, I push it first. Could refine it later.

> -----Original Message-----
> From: Beignet [mailto:beignet-bounces at lists.freedesktop.org] On Behalf Of
> Ruiling Song
> Sent: Thursday, July 16, 2015 16:41
> To: beignet at lists.freedesktop.org
> Cc: Song, Ruiling
> Subject: [Beignet] [PATCH] GBE: Fix a bug in phicopy coaleasing.
> 
> we cannot simply 'break', the isOpt is not set correctly when break.
> let's use if() check which will not interfere with the old isOpt logic.
> 
> Signed-off-by: Ruiling Song <ruiling.song at intel.com>
> ---
>  backend/src/llvm/llvm_gen_backend.cpp | 89 +++++++++++++++++---------
> ---------
>  1 file changed, 44 insertions(+), 45 deletions(-)
> 
> diff --git a/backend/src/llvm/llvm_gen_backend.cpp
> b/backend/src/llvm/llvm_gen_backend.cpp
> index 0ec113d..1e141cd 100644
> --- a/backend/src/llvm/llvm_gen_backend.cpp
> +++ b/backend/src/llvm/llvm_gen_backend.cpp
> @@ -2191,55 +2191,54 @@ namespace gbe
>          const ir::Register phiCopySrc = phiCopyDefInsn->getSrc(0);
>          const ir::UseSet *phiCopySrcUse = dag->getRegUse(phiCopySrc);
>          const ir::DefSet *phiCopySrcDef = dag->getRegDef(phiCopySrc);
> -        // non-ssa value. skip opt on such kind of value
> -        if (phiCopySrcDef->size() > 1) break;
> -        // we can only coalesce instruction dest
> -        if ((*(phiCopySrcDef->begin()))->getType() != ValueDef::DEF_INSN_DST)
> break;
> -
> -        const ir::Instruction *phiCopySrcDefInsn = (*(phiCopySrcDef->begin()))-
> >getInstruction();
> -        if(bb == phiDefBB && bb == phiCopySrcDefInsn->getParent()) {
> -          // phiCopy, phiCopySrc defined in same basicblock as phi
> -          // try to coalease phiCopy and phiCopySrc first.
> -          // consider below situation:
> -          // bb1:
> -          //    ...
> -          // bb2:
> -          //    x = phi [x1, bb1], [x2, bb2]
> -          //    x2 = x+1;
> -          // after de-ssa:
> -          // bb2:
> -          //    mov x, x-copy
> -          //    add x2, x, 1
> -          //    mov x-copy, x2
> -          //  obviously x2, x-copy and x2 can be mapped to same virtual register
> -
> -          ir::BasicBlock::const_iterator iter =
> ir::BasicBlock::const_iterator(phiCopySrcDefInsn);
> -          ir::BasicBlock::const_iterator iterE = bb->end();
> -          // check no use of phi in this basicblock between [phiCopySrc def, bb
> end]
> -          bool phiPhiCopySrcInterfere = false;
> -          while (iter != iterE) {
> -            const ir::Instruction *insn = iter.node();
> -            // check phiUse
> -            for (unsigned i = 0; i < insn->getSrcNum(); i++) {
> -              ir::Register src = insn->getSrc(i);
> -              if (src == phi) {
> -                phiPhiCopySrcInterfere = true; break;
> +
> +        // we should only do coaleasing on instruction-def and ssa-value
> +        if (phiCopySrcDef->size() == 1 && (*(phiCopySrcDef->begin()))-
> >getType() == ValueDef::DEF_INSN_DST) {
> +          const ir::Instruction *phiCopySrcDefInsn = (*(phiCopySrcDef-
> >begin()))->getInstruction();
> +          if(bb == phiDefBB && bb == phiCopySrcDefInsn->getParent()) {
> +            // phiCopy, phiCopySrc defined in same basicblock as phi
> +            // try to coalease phiCopy and phiCopySrc first.
> +            // consider below situation:
> +            // bb1:
> +            //    ...
> +            // bb2:
> +            //    x = phi [x1, bb1], [x2, bb2]
> +            //    x2 = x+1;
> +            // after de-ssa:
> +            // bb2:
> +            //    mov x, x-copy
> +            //    add x2, x, 1
> +            //    mov x-copy, x2
> +            //  obviously x2, x-copy and x2 can be mapped to same virtual register
> +
> +            ir::BasicBlock::const_iterator iter =
> ir::BasicBlock::const_iterator(phiCopySrcDefInsn);
> +            ir::BasicBlock::const_iterator iterE = bb->end();
> +            // check no use of phi in this basicblock between [phiCopySrc def, bb
> end]
> +            bool phiPhiCopySrcInterfere = false;
> +            while (iter != iterE) {
> +              const ir::Instruction *insn = iter.node();
> +              // check phiUse
> +              for (unsigned i = 0; i < insn->getSrcNum(); i++) {
> +                ir::Register src = insn->getSrc(i);
> +                if (src == phi) {
> +                  phiPhiCopySrcInterfere = true; break;
> +                }
>                }
> +              ++iter;
>              }
> -            ++iter;
> -          }
> -          if (!phiPhiCopySrcInterfere) {
> -            // phiCopy source can be coaleased with phiCopy
> -            const_cast<Instruction *>(phiCopyDefInsn)->remove();
> +            if (!phiPhiCopySrcInterfere) {
> +              // phiCopy source can be coaleased with phiCopy
> +              const_cast<Instruction *>(phiCopyDefInsn)->remove();
> 
> -            for (auto &s : *phiCopySrcDef) {
> -              const Instruction *phiSrcDefInsn = s->getInstruction();
> -              replaceDst(const_cast<Instruction *>(phiSrcDefInsn), phiCopySrc,
> phiCopy);
> -            }
> +              for (auto &s : *phiCopySrcDef) {
> +                const Instruction *phiSrcDefInsn = s->getInstruction();
> +                replaceDst(const_cast<Instruction *>(phiSrcDefInsn), phiCopySrc,
> phiCopy);
> +              }
> 
> -            for (auto &s : *phiCopySrcUse) {
> -              const Instruction *phiSrcUseInsn = s->getInstruction();
> -              replaceSrc(const_cast<Instruction *>(phiSrcUseInsn), phiCopySrc,
> phiCopy);
> +              for (auto &s : *phiCopySrcUse) {
> +                const Instruction *phiSrcUseInsn = s->getInstruction();
> +                replaceSrc(const_cast<Instruction *>(phiSrcUseInsn), phiCopySrc,
> phiCopy);
> +              }
>              }
>            }
>          }
> --
> 2.3.6
> 
> _______________________________________________
> Beignet mailing list
> Beignet at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet