[Beignet] [PATCH] fix opencv_test_imgproc subcase OCL_ImgProc/Accumulate.Mask regression.

[Zhigang Gong]

Xiong hu,

Good job. Some comments as below, The 

On Tue, Aug 26, 2014 at 05:36:21AM +0800, xionghu.luo at intel.com wrote:
> From: Luo Xionghu <xionghu.luo at intel.com>
> 
> This regression is caused by structural analysis when check the if-then
> node, acturally there are four types of if-then node according to the
> topology and fallthrough information. fallthrough check is added in this
> patch.
> 
> Signed-off-by: Luo Xionghu <xionghu.luo at intel.com>
> ---
>  backend/src/backend/gen_insn_selection.cpp |    4 +++-
>  backend/src/ir/function.hpp                |    5 +++++
>  backend/src/ir/structural_analysis.cpp     |    9 ++++++++-
>  backend/src/ir/structural_analysis.hpp     |   16 +++++++++++++++-
>  4 files changed, 31 insertions(+), 3 deletions(-)
> 
> diff --git a/backend/src/backend/gen_insn_selection.cpp b/backend/src/backend/gen_insn_selection.cpp
> index b7a39af..9a552b1 100644
> --- a/backend/src/backend/gen_insn_selection.cpp
> +++ b/backend/src/backend/gen_insn_selection.cpp
> @@ -4018,7 +4018,9 @@ namespace gbe
>            sel.curr.physicalFlag = 0;
>            sel.curr.flagIndex = (uint64_t)pred;
>            sel.curr.externFlag = 1;
> -          sel.curr.inversePredicate = 1;
> +          if(insn.getParent()->need_reverse ){
> +            sel.curr.inversePredicate = 1;
> +          }
  It's clearer to use sel.curr.inversePredicate = insn.getParent->need_reverse;
>            sel.curr.predicate = GEN_PREDICATE_NORMAL;
>            sel.IF(GenRegister::immd(0), jip, uip);
>            sel.curr.inversePredicate = 0;
> diff --git a/backend/src/ir/function.hpp b/backend/src/ir/function.hpp
> index c5582b4..b877bce 100644
> --- a/backend/src/ir/function.hpp
> +++ b/backend/src/ir/function.hpp
> @@ -87,6 +87,11 @@ namespace ir {
>      set <Register> definedPhiRegs;
>    /* these three are used by structure transforming */
>    public:
> +    /*if need_reverse is true,  need to reverse prediction.
> +     *if condition is TRUE, IF instruction will execute the following block,
        ~~~ should be * if, you missed one space between * and if
> +     * different from BRA instruction, so all the IF instruction need_reverse
> +     * except two special case(fallthrough is the same with succs.). */
> +    bool need_reverse;
>      /* if needEndif is true, it means that this bb is the exit of an
>       * outermost structure, so this block needs another endif to match
>       * the if inserted at the entry of this structure, otherwise this
> diff --git a/backend/src/ir/structural_analysis.cpp b/backend/src/ir/structural_analysis.cpp
> index dfc2118..c106fa7 100644
> --- a/backend/src/ir/structural_analysis.cpp
> +++ b/backend/src/ir/structural_analysis.cpp
> @@ -120,6 +120,7 @@ namespace analysis
>      /* since this node is an if node, so we remove the BRA instruction at the bottom of the exit BB of 'node',
>       * and insert IF instead
>       */
> +    pbb->need_reverse = node->need_reverse;
It's better to add a new member to the IF(BRANCH) instruction to indicate this is a IF or an "inverse IF".
Add a inversePredicate member to the branch instruction which is only valid for IF opcode and add a new method
getInversePred() to that instruction and use that method function at the backend stage to check whether this is
a IF NOT or IF Then. It's should be an instruction attribute rather than a basic block's. And please use
inversePredicate and need_inverse rather than need_reverse to keep consistent with the backend's naming rule.

Thanks
Zhigang Gong