[Beignet] [PATCH 1/2] Fix convert long/ulong to float.

Zhigang Gong zhigang.gong at linux.intel.com
Mon Dec 23 02:32:29 PST 2013


My major concern is that this patch introduces a branch instruction at instruction selection
stage. And our current implementation assumes that the instruction selection stage should not
break the original BB, so it doesn't check the JMP instruction we generated here, and it still
treat the origin BB which now has some JMPs as a valid BB. Then the register interval calculating
may not be correct within that invalid BB.

IMO, we'd better avoid introducing branching at the select stage. Is it possible to mov it to
the earilier stage?

On Tue, Dec 17, 2013 at 03:32:12PM +0800, Yang Rong wrote:
> Previour implement don't handle rounding. The default rouding mode should be round to even.
> According float format, separate long/ulong to two part, first 23 non zero bits is mantissa,
> add 1 when the next bit is 1, and than round to even when all remain bits is zero.
> 
> Signed-off-by: Yang Rong <rong.r.yang at intel.com>
> ---
>  backend/src/backend/gen_context.cpp        | 110 +++++++++++++++++++++++++----
>  backend/src/backend/gen_context.hpp        |   2 +-
>  backend/src/backend/gen_insn_selection.cpp |  18 ++---
>  3 files changed, 108 insertions(+), 22 deletions(-)
> 
> diff --git a/backend/src/backend/gen_context.cpp b/backend/src/backend/gen_context.cpp
> index 6fd2dea..7ebe03c 100644
> --- a/backend/src/backend/gen_context.cpp
> +++ b/backend/src/backend/gen_context.cpp
> @@ -822,12 +822,95 @@ namespace gbe
>      p->pop();
>    }
>  
> -  void GenContext::UnsignedI64ToFloat(GenRegister dst, GenRegister high, GenRegister low, GenRegister tmp) {
> -    p->MOV(dst, high);
> -    p->MUL(dst, dst, GenRegister::immf(65536.f * 65536.f));
> -    tmp.type = GEN_TYPE_F;
> -    p->MOV(tmp, low);
> -    p->ADD(dst, dst, tmp);
> +  void GenContext::UnsignedI64ToFloat(GenRegister dst, GenRegister high, GenRegister low, GenRegister exp,
> +                                            GenRegister mantissa, GenRegister tmp, GenRegister flag) {
> +    uint32_t jip0, jip1;
> +    GenRegister dst_ud = GenRegister::retype(dst, GEN_TYPE_UD);
> +    p->FBH(exp, high);
> +    p->ADD(exp, GenRegister::negate(exp), GenRegister::immud(31));  //exp = 32 when high == 0
> +    p->push();
> +      p->curr.useFlag(flag.flag_nr(), flag.flag_subnr());
> +      p->curr.predicate = GEN_PREDICATE_NONE;
> +      p->CMP(GEN_CONDITIONAL_EQ, exp, GenRegister::immud(32));   //high == 0
> +      p->curr.predicate = GEN_PREDICATE_NORMAL;
> +      p->MOV(dst, low);
> +      p->push();
> +        if (simdWidth == 8)
> +          p->curr.predicate = GEN_PREDICATE_ALIGN1_ALL8H;
> +        else if (simdWidth == 16)
> +          p->curr.predicate = GEN_PREDICATE_ALIGN1_ALL16H;
> +        else
> +          NOT_IMPLEMENTED;
> +        p->curr.execWidth = 1;
> +        p->curr.noMask = 1;
> +        p->JMPI(GenRegister::immud(0));
> +        jip0 = p->n_instruction() - 1;
> +      p->pop();
> +
> +      p->curr.predicate = GEN_PREDICATE_NONE;
> +      p->CMP(GEN_CONDITIONAL_G, exp, GenRegister::immud(23));
> +      p->curr.predicate = GEN_PREDICATE_NORMAL;
> +      p->CMP(GEN_CONDITIONAL_L, exp, GenRegister::immud(32));  //exp>23 && high!=0
> +      p->ADD(tmp, exp, GenRegister::immud(-23));
> +      p->SHR(mantissa, high, tmp);
> +      p->AND(mantissa, mantissa, GenRegister::immud(0x7fffff));
> +      p->SHR(dst_ud, low, tmp);   //dst is temp regitster here
> +      p->ADD(tmp, GenRegister::negate(tmp), GenRegister::immud(32));
> +      p->SHL(high, high, tmp);
> +      p->OR(high, high, dst_ud);
> +      p->SHL(low, low, tmp);
> +      p->push();
> +        if (simdWidth == 8)
> +          p->curr.predicate = GEN_PREDICATE_ALIGN1_ALL8H;
> +        else if (simdWidth == 16)
> +          p->curr.predicate = GEN_PREDICATE_ALIGN1_ALL16H;
> +        else
> +          NOT_IMPLEMENTED;
> +        p->curr.execWidth = 1;
> +        p->curr.noMask = 1;
> +        p->JMPI(GenRegister::immud(0));
> +        jip1 = p->n_instruction() - 1;
> +      p->pop();
> +
> +      p->curr.predicate = GEN_PREDICATE_NONE;
> +      p->CMP(GEN_CONDITIONAL_EQ, exp, GenRegister::immud(23));
> +      p->curr.predicate = GEN_PREDICATE_NORMAL;
> +      p->MOV(dst_ud, GenRegister::immud(0));   //exp==9, SHR == 0
> +
> +      p->curr.predicate = GEN_PREDICATE_NONE;
> +      p->CMP(GEN_CONDITIONAL_L, exp, GenRegister::immud(23));
> +      p->curr.predicate = GEN_PREDICATE_NORMAL;
> +      p->ADD(tmp, exp, GenRegister::immud(9));
> +      p->SHR(dst_ud, low, tmp);   //dst is temp regitster here
> +
> +      p->curr.predicate = GEN_PREDICATE_NONE;
> +      p->CMP(GEN_CONDITIONAL_LE, exp, GenRegister::immud(23));
> +      p->curr.predicate = GEN_PREDICATE_NORMAL;
> +      p->ADD(tmp, GenRegister::negate(exp), GenRegister::immud(23));
> +      p->SHL(mantissa, high, tmp);
> +      p->OR(mantissa, mantissa, dst_ud);
> +      p->AND(mantissa, mantissa, GenRegister::immud(0x7fffff));
> +      p->SHL(high, low, tmp);
> +      p->MOV(low, GenRegister::immud(0));
> +
> +      p->patchJMPI(jip1, (p->n_instruction() - jip1 + 1) * 2);
> +      p->curr.predicate = GEN_PREDICATE_NONE;
> +      p->CMP(GEN_CONDITIONAL_LE, exp, GenRegister::immud(31));  //update dst where high != 0
> +      p->curr.predicate = GEN_PREDICATE_NORMAL;
> +      p->ADD(exp, exp, GenRegister::immud(159));
> +      p->SHL(exp, exp, GenRegister::immud(23));
> +      p->OR(dst_ud, exp, mantissa);
> +
> +      p->CMP(GEN_CONDITIONAL_GE, high, GenRegister::immud(0x80000000));
> +      p->ADD(dst_ud, dst_ud, GenRegister::immud(1));
> +
> +      p->CMP(GEN_CONDITIONAL_EQ, high, GenRegister::immud(0x80000000));
> +      p->CMP(GEN_CONDITIONAL_EQ, low, GenRegister::immud(0x0));
> +      p->AND(dst_ud, dst_ud, GenRegister::immud(0xfffffffe));
> +      p->patchJMPI(jip0, (p->n_instruction() - jip0 + 1) * 2);
> +
> +    p->pop();
> +
>    }
>  
>    void GenContext::emitI64ToFloatInstruction(const SelectionInstruction &insn) {
> @@ -835,16 +918,19 @@ namespace gbe
>      GenRegister dest = ra->genReg(insn.dst(0));
>      GenRegister high = ra->genReg(insn.dst(1));
>      GenRegister low = ra->genReg(insn.dst(2));
> -    GenRegister tmp = ra->genReg(insn.dst(3));
> -    GenRegister flagReg = ra->genReg(insn.dst(4));
> +    GenRegister exp = ra->genReg(insn.dst(3));
> +    GenRegister mantissa = ra->genReg(insn.dst(4));
> +    GenRegister tmp = ra->genReg(insn.dst(5));
> +    GenRegister f0 = ra->genReg(insn.dst(6));
> +    GenRegister f1 = ra->genReg(insn.dst(7));
>      loadTopHalf(high, src);
>      loadBottomHalf(low, src);
>      if(!src.is_signed_int()) {
> -      UnsignedI64ToFloat(dest, high, low, tmp);
> +      UnsignedI64ToFloat(dest, high, low, exp, mantissa, tmp, f0);
>      } else {
>        p->push();
>        p->curr.predicate = GEN_PREDICATE_NONE;
> -      p->curr.useFlag(flagReg.flag_nr(), flagReg.flag_subnr());
> +      p->curr.useFlag(f1.flag_nr(), f1.flag_subnr());
>        p->CMP(GEN_CONDITIONAL_GE, high, GenRegister::immud(0x80000000));
>        p->curr.predicate = GEN_PREDICATE_NORMAL;
>        p->NOT(high, high);
> @@ -853,9 +939,9 @@ namespace gbe
>        addWithCarry(low, low, tmp);
>        p->ADD(high, high, tmp);
>        p->pop();
> -      UnsignedI64ToFloat(dest, high, low, tmp);
> +      UnsignedI64ToFloat(dest, high, low, exp, mantissa, tmp, f0);
>        p->push();
> -      p->curr.useFlag(flagReg.flag_nr(), flagReg.flag_subnr());
> +      p->curr.useFlag(f1.flag_nr(), f1.flag_subnr());
>        dest.type = GEN_TYPE_UD;
>        p->OR(dest, dest, GenRegister::immud(0x80000000));
>        p->pop();
> diff --git a/backend/src/backend/gen_context.hpp b/backend/src/backend/gen_context.hpp
> index f8ef8e0..c9c5621 100644
> --- a/backend/src/backend/gen_context.hpp
> +++ b/backend/src/backend/gen_context.hpp
> @@ -92,7 +92,7 @@ namespace gbe
>      void I32FullMult(GenRegister high, GenRegister low, GenRegister src0, GenRegister src1);
>      void I64FullMult(GenRegister dst1, GenRegister dst2, GenRegister dst3, GenRegister dst4, GenRegister x_high, GenRegister x_low, GenRegister y_high, GenRegister y_low);
>      void saveFlag(GenRegister dest, int flag, int subFlag);
> -    void UnsignedI64ToFloat(GenRegister dst, GenRegister high, GenRegister low, GenRegister tmp);
> +    void UnsignedI64ToFloat(GenRegister dst, GenRegister high, GenRegister low, GenRegister exp, GenRegister mantissa, GenRegister tmp, GenRegister flag);
>  
>      /*! Final Gen ISA emission helper functions */
>      void emitLabelInstruction(const SelectionInstruction &insn);
> diff --git a/backend/src/backend/gen_insn_selection.cpp b/backend/src/backend/gen_insn_selection.cpp
> index 23e9da7..a35b237 100644
> --- a/backend/src/backend/gen_insn_selection.cpp
> +++ b/backend/src/backend/gen_insn_selection.cpp
> @@ -473,7 +473,7 @@ namespace gbe
>  #undef ALU3
>  #undef I64Shift
>      /*! Convert 64-bit integer to 32-bit float */
> -    void CONVI64_TO_F(Reg dst, Reg src, GenRegister tmp[4]);
> +    void CONVI64_TO_F(Reg dst, Reg src, GenRegister tmp[7]);
>      /*! Convert 64-bit integer to 32-bit float */
>      void CONVF_TO_I64(Reg dst, Reg src, GenRegister tmp[3]);
>      /*! Saturated 64bit x*y + z */
> @@ -1132,11 +1132,11 @@ namespace gbe
>        insn->dst(i + 1) = tmp[i];
>    }
>  
> -  void Selection::Opaque::CONVI64_TO_F(Reg dst, Reg src, GenRegister tmp[4]) {
> -    SelectionInstruction *insn = this->appendInsn(SEL_OP_CONVI64_TO_F, 5, 1);
> +  void Selection::Opaque::CONVI64_TO_F(Reg dst, Reg src, GenRegister tmp[7]) {
> +    SelectionInstruction *insn = this->appendInsn(SEL_OP_CONVI64_TO_F, 8, 1);
>      insn->dst(0) = dst;
>      insn->src(0) = src;
> -    for(int i = 0; i < 4; i ++)
> +    for(int i = 0; i < 7; i ++)
>        insn->dst(i + 1) = tmp[i];
>    }
>  
> @@ -2697,12 +2697,12 @@ namespace gbe
>        } else if ((dstType == ir::TYPE_S32 || dstType == ir::TYPE_U32) && srcFamily == FAMILY_QWORD) {
>          sel.CONVI64_TO_I(dst, src);
>        } else if (dstType == ir::TYPE_FLOAT && srcFamily == FAMILY_QWORD) {
> -        GenRegister tmp[4];
> -        for(int i=0; i<3; i++) {
> -          tmp[i] = sel.selReg(sel.reg(FAMILY_DWORD));
> -          tmp[i].type = GEN_TYPE_UD;
> +        GenRegister tmp[7];
> +        for(int i=0; i<5; i++) {
> +          tmp[i] = sel.selReg(sel.reg(FAMILY_DWORD), TYPE_U32);
>          }
> -        tmp[3] = sel.selReg(sel.reg(FAMILY_BOOL));
> +        tmp[5] = sel.selReg(sel.reg(FAMILY_BOOL), TYPE_BOOL);
> +        tmp[6] = sel.selReg(sel.reg(FAMILY_BOOL), TYPE_BOOL);
>          sel.CONVI64_TO_F(dst, src, tmp);
>        } else if (dst.isdf()) {
>          ir::Register r = sel.reg(ir::RegisterFamily::FAMILY_QWORD);
> -- 
> 1.8.1.2
> 
> _______________________________________________
> Beignet mailing list
> Beignet at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet


More information about the Beignet mailing list