do not force float for immediate value load

Submitted by Guo Yejun on Aug. 5, 2015, 12:39 a.m.

Details

Message ID 1438735152-23530-1-git-send-email-yejun.guo@intel.com
State New
Headers show

Not browsing as part of any series.

Commit Message

Guo Yejun Aug. 5, 2015, 12:39 a.m.
float format is used to load immediate value with TYPE_U32/TYPE_S32,
actually it is not necessary since it is not a performance sensitive
point.

Signed-off-by: Guo Yejun <yejun.guo@intel.com>
---
 backend/src/backend/gen_insn_selection.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Patch hide | download patch | download mbox

diff --git a/backend/src/backend/gen_insn_selection.cpp b/backend/src/backend/gen_insn_selection.cpp
index b0ba9e3..0256d24 100644
--- a/backend/src/backend/gen_insn_selection.cpp
+++ b/backend/src/backend/gen_insn_selection.cpp
@@ -3205,8 +3205,8 @@  namespace gbe
           }
           sel.MOV(dst, imm.getIntegerValue() ? GenRegister::immuw(0xffff) : GenRegister::immuw(0));
         break;
-        case TYPE_U32:
-        case TYPE_S32:
+        case TYPE_U32: sel.MOV(dst, GenRegister::immud(imm.getIntegerValue())); break;
+        case TYPE_S32: sel.MOV(dst, GenRegister::immd(imm.getIntegerValue())); break;
         case TYPE_FLOAT:
           sel.MOV(GenRegister::retype(dst, GEN_TYPE_F),
                   GenRegister::immf(imm.asFloatValue()));

Comments

The patch LGTM. But I think below description is a little clearer.

Use Integer for U32/S32 Immediate load.

Previously we use float type for U32/S32 immediate load from performance perspective, but using integer type would make things clearer and easy to debug.

> -----Original Message-----

> From: Beignet [mailto:beignet-bounces@lists.freedesktop.org] On Behalf Of

> Guo Yejun

> Sent: Wednesday, August 5, 2015 8:39 AM

> To: beignet@lists.freedesktop.org

> Cc: Guo, Yejun

> Subject: [Beignet] [PATCH] do not force float for immediate value load

> 

> float format is used to load immediate value with TYPE_U32/TYPE_S32, actually

> it is not necessary since it is not a performance sensitive point.

> 

> Signed-off-by: Guo Yejun <yejun.guo@intel.com>

> ---

>  backend/src/backend/gen_insn_selection.cpp | 4 ++--

>  1 file changed, 2 insertions(+), 2 deletions(-)

> 

> diff --git a/backend/src/backend/gen_insn_selection.cpp

> b/backend/src/backend/gen_insn_selection.cpp

> index b0ba9e3..0256d24 100644

> --- a/backend/src/backend/gen_insn_selection.cpp

> +++ b/backend/src/backend/gen_insn_selection.cpp

> @@ -3205,8 +3205,8 @@ namespace gbe

>            }

>            sel.MOV(dst, imm.getIntegerValue() ? GenRegister::immuw(0xffff) :

> GenRegister::immuw(0));

>          break;

> -        case TYPE_U32:

> -        case TYPE_S32:

> +        case TYPE_U32: sel.MOV(dst,

> GenRegister::immud(imm.getIntegerValue())); break;

> +        case TYPE_S32: sel.MOV(dst,

> + GenRegister::immd(imm.getIntegerValue())); break;

>          case TYPE_FLOAT:

>            sel.MOV(GenRegister::retype(dst, GEN_TYPE_F),

>                    GenRegister::immf(imm.asFloatValue()));

> --

> 1.9.1

> 

> _______________________________________________

> Beignet mailing list

> Beignet@lists.freedesktop.org

> http://lists.freedesktop.org/mailman/listinfo/beignet
Yes, thanks.

-----Original Message-----
From: Song, Ruiling 

Sent: Thursday, August 06, 2015 11:00 AM
To: Guo, Yejun; beignet@lists.freedesktop.org
Cc: Guo, Yejun
Subject: RE: [Beignet] [PATCH] do not force float for immediate value load


The patch LGTM. But I think below description is a little clearer.

Use Integer for U32/S32 Immediate load.

Previously we use float type for U32/S32 immediate load from performance perspective, but using integer type would make things clearer and easy to debug.

> -----Original Message-----

> From: Beignet [mailto:beignet-bounces@lists.freedesktop.org] On Behalf 

> Of Guo Yejun

> Sent: Wednesday, August 5, 2015 8:39 AM

> To: beignet@lists.freedesktop.org

> Cc: Guo, Yejun

> Subject: [Beignet] [PATCH] do not force float for immediate value load

> 

> float format is used to load immediate value with TYPE_U32/TYPE_S32, 

> actually it is not necessary since it is not a performance sensitive point.

> 

> Signed-off-by: Guo Yejun <yejun.guo@intel.com>

> ---

>  backend/src/backend/gen_insn_selection.cpp | 4 ++--

>  1 file changed, 2 insertions(+), 2 deletions(-)

> 

> diff --git a/backend/src/backend/gen_insn_selection.cpp

> b/backend/src/backend/gen_insn_selection.cpp

> index b0ba9e3..0256d24 100644

> --- a/backend/src/backend/gen_insn_selection.cpp

> +++ b/backend/src/backend/gen_insn_selection.cpp

> @@ -3205,8 +3205,8 @@ namespace gbe

>            }

>            sel.MOV(dst, imm.getIntegerValue() ? GenRegister::immuw(0xffff) :

> GenRegister::immuw(0));

>          break;

> -        case TYPE_U32:

> -        case TYPE_S32:

> +        case TYPE_U32: sel.MOV(dst,

> GenRegister::immud(imm.getIntegerValue())); break;

> +        case TYPE_S32: sel.MOV(dst,

> + GenRegister::immd(imm.getIntegerValue())); break;

>          case TYPE_FLOAT:

>            sel.MOV(GenRegister::retype(dst, GEN_TYPE_F),

>                    GenRegister::immf(imm.asFloatValue()));

> --

> 1.9.1

> 

> _______________________________________________

> Beignet mailing list

> Beignet@lists.freedesktop.org

> http://lists.freedesktop.org/mailman/listinfo/beignet