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

Zhigang Gong zhigang.gong at linux.intel.com
Tue Aug 26 22:46:32 PDT 2014


This version LGTM, pushed, thanks.

On Tue, Aug 26, 2014 at 10:12:28AM +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.
> 
> v2: add inversePredicate member and function for BranchInstruction;
> print the exact meanning of IF instruction in GEN_IR.
> 
> Signed-off-by: Luo Xionghu <xionghu.luo at intel.com>
> ---
>  backend/src/backend/gen_insn_selection.cpp |    2 +-
>  backend/src/ir/instruction.cpp             |   12 +++++++++---
>  backend/src/ir/instruction.hpp             |    4 +++-
>  backend/src/ir/structural_analysis.cpp     |   10 ++++++++--
>  backend/src/ir/structural_analysis.hpp     |   16 +++++++++++++++-
>  5 files changed, 36 insertions(+), 8 deletions(-)
> 
> diff --git a/backend/src/backend/gen_insn_selection.cpp b/backend/src/backend/gen_insn_selection.cpp
> index b7a39af..170a9d8 100644
> --- a/backend/src/backend/gen_insn_selection.cpp
> +++ b/backend/src/backend/gen_insn_selection.cpp
> @@ -4018,7 +4018,7 @@ namespace gbe
>            sel.curr.physicalFlag = 0;
>            sel.curr.flagIndex = (uint64_t)pred;
>            sel.curr.externFlag = 1;
> -          sel.curr.inversePredicate = 1;
> +          sel.curr.inversePredicate = insn.getInversePredicated();
>            sel.curr.predicate = GEN_PREDICATE_NORMAL;
>            sel.IF(GenRegister::immd(0), jip, uip);
>            sel.curr.inversePredicate = 0;
> diff --git a/backend/src/ir/instruction.cpp b/backend/src/ir/instruction.cpp
> index bfb2000..370fb87 100644
> --- a/backend/src/ir/instruction.cpp
> +++ b/backend/src/ir/instruction.cpp
> @@ -348,13 +348,14 @@ namespace ir {
>        public NDstPolicy<BranchInstruction, 0>
>      {
>      public:
> -      INLINE BranchInstruction(Opcode op, LabelIndex labelIndex, Register predicate) {
> +      INLINE BranchInstruction(Opcode op, LabelIndex labelIndex, Register predicate, bool inv_pred=false) {
>          GBE_ASSERT(op == OP_BRA || op == OP_IF);
>          this->opcode = op;
>          this->predicate = predicate;
>          this->labelIndex = labelIndex;
>          this->hasPredicate = true;
>          this->hasLabel = true;
> +        this->inversePredicate = inv_pred;
>        }
>        INLINE BranchInstruction(Opcode op, LabelIndex labelIndex) {
>          GBE_ASSERT(op == OP_BRA || op == OP_ELSE || op == OP_ENDIF);
> @@ -385,11 +386,13 @@ namespace ir {
>          predicate = reg;
>        }
>        INLINE bool isPredicated(void) const { return hasPredicate; }
> +      INLINE bool getInversePredicated(void) const { return inversePredicate; }
>        INLINE bool wellFormed(const Function &fn, std::string &why) const;
>        INLINE void out(std::ostream &out, const Function &fn) const;
>        Register predicate;    //!< Predication means conditional branch
>        LabelIndex labelIndex; //!< Index of the label the branch targets
>        bool hasPredicate:1;   //!< Is it predicated?
> +      bool inversePredicate:1;   //!< Is it inverse predicated?
>        bool hasLabel:1;       //!< Is there any target label?
>        Register dst[0];       //!< No destination
>      };
> @@ -1142,6 +1145,8 @@ namespace ir {
>  
>      INLINE void BranchInstruction::out(std::ostream &out, const Function &fn) const {
>        this->outOpcode(out);
> +      if(opcode == OP_IF && inversePredicate)
> +        out << " !";
>        if (hasPredicate)
>          out << "<%" << this->getSrc(fn, 0) << ">";
>        if (hasLabel) out << " -> label$" << labelIndex;
> @@ -1463,6 +1468,7 @@ DECL_MEM_FN(LoadInstruction, bool, isAligned(void), isAligned())
>  DECL_MEM_FN(LoadImmInstruction, Type, getType(void), getType())
>  DECL_MEM_FN(LabelInstruction, LabelIndex, getLabelIndex(void), getLabelIndex())
>  DECL_MEM_FN(BranchInstruction, bool, isPredicated(void), isPredicated())
> +DECL_MEM_FN(BranchInstruction, bool, getInversePredicated(void), getInversePredicated())
>  DECL_MEM_FN(BranchInstruction, LabelIndex, getLabelIndex(void), getLabelIndex())
>  DECL_MEM_FN(SyncInstruction, uint32_t, getParameters(void), getParameters())
>  DECL_MEM_FN(SampleInstruction, Type, getSrcType(void), getSrcType())
> @@ -1615,8 +1621,8 @@ DECL_MEM_FN(GetImageInfoInstruction, uint8_t, getImageIndex(void), getImageIndex
>    }
>  
>    // IF
> -  Instruction IF(LabelIndex labelIndex, Register pred) {
> -    return internal::BranchInstruction(OP_IF, labelIndex, pred).convert();
> +  Instruction IF(LabelIndex labelIndex, Register pred, bool inv_pred) {
> +    return internal::BranchInstruction(OP_IF, labelIndex, pred, inv_pred).convert();
>    }
>  
>    // ELSE
> diff --git a/backend/src/ir/instruction.hpp b/backend/src/ir/instruction.hpp
> index e245638..39fb2db 100644
> --- a/backend/src/ir/instruction.hpp
> +++ b/backend/src/ir/instruction.hpp
> @@ -430,6 +430,8 @@ namespace ir {
>    public:
>      /*! Indicate if the branch is predicated */
>      bool isPredicated(void) const;
> +    /*! Indicate if the branch is inverse predicated */
> +    bool getInversePredicated(void) const;
>      /*! Return the predicate register (if predicated) */
>      RegisterData getPredicate(void) const {
>        GBE_ASSERTM(this->isPredicated() == true, "Branch is not predicated");
> @@ -663,7 +665,7 @@ namespace ir {
>    /*! (pred) bra labelIndex */
>    Instruction BRA(LabelIndex labelIndex, Register pred);
>    /*! (pred) if labelIndex */
> -  Instruction IF(LabelIndex labelIndex, Register pred);
> +  Instruction IF(LabelIndex labelIndex, Register pred, bool inv_pred=true);
>    /*! else labelIndex */
>    Instruction ELSE(LabelIndex labelIndex);
>    /*! endif */
> diff --git a/backend/src/ir/structural_analysis.cpp b/backend/src/ir/structural_analysis.cpp
> index dfc2118..e6fa777 100644
> --- a/backend/src/ir/structural_analysis.cpp
> +++ b/backend/src/ir/structural_analysis.cpp
> @@ -121,7 +121,7 @@ namespace analysis
>       * and insert IF instead
>       */
>      pbb->erase(it);
> -    ir::Instruction insn = ir::IF(matchingElseLabel, reg);
> +    ir::Instruction insn = ir::IF(matchingElseLabel, reg, node->inversePredicate);
>      ir::Instruction* p_new_insn = pbb->getParent().newInstruction(insn);
>      pbb->append(*p_new_insn);
>      pbb->matchingEndifLabel = matchingEndifLabel;
> @@ -724,7 +724,7 @@ namespace analysis
>        n = *(++(node->succs().begin()));
>  
>        /* check for if node then n */
> -      if(n->succs().size() == 1 &&
> +      if( n->succs().size() == 1 &&
>           n->preds().size() == 1 &&
>           *(n->succs().begin()) == m &&
>           !n->hasBarrier() && !node->hasBarrier())
> @@ -734,6 +734,9 @@ namespace analysis
>          nset.insert(n);
>  
>          Node* p = new IfThenNode(node, n);
> +        if(node->fallthrough() == m){
> +          node->inversePredicate = false;
> +        }
>  
>          if(node->canBeHandled == false || n->canBeHandled == false)
>            p->canBeHandled = false;
> @@ -752,6 +755,9 @@ namespace analysis
>          nset.insert(m);
>  
>          Node* p = new IfThenNode(node, m);
> +        if(node->fallthrough() == n){
> +          node->inversePredicate = false;
> +        }
>  
>          if(node->canBeHandled == false || m->canBeHandled == false)
>            p->canBeHandled = false;
> diff --git a/backend/src/ir/structural_analysis.hpp b/backend/src/ir/structural_analysis.hpp
> index 06c2f5f..03df39e 100644
> --- a/backend/src/ir/structural_analysis.hpp
> +++ b/backend/src/ir/structural_analysis.hpp
> @@ -87,7 +87,7 @@ namespace analysis
>    class Node
>    {
>    public:
> -    Node(RegionType rtype, const NodeList& children): has_barrier(false), mark(false), canBeHandled(true)
> +    Node(RegionType rtype, const NodeList& children): has_barrier(false), mark(false), canBeHandled(true), inversePredicate(true)
>      {
>        this->rtype = rtype;
>        this->children = children;
> @@ -118,6 +118,20 @@ namespace analysis
>      bool canBeHandled;
>      //label is for debug
>      int label;
> +    /* inversePredicate should be false under two circumstance,
> +     * fallthrough is the same with succs:
> +     * (1) n->succs == m && node->fallthrough == m
> +     * node
> +     * | \
> +     * |  \
> +     * m<--n
> +     * (2) m->succs == n && node->fallthrough == n
> +     * node
> +     * | \
> +     * |  \
> +     * m-->n
> +     * */
> +    bool inversePredicate;
>    };
>  
>    /* represents basic block */
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Beignet mailing list
> Beignet at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet


More information about the Beignet mailing list