[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