libocl: fix degrees function precision issue.

Submitted by Luo, Xionghu on Aug. 6, 2015, 7:57 a.m.

Details

Message ID 1438847870-22514-1-git-send-email-xionghu.luo@intel.com
State New
Headers show

Not browsing as part of any series.

Commit Message

Luo, Xionghu Aug. 6, 2015, 7:57 a.m.
From: Luo Xionghu <xionghu.luo@intel.com>

should define and use M_180_PI_F directly instead of using 180/M_PI_F.

Signed-off-by: Luo Xionghu <xionghu.luo@intel.com>
---
 backend/src/libocl/include/ocl_float.h     | 1 +
 backend/src/libocl/tmpl/ocl_common.tmpl.cl | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

Patch hide | download patch | download mbox

diff --git a/backend/src/libocl/include/ocl_float.h b/backend/src/libocl/include/ocl_float.h
index 916233b..e63eaf9 100644
--- a/backend/src/libocl/include/ocl_float.h
+++ b/backend/src/libocl/include/ocl_float.h
@@ -88,6 +88,7 @@  INLINE_OVERLOADABLE int __ocl_finitef (float x){
 #define M_PI_4_F     0.7853981633974483F
 #define M_1_PI_F     0.3183098861837907F
 #define M_2_PI_F     0.6366197723675814F
+#define M_180_PI_F   57.295779513082321F
 #define M_2_SQRTPI_F 1.1283791670955126F
 #define M_SQRT2_F    1.4142135623730951F
 #define M_SQRT1_2_F  0.7071067811865476F
diff --git a/backend/src/libocl/tmpl/ocl_common.tmpl.cl b/backend/src/libocl/tmpl/ocl_common.tmpl.cl
index 76aca2b..136fe70 100644
--- a/backend/src/libocl/tmpl/ocl_common.tmpl.cl
+++ b/backend/src/libocl/tmpl/ocl_common.tmpl.cl
@@ -44,7 +44,7 @@  OVERLOADABLE float clamp(float v, float l, float u) {
 
 
 OVERLOADABLE float degrees(float radians) {
-  return (180 / M_PI_F) * radians;
+  return M_180_PI_F * radians;
 }
 OVERLOADABLE float radians(float degrees) {
   return (M_PI_F / 180) * degrees;

Comments

LGTM, thx.

> -----Original Message-----
> From: Beignet [mailto:beignet-bounces@lists.freedesktop.org] On Behalf Of
> xionghu.luo@intel.com
> Sent: Thursday, August 6, 2015 3:58 PM
> To: beignet@lists.freedesktop.org
> Cc: xionghu.luo@intel.com
> Subject: [Beignet] [PATCH] libocl: fix degrees function precision issue.
> 
> From: Luo Xionghu <xionghu.luo@intel.com>
> 
> should define and use M_180_PI_F directly instead of using 180/M_PI_F.
> 
> Signed-off-by: Luo Xionghu <xionghu.luo@intel.com>
> ---
>  backend/src/libocl/include/ocl_float.h     | 1 +
>  backend/src/libocl/tmpl/ocl_common.tmpl.cl | 2 +-
>  2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/backend/src/libocl/include/ocl_float.h
> b/backend/src/libocl/include/ocl_float.h
> index 916233b..e63eaf9 100644
> --- a/backend/src/libocl/include/ocl_float.h
> +++ b/backend/src/libocl/include/ocl_float.h
> @@ -88,6 +88,7 @@ INLINE_OVERLOADABLE int __ocl_finitef (float x){
>  #define M_PI_4_F     0.7853981633974483F
>  #define M_1_PI_F     0.3183098861837907F
>  #define M_2_PI_F     0.6366197723675814F
> +#define M_180_PI_F   57.295779513082321F
>  #define M_2_SQRTPI_F 1.1283791670955126F
>  #define M_SQRT2_F    1.4142135623730951F
>  #define M_SQRT1_2_F  0.7071067811865476F diff --git
> a/backend/src/libocl/tmpl/ocl_common.tmpl.cl
> b/backend/src/libocl/tmpl/ocl_common.tmpl.cl
> index 76aca2b..136fe70 100644
> --- a/backend/src/libocl/tmpl/ocl_common.tmpl.cl
> +++ b/backend/src/libocl/tmpl/ocl_common.tmpl.cl
> @@ -44,7 +44,7 @@ OVERLOADABLE float clamp(float v, float l, float u) {
> 
> 
>  OVERLOADABLE float degrees(float radians) {
> -  return (180 / M_PI_F) * radians;
> +  return M_180_PI_F * radians;
>  }
>  OVERLOADABLE float radians(float degrees) {
>    return (M_PI_F / 180) * degrees;
> --
> 1.9.1
> 
> _______________________________________________
> Beignet mailing list
> Beignet@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet
I sent the v2 patch which include half degrees function fix, please push that one and ignore this. Thanks.

Luo Xionghu
Best Regards

-----Original Message-----
From: Zhigang Gong [mailto:zhigang.gong@linux.intel.com] 

Sent: Thursday, August 6, 2015 4:27 PM
To: Luo, Xionghu; beignet@lists.freedesktop.org
Subject: RE: [Beignet] [PATCH] libocl: fix degrees function precision issue.

LGTM, thx.

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

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

> Of xionghu.luo@intel.com

> Sent: Thursday, August 6, 2015 3:58 PM

> To: beignet@lists.freedesktop.org

> Cc: xionghu.luo@intel.com

> Subject: [Beignet] [PATCH] libocl: fix degrees function precision issue.

> 

> From: Luo Xionghu <xionghu.luo@intel.com>

> 

> should define and use M_180_PI_F directly instead of using 180/M_PI_F.

> 

> Signed-off-by: Luo Xionghu <xionghu.luo@intel.com>

> ---

>  backend/src/libocl/include/ocl_float.h     | 1 +

>  backend/src/libocl/tmpl/ocl_common.tmpl.cl | 2 +-

>  2 files changed, 2 insertions(+), 1 deletion(-)

> 

> diff --git a/backend/src/libocl/include/ocl_float.h

> b/backend/src/libocl/include/ocl_float.h

> index 916233b..e63eaf9 100644

> --- a/backend/src/libocl/include/ocl_float.h

> +++ b/backend/src/libocl/include/ocl_float.h

> @@ -88,6 +88,7 @@ INLINE_OVERLOADABLE int __ocl_finitef (float x){

>  #define M_PI_4_F     0.7853981633974483F

>  #define M_1_PI_F     0.3183098861837907F

>  #define M_2_PI_F     0.6366197723675814F

> +#define M_180_PI_F   57.295779513082321F

>  #define M_2_SQRTPI_F 1.1283791670955126F

>  #define M_SQRT2_F    1.4142135623730951F

>  #define M_SQRT1_2_F  0.7071067811865476F diff --git 

> a/backend/src/libocl/tmpl/ocl_common.tmpl.cl

> b/backend/src/libocl/tmpl/ocl_common.tmpl.cl

> index 76aca2b..136fe70 100644

> --- a/backend/src/libocl/tmpl/ocl_common.tmpl.cl

> +++ b/backend/src/libocl/tmpl/ocl_common.tmpl.cl

> @@ -44,7 +44,7 @@ OVERLOADABLE float clamp(float v, float l, float u) 

> {

> 

> 

>  OVERLOADABLE float degrees(float radians) {

> -  return (180 / M_PI_F) * radians;

> +  return M_180_PI_F * radians;

>  }

>  OVERLOADABLE float radians(float degrees) {

>    return (M_PI_F / 180) * degrees;

> --

> 1.9.1

> 

> _______________________________________________

> Beignet mailing list

> Beignet@lists.freedesktop.org

> http://lists.freedesktop.org/mailman/listinfo/beignet
On Thu, Aug 6, 2015 at 12:57 AM,  <xionghu.luo@intel.com> wrote:
> From: Luo Xionghu <xionghu.luo@intel.com>
>
> should define and use M_180_PI_F directly instead of using 180/M_PI_F.
>
> Signed-off-by: Luo Xionghu <xionghu.luo@intel.com>
> ---
>  backend/src/libocl/include/ocl_float.h     | 1 +
>  backend/src/libocl/tmpl/ocl_common.tmpl.cl | 2 +-
>  2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/backend/src/libocl/include/ocl_float.h b/backend/src/libocl/include/ocl_float.h
> index 916233b..e63eaf9 100644
> --- a/backend/src/libocl/include/ocl_float.h
> +++ b/backend/src/libocl/include/ocl_float.h
> @@ -88,6 +88,7 @@ INLINE_OVERLOADABLE int __ocl_finitef (float x){
>  #define M_PI_4_F     0.7853981633974483F
>  #define M_1_PI_F     0.3183098861837907F
>  #define M_2_PI_F     0.6366197723675814F
> +#define M_180_PI_F   57.295779513082321F
>  #define M_2_SQRTPI_F 1.1283791670955126F
>  #define M_SQRT2_F    1.4142135623730951F
>  #define M_SQRT1_2_F  0.7071067811865476F
> diff --git a/backend/src/libocl/tmpl/ocl_common.tmpl.cl b/backend/src/libocl/tmpl/ocl_common.tmpl.cl
> index 76aca2b..136fe70 100644
> --- a/backend/src/libocl/tmpl/ocl_common.tmpl.cl
> +++ b/backend/src/libocl/tmpl/ocl_common.tmpl.cl
> @@ -44,7 +44,7 @@ OVERLOADABLE float clamp(float v, float l, float u) {
>
>
>  OVERLOADABLE float degrees(float radians) {
> -  return (180 / M_PI_F) * radians;
> +  return M_180_PI_F * radians;

I was surprised by this, so I wrote a program to test. Indeed, 180 /
(float)M_PI is less precise:

(float)(180 / M_PI) = 57.29577636718750000 (0x1.ca5dc0p+5) (0x42652ee0)
(180 / (float)M_PI) = 57.29577636718750000 (0x1.ca5dc0p+5) (0x42652ee0)
57.295779513082321F = 57.29578018188476562 (0x1.ca5dc2p+5) (0x42652ee1)

A difference of one bit. :)
> I was surprised by this, so I wrote a program to test. Indeed, 180 / (float)M_PI is

> less precise:

> 

> (float)(180 / M_PI) = 57.29577636718750000 (0x1.ca5dc0p+5) (0x42652ee0)

> (180 / (float)M_PI) = 57.29577636718750000 (0x1.ca5dc0p+5) (0x42652ee0)

> 57.295779513082321F = 57.29578018188476562 (0x1.ca5dc2p+5) (0x42652ee1)


It seems the precision loss was because pi has ultimate precision bits.
As division is not correctly rounded, but multiply is correctly rounded.
So I tried 180 * M_1_PI_F, (M_1_PI_F stands for 1.0/pi) the value is still (0x1.ca5dc0p+5) (0x42652ee0)
The correct value of 180/m_pi is 0x1.ca5dc1a63p+5.
When it is represented using single floating point, the guard bits are '110'  (comes from 1a).
Using round to even rule, should be round-up. Then we got 0x1.ca5dc2p+5
The patch is good!
> 

> A difference of one bit. :)

> _______________________________________________

> Beignet mailing list

> Beignet@lists.freedesktop.org

> http://lists.freedesktop.org/mailman/listinfo/beignet
This patch cause utest compiler_degrees fail, but it should be utest bug, can you send a patch to fix it.

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

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

> Luo, Xionghu

> Sent: Thursday, August 6, 2015 16:31

> To: 'Zhigang Gong'; beignet@lists.freedesktop.org

> Subject: Re: [Beignet] [PATCH] libocl: fix degrees function precision issue.

> 

> I sent the v2 patch which include half degrees function fix, please push that

> one and ignore this. Thanks.

> 

> Luo Xionghu

> Best Regards

> 

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

> From: Zhigang Gong [mailto:zhigang.gong@linux.intel.com]

> Sent: Thursday, August 6, 2015 4:27 PM

> To: Luo, Xionghu; beignet@lists.freedesktop.org

> Subject: RE: [Beignet] [PATCH] libocl: fix degrees function precision issue.

> 

> LGTM, thx.

> 

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

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

> > Of xionghu.luo@intel.com

> > Sent: Thursday, August 6, 2015 3:58 PM

> > To: beignet@lists.freedesktop.org

> > Cc: xionghu.luo@intel.com

> > Subject: [Beignet] [PATCH] libocl: fix degrees function precision issue.

> >

> > From: Luo Xionghu <xionghu.luo@intel.com>

> >

> > should define and use M_180_PI_F directly instead of using 180/M_PI_F.

> >

> > Signed-off-by: Luo Xionghu <xionghu.luo@intel.com>

> > ---

> >  backend/src/libocl/include/ocl_float.h     | 1 +

> >  backend/src/libocl/tmpl/ocl_common.tmpl.cl | 2 +-

> >  2 files changed, 2 insertions(+), 1 deletion(-)

> >

> > diff --git a/backend/src/libocl/include/ocl_float.h

> > b/backend/src/libocl/include/ocl_float.h

> > index 916233b..e63eaf9 100644

> > --- a/backend/src/libocl/include/ocl_float.h

> > +++ b/backend/src/libocl/include/ocl_float.h

> > @@ -88,6 +88,7 @@ INLINE_OVERLOADABLE int __ocl_finitef (float x){

> >  #define M_PI_4_F     0.7853981633974483F

> >  #define M_1_PI_F     0.3183098861837907F

> >  #define M_2_PI_F     0.6366197723675814F

> > +#define M_180_PI_F   57.295779513082321F

> >  #define M_2_SQRTPI_F 1.1283791670955126F

> >  #define M_SQRT2_F    1.4142135623730951F

> >  #define M_SQRT1_2_F  0.7071067811865476F diff --git

> > a/backend/src/libocl/tmpl/ocl_common.tmpl.cl

> > b/backend/src/libocl/tmpl/ocl_common.tmpl.cl

> > index 76aca2b..136fe70 100644

> > --- a/backend/src/libocl/tmpl/ocl_common.tmpl.cl

> > +++ b/backend/src/libocl/tmpl/ocl_common.tmpl.cl

> > @@ -44,7 +44,7 @@ OVERLOADABLE float clamp(float v, float l, float u)

> > {

> >

> >

> >  OVERLOADABLE float degrees(float radians) {

> > -  return (180 / M_PI_F) * radians;

> > +  return M_180_PI_F * radians;

> >  }

> >  OVERLOADABLE float radians(float degrees) {

> >    return (M_PI_F / 180) * degrees;

> > --

> > 1.9.1

> >

> > _______________________________________________

> > Beignet mailing list

> > Beignet@lists.freedesktop.org

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

> 

> _______________________________________________

> Beignet mailing list

> Beignet@lists.freedesktop.org

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