[Beignet] [PATCH] GBE: fix regression caused by simple block optimization.

[Gong, Zhigang]

Thanks for your comment.
Actually, no need to add such a comment, as it is not special case.
We haven't add all the other instructions into this list because all of them haven't use CMP/SEL/... in it.
Unaligned store is just one of those instructions, and you can check the gen_insn_selection.cpp, there
is even no deadicated unaligned store function.


> -----Original Message-----
> From: Luo, Xionghu
> Sent: Thursday, October 23, 2014 3:58 PM
> To: Gong, Zhigang; beignet at lists.freedesktop.org
> Cc: Gong, Zhigang
> Subject: RE: [Beignet] [PATCH] GBE: fix regression caused by simple block
> optimization.
> 
> LGTM.
> Add comments for why unaligned store instruction not excluded before push,
> please.
> Thanks.
> 
> Luo Xionghu
> Best Regards
> 
> -----Original Message-----
> From: Beignet [mailto:beignet-bounces at lists.freedesktop.org] On Behalf Of
> Zhigang Gong
> Sent: Thursday, October 23, 2014 2:26 PM
> To: beignet at lists.freedesktop.org
> Cc: Gong, Zhigang
> Subject: [Beignet] [PATCH] GBE: fix regression caused by simple block
> optimization.
> 
> Almost all 64bit related instructions and unaligned load instruction should be
> complex instruction. We need to exclude them from simple block.
> 
> Signed-off-by: Zhigang Gong <zhigang.gong at intel.com>
> ---
>  backend/src/backend/gen_insn_selection.cpp | 28
> ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
> 
> diff --git a/backend/src/backend/gen_insn_selection.cpp
> b/backend/src/backend/gen_insn_selection.cpp
> index 1a37ef1..605fdd5 100644
> --- a/backend/src/backend/gen_insn_selection.cpp
> +++ b/backend/src/backend/gen_insn_selection.cpp
> @@ -407,6 +407,8 @@ namespace gbe
>      void matchBasicBlock(const ir::BasicBlock &bb, uint32_t insnNum);
>      /*! a simple block can use predication instead of if/endif*/
>      bool isSimpleBlock(const ir::BasicBlock &bb, uint32_t insnNum);
> +    /*! an instruction has a QWORD family src or dst operand. */
> +    bool hasQWord(const ir::Instruction &insn);
>      /*! A root instruction needs to be generated */
>      bool isRoot(const ir::Instruction &insn) const;
> 
> @@ -1494,6 +1496,20 @@ namespace gbe
>      return false;
>    }
> 
> +  bool Selection::Opaque::hasQWord(const ir::Instruction &insn) {
> +    for (uint32_t i = 0; i < insn.getSrcNum(); i++) {
> +      const ir::Register reg = insn.getSrc(i);
> +      if (getRegisterFamily(reg) == ir::FAMILY_QWORD)
> +        return true;
> +    }
> +    for (uint32_t i = 0; i < insn.getDstNum(); i++) {
> +      const ir::Register reg = insn.getDst(i);
> +      if (getRegisterFamily(reg) == ir::FAMILY_QWORD)
> +        return true;
> +    }
> +    return false;
> +  }
> +
>    bool Selection::Opaque::isSimpleBlock(const ir::BasicBlock &bb, uint32_t
> insnNum) {
> 
>      // FIXME should include structured innermost if/else/endif @@ -1511,6
> +1527,18 @@ namespace gbe
>           insn.getOpcode() == ir::OP_SIMD_ALL ||
>           insn.getOpcode() == ir::OP_ELSE)
>          return false;
> +
> +      // Most of the QWord(long) related instruction introduce some CMP or
> +      // more than 10 actual instructions at latter stage.
> +      if (hasQWord(insn))
> +        return false;
> +
> +      // Unaligned load may introduce CMP instruction.
> +      if ( insn.isMemberOf<ir::LoadInstruction>()) {
> +        const ir::LoadInstruction &ld = ir::cast<ir::LoadInstruction>(insn);
> +        if (!ld.isAligned())
> +          return false;
> +      }
>      }
> 
>      // there would generate a extra CMP instruction for predicated BRA with
> extern flag,
> --
> 1.8.3.2
> 
> _______________________________________________
> Beignet mailing list
> Beignet at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet