[Beignet] [PATCH] GBE: fix one double related bugs for post register scheduling.

Song, Ruiling ruiling.song at intel.com
Tue Nov 11 18:55:32 PST 2014


The patch LGTM.

> -----Original Message-----
> From: Beignet [mailto:beignet-bounces at lists.freedesktop.org] On Behalf Of
> Zhigang Gong
> Sent: Tuesday, November 11, 2014 8:54 PM
> To: beignet at lists.freedesktop.org
> Cc: Gong, Zhigang
> Subject: [Beignet] [PATCH] GBE: fix one double related bugs for post register
> scheduling.
> 
> We need to set the temporary register to U64 type, otherwise latter post
> register scheduling will do some bad things.
> 
> Although we don't support double currently, this bug still could be triggerred
> easily if you use printf("%f", foo).
> 
> Signed-off-by: Zhigang Gong <zhigang.gong at intel.com>
> ---
>  backend/src/backend/gen75_encoder.cpp      | 3 ++-
>  backend/src/backend/gen8_encoder.cpp       | 3 ++-
>  backend/src/backend/gen_encoder.cpp        | 3 ++-
>  backend/src/backend/gen_insn_selection.cpp | 6 +++---
>  4 files changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/backend/src/backend/gen75_encoder.cpp
> b/backend/src/backend/gen75_encoder.cpp
> index bd3dd67..c77ce4d 100644
> --- a/backend/src/backend/gen75_encoder.cpp
> +++ b/backend/src/backend/gen75_encoder.cpp
> @@ -211,8 +211,9 @@ namespace gbe
>      pop();
>    }
> 
> -  void Gen75Encoder::MOV_DF(GenRegister dest, GenRegister src0,
> GenRegister r) {
> +  void Gen75Encoder::MOV_DF(GenRegister dest, GenRegister src0,
> + GenRegister tmp) {
>      GBE_ASSERT((src0.type == GEN_TYPE_F && dest.isdf()) || (src0.isdf()
> && dest.type == GEN_TYPE_F));
> +    GenRegister r = GenRegister::retype(tmp, GEN_TYPE_F);
>      int w = curr.execWidth;
>      GenRegister r0;
>      r0 = GenRegister::h2(r);
> diff --git a/backend/src/backend/gen8_encoder.cpp
> b/backend/src/backend/gen8_encoder.cpp
> index 749bc52..ae2d4eb 100644
> --- a/backend/src/backend/gen8_encoder.cpp
> +++ b/backend/src/backend/gen8_encoder.cpp
> @@ -219,8 +219,9 @@ namespace gbe
>      pop();
>    }
> 
> -  void Gen8Encoder::MOV_DF(GenRegister dest, GenRegister src0,
> GenRegister r) {
> +  void Gen8Encoder::MOV_DF(GenRegister dest, GenRegister src0,
> + GenRegister tmp) {
>      GBE_ASSERT((src0.type == GEN_TYPE_F && dest.isdf()) || (src0.isdf()
> && dest.type == GEN_TYPE_F));
> +    GenRegister r = GenRegister::retype(tmp, GEN_TYPE_F);
>      int w = curr.execWidth;
>      GenRegister r0;
>      r0 = GenRegister::h2(r);
> diff --git a/backend/src/backend/gen_encoder.cpp
> b/backend/src/backend/gen_encoder.cpp
> index dc2cbd0..e4df109 100644
> --- a/backend/src/backend/gen_encoder.cpp
> +++ b/backend/src/backend/gen_encoder.cpp
> @@ -618,8 +618,9 @@ namespace gbe
>      MOV(dest.top_half(this->simdWidth), u1);
>    }
> 
> -  void GenEncoder::MOV_DF(GenRegister dest, GenRegister src0,
> GenRegister r) {
> +  void GenEncoder::MOV_DF(GenRegister dest, GenRegister src0,
> + GenRegister tmp) {
>      GBE_ASSERT((src0.type == GEN_TYPE_F && dest.isdf()) || (src0.isdf()
> && dest.type == GEN_TYPE_F));
> +    GenRegister r = GenRegister::retype(tmp, GEN_TYPE_F);
>      int w = curr.execWidth;
>      GenRegister r0;
>      int factor = 1;
> diff --git a/backend/src/backend/gen_insn_selection.cpp
> b/backend/src/backend/gen_insn_selection.cpp
> index b6d75b1..cd968c0 100644
> --- a/backend/src/backend/gen_insn_selection.cpp
> +++ b/backend/src/backend/gen_insn_selection.cpp
> @@ -1982,7 +1982,7 @@ namespace gbe
>            case ir::OP_MOV:
>              if (dst.isdf()) {
>                ir::Register r =
> sel.reg(ir::RegisterFamily::FAMILY_QWORD);
> -              sel.MOV_DF(dst, src, sel.selReg(r));
> +              sel.MOV_DF(dst, src, sel.selReg(r, ir::TYPE_U64));
>              } else {
>                sel.push();
>                  auto dag = sel.regDAG[insn.getDst(0)]; @@ -2784,7
> +2784,7 @@ namespace gbe
>          case TYPE_S16: sel.MOV(dst,
> GenRegister::immw(imm.getIntegerValue())); break;
>          case TYPE_U8:  sel.MOV(dst,
> GenRegister::immuw(imm.getIntegerValue())); break;
>          case TYPE_S8:  sel.MOV(dst,
> GenRegister::immw(imm.getIntegerValue())); break;
> -        case TYPE_DOUBLE: sel.LOAD_DF_IMM(dst,
> GenRegister::immdf(imm.getDoubleValue()),
> sel.selReg(sel.reg(FAMILY_QWORD))); break;
> +        case TYPE_DOUBLE: sel.LOAD_DF_IMM(dst,
> + GenRegister::immdf(imm.getDoubleValue()),
> + sel.selReg(sel.reg(FAMILY_QWORD), TYPE_U64)); break;
>          case TYPE_S64: sel.LOAD_INT64_IMM(dst,
> GenRegister::immint64(imm.getIntegerValue())); break;
>          case TYPE_U64: sel.LOAD_INT64_IMM(dst,
> GenRegister::immint64(imm.getIntegerValue())); break;
>          default: NOT_SUPPORTED;
> @@ -3687,7 +3687,7 @@ namespace gbe
>        } else if ((dst.isdf() && srcType == ir::TYPE_FLOAT) ||
>                   (src.isdf() && dstType == ir::TYPE_FLOAT)) {
>          ir::Register r = sel.reg(ir::RegisterFamily::FAMILY_QWORD);
> -        sel.MOV_DF(dst, src, sel.selReg(r));
> +        sel.MOV_DF(dst, src, sel.selReg(r, TYPE_U64));
>        } else if (dst.isint64()) {
>          switch(src.type) {
>            case GEN_TYPE_F:
> --
> 1.8.3.2
> 
> _______________________________________________
> Beignet mailing list
> Beignet at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet


More information about the Beignet mailing list