libm round - fix for 16-bit CPU

Message ID 000301d408b0$61a6e1a0$24f4a4e0$@beniston.com
State New
Headers show
Series
  • libm round - fix for 16-bit CPU
Related show

Commit Message

Jon Beniston June 20, 2018, 4:04 p.m.
Hi,

The following is a patch for round on 16-bit CPUs to avoid a shift being out
of range:

libm/common/s_round.c (round): Make constant long so shift isn't out of
range on 16-bit CPUs

---
 newlib/libm/common/s_round.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.15.1

Comments

Craig Howland June 20, 2018, 4:22 p.m. | #1
On 06/20/2018 12:04 PM, Jon Beniston wrote:
> Hi,

>

> The following is a patch for round on 16-bit CPUs to avoid a shift being out

> of range:

>

> libm/common/s_round.c (round): Make constant long so shift isn't out of

> range on 16-bit CPUs

>

> ---

>   newlib/libm/common/s_round.c | 2 +-

>   1 file changed, 1 insertion(+), 1 deletion(-)

>

> diff --git a/newlib/libm/common/s_round.c b/newlib/libm/common/s_round.c

> index 047574a84..f0926d85f 100644

> --- a/newlib/libm/common/s_round.c

> +++ b/newlib/libm/common/s_round.c

> @@ -68,7 +68,7 @@ SEEALSO

>             msw &= 0x80000000;

>             if (exponent_less_1023 == -1)

>               /* Result is +1.0 or -1.0. */

> -            msw |= (1023 << 20);

> +            msw |= (1023L << 20);

>             lsw = 0;

>           }

>         else

Might it be slightly better to use a cast with the type of msw? While it would 
probably make no difference on any 16-bit target, it could prevent an 
intermediate change to 64 bits instead of 32 on some targets.  So

+            msw |= (((__int32_t)1023 << 20);
  
(Or, better for maintenance, but not as portable with a GCC extension:

+            msw |= (((typeof(msw))1023 << 20);
(There do seem to be multiple uses of typeof already, although the first few I checked are all gated with validity checks.  So probably not appropriate as shown here.))

Craig
Jon Beniston June 20, 2018, 7:47 p.m. | #2
Hi Craig,

>Might it be slightly better to use a cast with the type of msw? 


Yep.

From: Jon Beniston <jon@beniston.com>

Date: Wed, 20 Jun 2018 20:45:01 +0100 
Subject: [PATCH] libm/common/s_round.c (round): Add cast for 16-bit CPUs

---
 newlib/libm/common/s_round.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
 diff --git a/newlib/libm/common/s_round.c b/newlib/libm/common/s_round.c
index f0926d85f..e3a91d6c2 100644
--- a/newlib/libm/common/s_round.c
+++ b/newlib/libm/common/s_round.c
@@ -68,7 +68,7 @@ SEEALSO
           msw &= 0x80000000;
           if (exponent_less_1023 == -1)
             /* Result is +1.0 or -1.0. */
-            msw |= (1023 << 20);
+            msw |= ((__int32_t)1023 << 20);
           lsw = 0;
         }
       else
-- 
2.15.1
Corinna Vinschen June 21, 2018, 7:35 a.m. | #3
On Jun 20 20:47, Jon Beniston wrote:
> From: Jon Beniston <jon@beniston.com>

> Date: Wed, 20 Jun 2018 20:45:01 +0100 

> Subject: [PATCH] libm/common/s_round.c (round): Add cast for 16-bit CPUs

> 

> ---

>  newlib/libm/common/s_round.c | 2 +-

>  1 file changed, 1 insertion(+), 1 deletion(-)

>  diff --git a/newlib/libm/common/s_round.c b/newlib/libm/common/s_round.c

> index f0926d85f..e3a91d6c2 100644

> --- a/newlib/libm/common/s_round.c

> +++ b/newlib/libm/common/s_round.c

> @@ -68,7 +68,7 @@ SEEALSO

>            msw &= 0x80000000;

>            if (exponent_less_1023 == -1)

>              /* Result is +1.0 or -1.0. */

> -            msw |= (1023 << 20);

> +            msw |= ((__int32_t)1023 << 20);

>            lsw = 0;

>          }

>        else

> -- 

> 2.15.1


Pushed.


Thanks,
Corinna
-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

Patch

diff --git a/newlib/libm/common/s_round.c b/newlib/libm/common/s_round.c
index 047574a84..f0926d85f 100644
--- a/newlib/libm/common/s_round.c
+++ b/newlib/libm/common/s_round.c
@@ -68,7 +68,7 @@  SEEALSO
           msw &= 0x80000000;
           if (exponent_less_1023 == -1)
             /* Result is +1.0 or -1.0. */
-            msw |= (1023 << 20);
+            msw |= (1023L << 20);
           lsw = 0;
         }
       else