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

Yang, Rong R rong.r.yang at intel.com
Wed Dec 25 18:45:59 PST 2013


It is caused by wrong jumpDistance when patchJMPI. I have sent a new patch, thanks.

-----Original Message-----
From: Zhigang Gong [mailto:zhigang.gong at linux.intel.com] 
Sent: Wednesday, December 25, 2013 11:54 AM
To: Yang, Rong R
Cc: beignet at lists.freedesktop.org
Subject: Re: [Beignet] [PATCH 1/2] Fix convert long/ulong to float.

This patch hit a utest regression:

compiler_long_convert_to_float:
  compiler_long_convert_to_float()    [FAILED]
    Error: dst[i] == src[i]
  at file /home/gongzg/git/fdo/beignet/utests/compiler_long_convert.cpp, function compiler_long_convert_to_float, line 152

Could you take a look at it?

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