[Beignet] [Patch V2] Add FCmpInst ord support.

[Zhigang Gong]

In general, nice way to handle this type of functions.
One minor comment as below:

On Fri, Nov 15, 2013 at 10:44:11AM +0800, Yang Rong wrote:
> Because gen do not support isorder direct, use (src0 == src0) && (src1 == src1).
> BTW: can't use !unordered.
> 
> v2: Refine, don't need AND.
> 
> Signed-off-by: Yang Rong <rong.r.yang at intel.com>
> ---
>  backend/src/backend/gen_insn_selection.cpp | 7 ++++++-
>  backend/src/ir/instruction.cpp             | 1 +
>  backend/src/ir/instruction.hpp             | 2 ++
>  backend/src/ir/instruction.hxx             | 1 +
>  backend/src/llvm/llvm_gen_backend.cpp      | 1 +
>  5 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/backend/src/backend/gen_insn_selection.cpp b/backend/src/backend/gen_insn_selection.cpp
> index 5a6b9fd..3c915fd 100644
> --- a/backend/src/backend/gen_insn_selection.cpp
> +++ b/backend/src/backend/gen_insn_selection.cpp
> @@ -145,7 +145,8 @@ namespace gbe
>        case OP_GT: return GEN_CONDITIONAL_G;
>        case OP_EQ: return GEN_CONDITIONAL_EQ;
>        case OP_NE: return GEN_CONDITIONAL_NEQ;
> -      default: NOT_SUPPORTED; return 0u;
> +      case OP_ORD:  return -1u;
> +      default: NOT_SUPPORTED; return -1u;
Return a -1 for the above function is really weird for me. I took a look at the code, and found
it may be better to modify the CompareInstructionPattern : public SelectionPattern() to
not compute the genCmp at the entry part of the function. You can just remove the
const uint32_t genCmp = getGenCompare(opcode); And the made the following change should be ok.

-          sel.I64CMP(genCmp, src0, src1, tmp);
+          sel.I64CMP(getGenCompare(opcode), src0, src1, tmp)
        } else
-          sel.CMP(genCmp, src0, src1);
+          sel.CMP(getGenCompare(opcode), src0, src1);
}

Then we can remain the getGencompare the same as before, and return a valid gen compare opcode,
or trigger an error message. What do you think?

>      };
>    }
>  
> @@ -1955,6 +1956,7 @@ namespace gbe
>        // OK, we merge the instructions
>        const ir::CompareInstruction &cmpInsn = cast<CompareInstruction>(cmp->insn);
>        const ir::Opcode opcode = cmpInsn.getOpcode();
> +      if(opcode == OP_ORD) return false;
>        const uint32_t genCmp = getGenCompare(opcode);
>  
>        // Like for regular selects, we need a temporary since we cannot predicate
> @@ -2543,6 +2545,9 @@ namespace gbe
>            for(int i=0; i<3; i++)
>              tmp[i] = sel.selReg(sel.reg(FAMILY_DWORD));
>            sel.I64CMP(genCmp, src0, src1, tmp);
> +        } else if(opcode == OP_ORD) {
> +          sel.CMP(GEN_CONDITIONAL_EQ, src0, src0);
> +          sel.CMP(GEN_CONDITIONAL_EQ, src1, src1);
>          } else
>            sel.CMP(genCmp, src0, src1);
>        sel.pop();
> diff --git a/backend/src/ir/instruction.cpp b/backend/src/ir/instruction.cpp
> index da20d43..c624d7a 100644
> --- a/backend/src/ir/instruction.cpp
> +++ b/backend/src/ir/instruction.cpp
> @@ -1552,6 +1552,7 @@ DECL_MEM_FN(GetImageInfoInstruction, uint32_t, getInfoType(void), getInfoType())
>    DECL_EMIT_FUNCTION(LT)
>    DECL_EMIT_FUNCTION(GE)
>    DECL_EMIT_FUNCTION(GT)
> +  DECL_EMIT_FUNCTION(ORD)
>  
>  #undef DECL_EMIT_FUNCTION
>  
> diff --git a/backend/src/ir/instruction.hpp b/backend/src/ir/instruction.hpp
> index b1afd42..b7eebc0 100644
> --- a/backend/src/ir/instruction.hpp
> +++ b/backend/src/ir/instruction.hpp
> @@ -634,6 +634,8 @@ namespace ir {
>    Instruction GE(Type type, Register dst, Register src0, Register src1);
>    /*! ge.type dst src0 src1 */
>    Instruction GT(Type type, Register dst, Register src0, Register src1);
> +  /*! ord.type dst src0 src1 */
> +  Instruction ORD(Type type, Register dst, Register src0, Register src1);
>    /*! BITCAST.{dstType <- srcType} dst src */
>    Instruction BITCAST(Type dstType, Type srcType, Tuple dst, Tuple src, uint8_t dstNum, uint8_t srcNum);
>    /*! cvt.{dstType <- srcType} dst src */
> diff --git a/backend/src/ir/instruction.hxx b/backend/src/ir/instruction.hxx
> index 83ecd1d..cf79e09 100644
> --- a/backend/src/ir/instruction.hxx
> +++ b/backend/src/ir/instruction.hxx
> @@ -60,6 +60,7 @@ DECL_INSN(LE, CompareInstruction)
>  DECL_INSN(LT, CompareInstruction)
>  DECL_INSN(GE, CompareInstruction)
>  DECL_INSN(GT, CompareInstruction)
> +DECL_INSN(ORD, CompareInstruction)
>  DECL_INSN(BITCAST, BitCastInstruction)
>  DECL_INSN(CVT, ConvertInstruction)
>  DECL_INSN(SAT_CVT, ConvertInstruction)
> diff --git a/backend/src/llvm/llvm_gen_backend.cpp b/backend/src/llvm/llvm_gen_backend.cpp
> index 23e5442..688c2b2 100644
> --- a/backend/src/llvm/llvm_gen_backend.cpp
> +++ b/backend/src/llvm/llvm_gen_backend.cpp
> @@ -1596,6 +1596,7 @@ namespace gbe
>        case ICmpInst::FCMP_ULT: ctx.LT(type, dst, src0, src1); break;
>        case ICmpInst::FCMP_OGT:
>        case ICmpInst::FCMP_UGT: ctx.GT(type, dst, src0, src1); break;
> +      case ICmpInst::FCMP_ORD: ctx.ORD(type, dst, src0, src0); break;
>        default: NOT_SUPPORTED;
>      }
>    }
> -- 
> 1.8.1.2
> 
> _______________________________________________
> Beignet mailing list
> Beignet at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet