Re: [Patch-86512]: Subnormal float support in armv7(with -msoft-float) for intrinsics

Re: [Patch-86512]: Subnormal float support in armv7(with -msoft-float) for intrinsics

[Wilco Dijkstra]

Hi Umesh,

Looking at your patch, this would break all results which need to be normalized.


Index: libgcc/config/arm/ieee754-df.S
===================================================================
--- libgcc/config/arm/ieee754-df.S	(revision 262850)
+++ libgcc/config/arm/ieee754-df.S	(working copy)
@@ -203,8 +203,11 @@
 #endif
 
 	@ Determine how to normalize the result.
+	@ if result is denormal i.e (exp)=0,then don't normalise the result,
 LSYM(Lad_p):
 	cmp	xh, #0x00100000
+	blt	LSYM(Lad_e)
+	cmp	xh, #0x00100000
 	bcc	LSYM(Lad_a)
 	cmp	xh, #0x00200000
 	bcc	LSYM(Lad_e)

It seems Lad_a doesn't correctly handle the case where the result is a denormal. For this case
the result is correct so nothing else needs to be done. This requires an explicit test that the
exponent is zero - other cases still need to be renormalized as usual. This code looks overly
complex so any change will require extensive testing of all the corner cases.

Wilco

Re: [Patch-86512]: Subnormal float support in armv7(with -msoft-float) for intrinsics

[Umesh Kalappa]

Thank you all for your comments .

Wilco,
We tried some of the normalisation numbers and the fix works and please
could you help us with the input ,where  if you see that fix breaks down.

Thank you again
~Umesh
On Fri, Jul 20, 2018, 7:07 PM Wilco Dijkstra <Wilco.Dijkstra@arm.com> wrote:

> Hi Umesh,
>
> Looking at your patch, this would break all results which need to be
> normalized.
>
>
> Index: libgcc/config/arm/ieee754-df.S
> ===================================================================
> --- libgcc/config/arm/ieee754-df.S      (revision 262850)
> +++ libgcc/config/arm/ieee754-df.S      (working copy)
> @@ -203,8 +203,11 @@
>  #endif
>
>         @ Determine how to normalize the result.
> +       @ if result is denormal i.e (exp)=0,then don't normalise the
> result,
>  LSYM(Lad_p):
>         cmp     xh, #0x00100000
> +       blt     LSYM(Lad_e)
> +       cmp     xh, #0x00100000
>         bcc     LSYM(Lad_a)
>         cmp     xh, #0x00200000
>         bcc     LSYM(Lad_e)
>
> It seems Lad_a doesn't correctly handle the case where the result is a
> denormal. For this case
> the result is correct so nothing else needs to be done. This requires an
> explicit test that the
> exponent is zero - other cases still need to be renormalized as usual.
> This code looks overly
> complex so any change will require extensive testing of all the corner
> cases.
>
> Wilco
>

Re: [Patch-86512]: Subnormal float support in armv7(with -msoft-float) for intrinsics

[Wilco Dijkstra]

Umesh Kalappa wrote:

> We tried some of the normalisation numbers and the fix works and please
> could you help us with the input ,where  if you see that fix breaks down.

Well try any set of inputs which require normalisation. You'll find these no
longer get normalised and so will get incorrect results. Try basic cases like
1.0 - 0.75 which I think will return 0.625...

A basic test would be to run old vs new on a large set of inputs to verify
there aren't any obvious differences.

Wilco
       

Re: [Patch-86512]: Subnormal float support in armv7(with -msoft-float) for intrinsics

[Umesh Kalappa]

[-- Attachment #1: Type: text/plain, Size: 857 bytes --]

Thank you Wilco for the inputs and we agree that the  fix break down
for the case.

Meanwhile ,attached patch will take care the inputs and we are testing
the patch vigorously ,would you recommended any test-suite out there
for the same ?

Thank you
~Umesh


On Fri, Jul 20, 2018 at 10:04 PM, Wilco Dijkstra <Wilco.Dijkstra@arm.com> wrote:
> Umesh Kalappa wrote:
>
>> We tried some of the normalisation numbers and the fix works and please
>> could you help us with the input ,where  if you see that fix breaks down.
>
> Well try any set of inputs which require normalisation. You'll find these no
> longer get normalised and so will get incorrect results. Try basic cases like
> 1.0 - 0.75 which I think will return 0.625...
>
> A basic test would be to run old vs new on a large set of inputs to verify
> there aren't any obvious differences.
>
> Wilco
>

[-- Attachment #2: pr86512.patch --]
[-- Type: application/octet-stream, Size: 1370 bytes --]

Index: libgcc/config/arm/ieee754-df.S
===================================================================
--- libgcc/config/arm/ieee754-df.S	(revision 262850)
+++ libgcc/config/arm/ieee754-df.S	(working copy)
@@ -203,6 +203,7 @@
 #endif
 
 	@ Determine how to normalize the result.
+	@ if result is denormal i.e (exp)=0,then don't normalise the result,
 LSYM(Lad_p):
 	cmp	xh, #0x00100000
 	bcc	LSYM(Lad_a)
@@ -235,6 +236,8 @@
 
 	@ Result must be shifted left and exponent adjusted.
 LSYM(Lad_a):
+	cmp     r4,#0x0
+	beq     LSYM(Lad_e)
 	movs	ip, ip, lsl #1
 	adcs	xl, xl, xl
 	adc	xh, xh, xh
Index: gcc/testsuite/gcc.target/arm/pr86512.c
===================================================================
--- gcc/testsuite/gcc.target/arm/pr86512.c	(nonexistent)
+++ gcc/testsuite/gcc.target/arm/pr86512.c	(working copy)
@@ -0,0 +1,28 @@
+/* { dg-do run } */
+/* { dg-options "-O0 -msoft-float" } */
+
+#include<stdlib.h>
+#include<stdint.h>
+
+typedef union
+{
+    double d;
+    uint64_t i;
+} u;
+
+int main()
+{
+  u smallestPositiveNormal, smallesPositiveSubnormal, expectedResult, result;
+
+  smallesPositiveSubnormal.i = 1;
+
+  smallestPositiveNormal.i = 0x0010000000000000;
+  expectedResult.i = 0x000fffffffffffff;
+  result.d = smallestPositiveNormal.d - smallesPositiveSubnormal.d;
+
+  if (result.i != expectedResult.i)
+	abort();
+	
+  return 0;
+}
+

Re: [Patch-86512]: Subnormal float support in armv7(with -msoft-float) for intrinsics

[Richard Earnshaw (lists)]

So why is this only changing the double-precision implementation?
Surely, a problem like this will normally be common to both SP and DP
floating-point computations.

R.

On 23/07/18 08:46, Umesh Kalappa wrote:
> Thank you Wilco for the inputs and we agree that the  fix break down
> for the case.
> 
> Meanwhile ,attached patch will take care the inputs and we are testing
> the patch vigorously ,would you recommended any test-suite out there
> for the same ?
> 
> Thank you
> ~Umesh
> 
> 
> On Fri, Jul 20, 2018 at 10:04 PM, Wilco Dijkstra <Wilco.Dijkstra@arm.com> wrote:
>> Umesh Kalappa wrote:
>>
>>> We tried some of the normalisation numbers and the fix works and please
>>> could you help us with the input ,where  if you see that fix breaks down.
>>
>> Well try any set of inputs which require normalisation. You'll find these no
>> longer get normalised and so will get incorrect results. Try basic cases like
>> 1.0 - 0.75 which I think will return 0.625...
>>
>> A basic test would be to run old vs new on a large set of inputs to verify
>> there aren't any obvious differences.
>>
>> Wilco
>>
>>
>> pr86512.patch
>>
>>
>> Index: libgcc/config/arm/ieee754-df.S
>> ===================================================================
>> --- libgcc/config/arm/ieee754-df.S	(revision 262850)
>> +++ libgcc/config/arm/ieee754-df.S	(working copy)
>> @@ -203,6 +203,7 @@
>>  #endif
>>  
>>  	@ Determine how to normalize the result.
>> +	@ if result is denormal i.e (exp)=0,then don't normalise the result,
>>  LSYM(Lad_p):
>>  	cmp	xh, #0x00100000
>>  	bcc	LSYM(Lad_a)
>> @@ -235,6 +236,8 @@
>>  
>>  	@ Result must be shifted left and exponent adjusted.
>>  LSYM(Lad_a):
>> +	cmp     r4,#0x0
>> +	beq     LSYM(Lad_e)
>>  	movs	ip, ip, lsl #1
>>  	adcs	xl, xl, xl
>>  	adc	xh, xh, xh
>> Index: gcc/testsuite/gcc.target/arm/pr86512.c
>> ===================================================================
>> --- gcc/testsuite/gcc.target/arm/pr86512.c	(nonexistent)
>> +++ gcc/testsuite/gcc.target/arm/pr86512.c	(working copy)
>> @@ -0,0 +1,28 @@
>> +/* { dg-do run } */
>> +/* { dg-options "-O0 -msoft-float" } */
>> +
>> +#include<stdlib.h>
>> +#include<stdint.h>
>> +
>> +typedef union
>> +{
>> +    double d;
>> +    uint64_t i;
>> +} u;
>> +
>> +int main()
>> +{
>> +  u smallestPositiveNormal, smallesPositiveSubnormal, expectedResult, result;
>> +
>> +  smallesPositiveSubnormal.i = 1;
>> +
>> +  smallestPositiveNormal.i = 0x0010000000000000;
>> +  expectedResult.i = 0x000fffffffffffff;
>> +  result.d = smallestPositiveNormal.d - smallesPositiveSubnormal.d;
>> +
>> +  if (result.i != expectedResult.i)
>> +	abort();
>> +	
>> +  return 0;
>> +}
>> +

Re: [Patch-86512]: Subnormal float support in armv7(with -msoft-float) for intrinsics

[Umesh Kalappa]

[-- Attachment #1: Type: text/plain, Size: 3251 bytes --]

Hi Richard,

We tested on the SP and yes the problem persist on the SP too and
attached patch will fix the both SP and DP issues for the  denormal
resultant.
We bootstrapped the compiler ,look ok to us with minimal testing ,

Any floating point test-suite to test for the attached patch ? any
recommendations or inputs  ?

Thank you again
~Umesh


On Mon, Jul 23, 2018 at 3:28 PM, Richard Earnshaw (lists)
<Richard.Earnshaw@arm.com> wrote:
> So why is this only changing the double-precision implementation?
> Surely, a problem like this will normally be common to both SP and DP
> floating-point computations.
>
> R.
>
> On 23/07/18 08:46, Umesh Kalappa wrote:
>> Thank you Wilco for the inputs and we agree that the  fix break down
>> for the case.
>>
>> Meanwhile ,attached patch will take care the inputs and we are testing
>> the patch vigorously ,would you recommended any test-suite out there
>> for the same ?
>>
>> Thank you
>> ~Umesh
>>
>>
>> On Fri, Jul 20, 2018 at 10:04 PM, Wilco Dijkstra <Wilco.Dijkstra@arm.com> wrote:
>>> Umesh Kalappa wrote:
>>>
>>>> We tried some of the normalisation numbers and the fix works and please
>>>> could you help us with the input ,where  if you see that fix breaks down.
>>>
>>> Well try any set of inputs which require normalisation. You'll find these no
>>> longer get normalised and so will get incorrect results. Try basic cases like
>>> 1.0 - 0.75 which I think will return 0.625...
>>>
>>> A basic test would be to run old vs new on a large set of inputs to verify
>>> there aren't any obvious differences.
>>>
>>> Wilco
>>>
>>>
>>> pr86512.patch
>>>
>>>
>>> Index: libgcc/config/arm/ieee754-df.S
>>> ===================================================================
>>> --- libgcc/config/arm/ieee754-df.S   (revision 262850)
>>> +++ libgcc/config/arm/ieee754-df.S   (working copy)
>>> @@ -203,6 +203,7 @@
>>>  #endif
>>>
>>>      @ Determine how to normalize the result.
>>> +    @ if result is denormal i.e (exp)=0,then don't normalise the result,
>>>  LSYM(Lad_p):
>>>      cmp     xh, #0x00100000
>>>      bcc     LSYM(Lad_a)
>>> @@ -235,6 +236,8 @@
>>>
>>>      @ Result must be shifted left and exponent adjusted.
>>>  LSYM(Lad_a):
>>> +    cmp     r4,#0x0
>>> +    beq     LSYM(Lad_e)
>>>      movs    ip, ip, lsl #1
>>>      adcs    xl, xl, xl
>>>      adc     xh, xh, xh
>>> Index: gcc/testsuite/gcc.target/arm/pr86512.c
>>> ===================================================================
>>> --- gcc/testsuite/gcc.target/arm/pr86512.c   (nonexistent)
>>> +++ gcc/testsuite/gcc.target/arm/pr86512.c   (working copy)
>>> @@ -0,0 +1,28 @@
>>> +/* { dg-do run } */
>>> +/* { dg-options "-O0 -msoft-float" } */
>>> +
>>> +#include<stdlib.h>
>>> +#include<stdint.h>
>>> +
>>> +typedef union
>>> +{
>>> +    double d;
>>> +    uint64_t i;
>>> +} u;
>>> +
>>> +int main()
>>> +{
>>> +  u smallestPositiveNormal, smallesPositiveSubnormal, expectedResult, result;
>>> +
>>> +  smallesPositiveSubnormal.i = 1;
>>> +
>>> +  smallestPositiveNormal.i = 0x0010000000000000;
>>> +  expectedResult.i = 0x000fffffffffffff;
>>> +  result.d = smallestPositiveNormal.d - smallesPositiveSubnormal.d;
>>> +
>>> +  if (result.i != expectedResult.i)
>>> +    abort();
>>> +
>>> +  return 0;
>>> +}
>>> +
>

[-- Attachment #2: pr86512.patch --]
[-- Type: application/octet-stream, Size: 2236 bytes --]

Index: libgcc/config/arm/ieee754-sf.S
===================================================================
--- libgcc/config/arm/ieee754-sf.S	(revision 262850)
+++ libgcc/config/arm/ieee754-sf.S	(working copy)
@@ -165,7 +165,10 @@
 	RET
 
 	@ Result must be shifted left and exponent adjusted.
+	@ if result is denormal i.e (exp)=0,then don't normalise the result,
 LSYM(Lad_a):
+	cmp	r2,#0x0
+	beq	LSYM(Lad_e)
 	movs	r1, r1, lsl #1
 	adc	r0, r0, r0
 	tst	r0, #0x00800000
Index: libgcc/config/arm/ieee754-df.S
===================================================================
--- libgcc/config/arm/ieee754-df.S	(revision 262850)
+++ libgcc/config/arm/ieee754-df.S	(working copy)
@@ -234,7 +234,10 @@
 	RETLDM	"r4, r5"
 
 	@ Result must be shifted left and exponent adjusted.
+	@ if result is denormal i.e (exp)=0,then don't normalise the result,
 LSYM(Lad_a):
+	cmp     r4,#0x0
+	beq     LSYM(Lad_e)
 	movs	ip, ip, lsl #1
 	adcs	xl, xl, xl
 	adc	xh, xh, xh
Index: gcc/testsuite/gcc.target/arm/pr86512.c
===================================================================
--- gcc/testsuite/gcc.target/arm/pr86512.c	(nonexistent)
+++ gcc/testsuite/gcc.target/arm/pr86512.c	(working copy)
@@ -0,0 +1,59 @@
+/* { dg-do run } */
+/* { dg-options "-O0 -msoft-float" } */
+
+#include<stdlib.h>
+#include<stdint.h>
+
+typedef union
+{
+    float f;
+    int i;
+} f;
+
+typedef union
+{
+    double d;
+    uint64_t i;
+}d ;
+
+int testFloat()
+{
+  f smallestPositiveNormal, smallesPositiveSubnormal, expectedResult, result;
+
+  smallesPositiveSubnormal.i = 1;
+  smallestPositiveNormal.i =0x00800000;
+  expectedResult.i = 0x007fffff;
+
+  result.f = smallestPositiveNormal.f - smallesPositiveSubnormal.f;
+	
+  if (result.i != expectedResult.i)
+   return 1;
+
+  return 0;
+}
+
+int  testDouble()
+{
+  d smallestPositiveNormal, smallesPositiveSubnormal, expectedResult, result;
+
+  smallesPositiveSubnormal.i = 1;
+  smallestPositiveNormal.i = 0x0010000000000000;
+  expectedResult.i = 0x000fffffffffffff;
+
+  result.d = smallestPositiveNormal.d - smallesPositiveSubnormal.d;
+
+  if (result.i != expectedResult.i)
+   return 1;
+ 
+  return 0;
+}
+
+int main()
+{
+
+  if (testFloat() || testDouble())
+	abort();
+	
+  return 0;
+}
+

Re: [Patch-86512]: Subnormal float support in armv7(with -msoft-float) for intrinsics

[Ramana Radhakrishnan]

On Mon, Jul 23, 2018 at 12:09 PM, Umesh Kalappa
<umesh.kalappa0@gmail.com> wrote:
> Hi Richard,
>
> We tested on the SP and yes the problem persist on the SP too and
> attached patch will fix the both SP and DP issues for the  denormal
> resultant.
> We bootstrapped the compiler ,look ok to us with minimal testing ,

Have you run the full gcc testsuite as you should do if you want to
submit a patch and checked for no regressions ?


Ramana

>
> Any floating point test-suite to test for the attached patch ? any
> recommendations or inputs  ?
>



> Thank you again
> ~Umesh
>
>
> On Mon, Jul 23, 2018 at 3:28 PM, Richard Earnshaw (lists)
> <Richard.Earnshaw@arm.com> wrote:
>> So why is this only changing the double-precision implementation?
>> Surely, a problem like this will normally be common to both SP and DP
>> floating-point computations.
>>
>> R.
>>
>> On 23/07/18 08:46, Umesh Kalappa wrote:
>>> Thank you Wilco for the inputs and we agree that the  fix break down
>>> for the case.
>>>
>>> Meanwhile ,attached patch will take care the inputs and we are testing
>>> the patch vigorously ,would you recommended any test-suite out there
>>> for the same ?
>>>
>>> Thank you
>>> ~Umesh
>>>
>>>
>>> On Fri, Jul 20, 2018 at 10:04 PM, Wilco Dijkstra <Wilco.Dijkstra@arm.com> wrote:
>>>> Umesh Kalappa wrote:
>>>>
>>>>> We tried some of the normalisation numbers and the fix works and please
>>>>> could you help us with the input ,where  if you see that fix breaks down.
>>>>
>>>> Well try any set of inputs which require normalisation. You'll find these no
>>>> longer get normalised and so will get incorrect results. Try basic cases like
>>>> 1.0 - 0.75 which I think will return 0.625...
>>>>
>>>> A basic test would be to run old vs new on a large set of inputs to verify
>>>> there aren't any obvious differences.
>>>>
>>>> Wilco
>>>>
>>>>
>>>> pr86512.patch
>>>>
>>>>
>>>> Index: libgcc/config/arm/ieee754-df.S
>>>> ===================================================================
>>>> --- libgcc/config/arm/ieee754-df.S   (revision 262850)
>>>> +++ libgcc/config/arm/ieee754-df.S   (working copy)
>>>> @@ -203,6 +203,7 @@
>>>>  #endif
>>>>
>>>>      @ Determine how to normalize the result.
>>>> +    @ if result is denormal i.e (exp)=0,then don't normalise the result,
>>>>  LSYM(Lad_p):
>>>>      cmp     xh, #0x00100000
>>>>      bcc     LSYM(Lad_a)
>>>> @@ -235,6 +236,8 @@
>>>>
>>>>      @ Result must be shifted left and exponent adjusted.
>>>>  LSYM(Lad_a):
>>>> +    cmp     r4,#0x0
>>>> +    beq     LSYM(Lad_e)
>>>>      movs    ip, ip, lsl #1
>>>>      adcs    xl, xl, xl
>>>>      adc     xh, xh, xh
>>>> Index: gcc/testsuite/gcc.target/arm/pr86512.c
>>>> ===================================================================
>>>> --- gcc/testsuite/gcc.target/arm/pr86512.c   (nonexistent)
>>>> +++ gcc/testsuite/gcc.target/arm/pr86512.c   (working copy)
>>>> @@ -0,0 +1,28 @@
>>>> +/* { dg-do run } */
>>>> +/* { dg-options "-O0 -msoft-float" } */
>>>> +
>>>> +#include<stdlib.h>
>>>> +#include<stdint.h>
>>>> +
>>>> +typedef union
>>>> +{
>>>> +    double d;
>>>> +    uint64_t i;
>>>> +} u;
>>>> +
>>>> +int main()
>>>> +{
>>>> +  u smallestPositiveNormal, smallesPositiveSubnormal, expectedResult, result;
>>>> +
>>>> +  smallesPositiveSubnormal.i = 1;
>>>> +
>>>> +  smallestPositiveNormal.i = 0x0010000000000000;
>>>> +  expectedResult.i = 0x000fffffffffffff;
>>>> +  result.d = smallestPositiveNormal.d - smallesPositiveSubnormal.d;
>>>> +
>>>> +  if (result.i != expectedResult.i)
>>>> +    abort();
>>>> +
>>>> +  return 0;
>>>> +}
>>>> +
>>

Re: [Patch-86512]: Subnormal float support in armv7(with -msoft-float) for intrinsics

[Wilco Dijkstra]

Umesh Kalappa wrote:

> We tested on the SP and yes the problem persist on the SP too and
> attached patch will fix the both SP and DP issues for the  denormal
> resultant.

The patch now looks correct to me (but I can't approve).

> We bootstrapped the compiler ,look ok to us with minimal testing ,
>
> Any floating point test-suite to test for the attached patch ? any
> recommendations or inputs  ?

Running the GCC regression tests would be required since a bootstrap isn't 
useful for this kind of change. Assuming you use Linux, building and running
GLIBC with the changed GCC would give additional test coverage as it tests
all the math library functions.

I don't know of any IEEE conformance testsuites in the GNU world, which is
why I'm suggesting running some targeted and randomized tests. You could
use the generic soft-float code in libgcc/soft-fp/adddf3.c to compare the outputs.


>>> Index: libgcc/config/arm/ieee754-df.S
>>> ===================================================================
>>> --- libgcc/config/arm/ieee754-df.S   (revision 262850)
>>> +++ libgcc/config/arm/ieee754-df.S   (working copy)
>>> @@ -203,6 +203,7 @@
>>>  #endif
>>>
>>>      @ Determine how to normalize the result.
>>> +    @ if result is denormal i.e (exp)=0,then don't normalise the result,

Use a standard sentence here, eg. like:

If exp is zero and the mantissa unnormalized, return a denormal.

Wilco
    

Re: [Patch-86512]: Subnormal float support in armv7(with -msoft-float) for intrinsics

[Umesh Kalappa]

[-- Attachment #1: Type: text/plain, Size: 2215 bytes --]

Thank you All for the suggestions  and we tried runing the GCC
testsuite and found that no regression with the fix and also ran the
our regressions base for conformance with no regress.

Is ok for commit with below  Changelog ?
+++ libgcc/ChangeLog    (working copy)
@@ -1,3 +1,9 @@
+2018-07-18  Umesh Kalappa <umesh.kalappa0@gmail.com>
+
+       PR libgcc/86512
+       * config/arm/ieee754-df.S :Don't normalise the denormal result.
+       * config/arm/ieee754-sf.S:Likewise.
+
+
+++ gcc/testsuite/ChangeLog     (working copy)
@@ -1,3 +1,8 @@
+2018-07-18  Umesh Kalappa <umesh.kalappa0@gmail.com>
+
+       PR libgcc/86512
+       * gcc.target/arm/pr86512.c :New test.
+

On Mon, Jul 23, 2018 at 5:24 PM, Wilco Dijkstra <Wilco.Dijkstra@arm.com> wrote:
> Umesh Kalappa wrote:
>
>> We tested on the SP and yes the problem persist on the SP too and
>> attached patch will fix the both SP and DP issues for the  denormal
>> resultant.
>
> The patch now looks correct to me (but I can't approve).
>
>> We bootstrapped the compiler ,look ok to us with minimal testing ,
>>
>> Any floating point test-suite to test for the attached patch ? any
>> recommendations or inputs  ?
>
> Running the GCC regression tests would be required since a bootstrap isn't
> useful for this kind of change. Assuming you use Linux, building and running
> GLIBC with the changed GCC would give additional test coverage as it tests
> all the math library functions.
>
> I don't know of any IEEE conformance testsuites in the GNU world, which is
> why I'm suggesting running some targeted and randomized tests. You could
> use the generic soft-float code in libgcc/soft-fp/adddf3.c to compare the outputs.
>
>
>>>> Index: libgcc/config/arm/ieee754-df.S
>>>> ===================================================================
>>>> --- libgcc/config/arm/ieee754-df.S   (revision 262850)
>>>> +++ libgcc/config/arm/ieee754-df.S   (working copy)
>>>> @@ -203,6 +203,7 @@
>>>>  #endif
>>>>
>>>>      @ Determine how to normalize the result.
>>>> +    @ if result is denormal i.e (exp)=0,then don't normalise the result,
>
> Use a standard sentence here, eg. like:
>
> If exp is zero and the mantissa unnormalized, return a denormal.
>
> Wilco
>

[-- Attachment #2: pr86512.patch --]
[-- Type: application/octet-stream, Size: 2232 bytes --]

Index: libgcc/config/arm/ieee754-sf.S
===================================================================
--- libgcc/config/arm/ieee754-sf.S	(revision 262850)
+++ libgcc/config/arm/ieee754-sf.S	(working copy)
@@ -165,7 +165,10 @@
 	RET
 
 	@ Result must be shifted left and exponent adjusted.
+	@ If exp is zero and the mantissa unnormalized, return a denormal.
 LSYM(Lad_a):
+	cmp	r2,#0x0
+	beq	LSYM(Lad_e)
 	movs	r1, r1, lsl #1
 	adc	r0, r0, r0
 	tst	r0, #0x00800000
Index: libgcc/config/arm/ieee754-df.S
===================================================================
--- libgcc/config/arm/ieee754-df.S	(revision 262850)
+++ libgcc/config/arm/ieee754-df.S	(working copy)
@@ -234,7 +234,10 @@
 	RETLDM	"r4, r5"
 
 	@ Result must be shifted left and exponent adjusted.
+	@ If exp is zero and the mantissa unnormalized, return a denormal.
 LSYM(Lad_a):
+	cmp     r4,#0x0
+	beq     LSYM(Lad_e)
 	movs	ip, ip, lsl #1
 	adcs	xl, xl, xl
 	adc	xh, xh, xh
Index: gcc/testsuite/gcc.target/arm/pr86512.c
===================================================================
--- gcc/testsuite/gcc.target/arm/pr86512.c	(nonexistent)
+++ gcc/testsuite/gcc.target/arm/pr86512.c	(working copy)
@@ -0,0 +1,59 @@
+/* { dg-do run } */
+/* { dg-options "-O0 -msoft-float" } */
+
+#include<stdlib.h>
+#include<stdint.h>
+
+typedef union
+{
+    float f;
+    int i;
+} f;
+
+typedef union
+{
+    double d;
+    uint64_t i;
+}d ;
+
+int testFloat()
+{
+  f smallestPositiveNormal, smallesPositiveSubnormal, expectedResult, result;
+
+  smallesPositiveSubnormal.i = 1;
+  smallestPositiveNormal.i =0x00800000;
+  expectedResult.i = 0x007fffff;
+
+  result.f = smallestPositiveNormal.f - smallesPositiveSubnormal.f;
+	
+  if (result.i != expectedResult.i)
+   return 1;
+
+  return 0;
+}
+
+int  testDouble()
+{
+  d smallestPositiveNormal, smallesPositiveSubnormal, expectedResult, result;
+
+  smallesPositiveSubnormal.i = 1;
+  smallestPositiveNormal.i = 0x0010000000000000;
+  expectedResult.i = 0x000fffffffffffff;
+
+  result.d = smallestPositiveNormal.d - smallesPositiveSubnormal.d;
+
+  if (result.i != expectedResult.i)
+   return 1;
+ 
+  return 0;
+}
+
+int main()
+{
+
+  if (testFloat() || testDouble())
+	abort();
+	
+  return 0;
+}
+

Re: [Patch-86512]: Subnormal float support in armv7(with -msoft-float) for intrinsics

[Umesh Kalappa]

Hi,

Any more suggestions or comments on the patch ?

Thank you
~Umesh

On Tue, Jul 24, 2018, 2:08 PM Umesh Kalappa <umesh.kalappa0@gmail.com>
wrote:

> Thank you All for the suggestions  and we tried runing the GCC
> testsuite and found that no regression with the fix and also ran the
> our regressions base for conformance with no regress.
>
> Is ok for commit with below  Changelog ?
> +++ libgcc/ChangeLog    (working copy)
> @@ -1,3 +1,9 @@
> +2018-07-18  Umesh Kalappa <umesh.kalappa0@gmail.com>
> +
> +       PR libgcc/86512
> +       * config/arm/ieee754-df.S :Don't normalise the denormal result.
> +       * config/arm/ieee754-sf.S:Likewise.
> +
> +
> +++ gcc/testsuite/ChangeLog     (working copy)
> @@ -1,3 +1,8 @@
> +2018-07-18  Umesh Kalappa <umesh.kalappa0@gmail.com>
> +
> +       PR libgcc/86512
> +       * gcc.target/arm/pr86512.c :New test.
> +
>
> On Mon, Jul 23, 2018 at 5:24 PM, Wilco Dijkstra <Wilco.Dijkstra@arm.com>
> wrote:
> > Umesh Kalappa wrote:
> >
> >> We tested on the SP and yes the problem persist on the SP too and
> >> attached patch will fix the both SP and DP issues for the  denormal
> >> resultant.
> >
> > The patch now looks correct to me (but I can't approve).
> >
> >> We bootstrapped the compiler ,look ok to us with minimal testing ,
> >>
> >> Any floating point test-suite to test for the attached patch ? any
> >> recommendations or inputs  ?
> >
> > Running the GCC regression tests would be required since a bootstrap
> isn't
> > useful for this kind of change. Assuming you use Linux, building and
> running
> > GLIBC with the changed GCC would give additional test coverage as it
> tests
> > all the math library functions.
> >
> > I don't know of any IEEE conformance testsuites in the GNU world, which
> is
> > why I'm suggesting running some targeted and randomized tests. You could
> > use the generic soft-float code in libgcc/soft-fp/adddf3.c to compare
> the outputs.
> >
> >
> >>>> Index: libgcc/config/arm/ieee754-df.S
> >>>> ===================================================================
> >>>> --- libgcc/config/arm/ieee754-df.S   (revision 262850)
> >>>> +++ libgcc/config/arm/ieee754-df.S   (working copy)
> >>>> @@ -203,6 +203,7 @@
> >>>>  #endif
> >>>>
> >>>>      @ Determine how to normalize the result.
> >>>> +    @ if result is denormal i.e (exp)=0,then don't normalise the
> result,
> >
> > Use a standard sentence here, eg. like:
> >
> > If exp is zero and the mantissa unnormalized, return a denormal.
> >
> > Wilco
> >
>

Re: [Patch-86512]: Subnormal float support in armv7(with -msoft-float) for intrinsics

[Nicolas Pitre]

[-- Attachment #1: Type: text/plain, Size: 538 bytes --]

Umesh Kalappa wrote:

> Any more suggestions or comments on the patch ?

The patch is suboptimal as it introduces 2 additional instructions in a 
fairly common path for a branch that is very unlikely to be taken in 
practice.

I'm therefore proposing this alternative patch to fix the issue in an 
optimal way. I'm also using this opportunity to update my email address 
as the one currently in those files has been obsolete for more than 10 
years at this point, in the hope that I get notified of similar issues 
directly in the future.

[-- Attachment #2: Type: text/plain, Size: 2262 bytes --]

diff --git a/libgcc/ChangeLog b/libgcc/ChangeLog
index c13bf4cb2f6..c19d05c8a2e 100644
--- a/libgcc/ChangeLog
+++ b/libgcc/ChangeLog
@@ -1,3 +1,9 @@
+2018-07-26  Nicolas Pitre <nico@fluxnic.net>
+
+	* config/arm/ieee754-df.S: Don't shortcut denormal handling when
+	exponent goes negative. Update my email address.
+	* config/arm/ieee754-sf.S: Likewise.
+
 2018-07-05  James Clarke  <jrtc27@jrtc27.com>
 
 	* configure: Regenerated.
diff --git a/libgcc/config/arm/ieee754-df.S b/libgcc/config/arm/ieee754-df.S
index 8741aa99245..ee7a9835394 100644
--- a/libgcc/config/arm/ieee754-df.S
+++ b/libgcc/config/arm/ieee754-df.S
@@ -1,7 +1,7 @@
 /* ieee754-df.S double-precision floating point support for ARM
 
    Copyright (C) 2003-2018 Free Software Foundation, Inc.
-   Contributed by Nicolas Pitre (nico@cam.org)
+   Contributed by Nicolas Pitre (nico@fluxnic.net)
 
    This file is free software; you can redistribute it and/or modify it
    under the terms of the GNU General Public License as published by the
@@ -238,9 +238,10 @@ LSYM(Lad_a):
 	movs	ip, ip, lsl #1
 	adcs	xl, xl, xl
 	adc	xh, xh, xh
-	tst	xh, #0x00100000
-	sub	r4, r4, #1
-	bne	LSYM(Lad_e)
+	subs	r4, r4, #1
+	do_it	hs
+	tsths	xh, #0x00100000
+	bhi	LSYM(Lad_e)
 
 	@ No rounding necessary since ip will always be 0 at this point.
 LSYM(Lad_l):
diff --git a/libgcc/config/arm/ieee754-sf.S b/libgcc/config/arm/ieee754-sf.S
index d80d5e9080c..640c97ed550 100644
--- a/libgcc/config/arm/ieee754-sf.S
+++ b/libgcc/config/arm/ieee754-sf.S
@@ -1,7 +1,7 @@
 /* ieee754-sf.S single-precision floating point support for ARM
 
    Copyright (C) 2003-2018 Free Software Foundation, Inc.
-   Contributed by Nicolas Pitre (nico@cam.org)
+   Contributed by Nicolas Pitre (nico@fluxnic.net)
 
    This file is free software; you can redistribute it and/or modify it
    under the terms of the GNU General Public License as published by the
@@ -168,9 +168,10 @@ LSYM(Lad_e):
 LSYM(Lad_a):
 	movs	r1, r1, lsl #1
 	adc	r0, r0, r0
-	tst	r0, #0x00800000
-	sub	r2, r2, #1
-	bne	LSYM(Lad_e)
+	subs	r2, r2, #1
+	do_it	hs
+	tsths	r0, #0x00800000
+	bhi	LSYM(Lad_e)
 	
 	@ No rounding necessary since r1 will always be 0 at this point.
 LSYM(Lad_l):

Re: [Patch-86512]: Subnormal float support in armv7(with -msoft-float) for intrinsics

[Wilco Dijkstra]

Hi Nicolas,

I think your patch doesn't quite work as expected:

@@ -238,9 +238,10 @@ LSYM(Lad_a):
 	movs	ip, ip, lsl #1
 	adcs	xl, xl, xl
 	adc	xh, xh, xh
-	tst	xh, #0x00100000
-	sub	r4, r4, #1
-	bne	LSYM(Lad_e)
+	subs	r4, r4, #1
+	do_it	hs
+	tsths	xh, #0x00100000
+	bhi	LSYM(Lad_e)

If the exponent in r4 is zero, the carry bit will be clear, so we don't execute the tsths
and fallthrough (the denormal will be normalized and then denormalized again, but
that's so rare it doesn't matter really).

However if r4 is non-zero, the carry will be set, and the tsths will be executed. This
clears the carry and sets the Z flag based on bit 20. We will now also always
fallthrough rather than take the branch if bit 20 is non-zero. This may still give the
correct answer, however it would add considerable extra overhead... I think using
a cmp rather than tst would work.

Wilco

Re: [Patch-86512]: Subnormal float support in armv7(with -msoft-float) for intrinsics

[Nicolas Pitre]

On Fri, 27 Jul 2018, Wilco Dijkstra wrote:

> Hi Nicolas,
> 
> I think your patch doesn't quite work as expected:
> 
> @@ -238,9 +238,10 @@ LSYM(Lad_a):
>  	movs	ip, ip, lsl #1
>  	adcs	xl, xl, xl
>  	adc	xh, xh, xh
> -	tst	xh, #0x00100000
> -	sub	r4, r4, #1
> -	bne	LSYM(Lad_e)
> +	subs	r4, r4, #1
> +	do_it	hs
> +	tsths	xh, #0x00100000
> +	bhi	LSYM(Lad_e)
> 
> If the exponent in r4 is zero, the carry bit will be clear, so we don't execute the tsths
> and fallthrough (the denormal will be normalized and then denormalized again, but
> that's so rare it doesn't matter really).

And that's what is intended.

> However if r4 is non-zero, the carry will be set, and the tsths will be executed. This
> clears the carry and sets the Z flag based on bit 20.

No, not at all. The carry is not affected. And that's the point of the 
tst instruction here rather than a cmp: it sets the N and Z flags but 
leaves C alone as there is no shifter involved.


Nicolas

Re: [Patch-86512]: Subnormal float support in armv7(with -msoft-float) for intrinsics

[Nicolas Pitre]

On Fri, 27 Jul 2018, Nicolas Pitre wrote:

> On Fri, 27 Jul 2018, Wilco Dijkstra wrote:
> 
> > Hi Nicolas,
> > 
> > I think your patch doesn't quite work as expected:
> > 
> > @@ -238,9 +238,10 @@ LSYM(Lad_a):
> >  	movs	ip, ip, lsl #1
> >  	adcs	xl, xl, xl
> >  	adc	xh, xh, xh
> > -	tst	xh, #0x00100000
> > -	sub	r4, r4, #1
> > -	bne	LSYM(Lad_e)
> > +	subs	r4, r4, #1
> > +	do_it	hs
> > +	tsths	xh, #0x00100000
> > +	bhi	LSYM(Lad_e)
> > 
> > If the exponent in r4 is zero, the carry bit will be clear, so we don't execute the tsths
> > and fallthrough (the denormal will be normalized and then denormalized again, but
> > that's so rare it doesn't matter really).
> 
> And that's what is intended.
> 
> > However if r4 is non-zero, the carry will be set, and the tsths will be executed. This
> > clears the carry and sets the Z flag based on bit 20.
> 
> No, not at all. The carry is not affected. And that's the point of the 
> tst instruction here rather than a cmp: it sets the N and Z flags but 
> leaves C alone as there is no shifter involved.

That being said, a cmp as you suggested would have worked too (with the 
bhi changed to bhs). But the patch as is produces the same behavior.


Nicolas

Re: [Patch-86512]: Subnormal float support in armv7(with -msoft-float) for intrinsics

[Wilco Dijkstra]

Nicolas Pitre wrote:

>> However if r4 is non-zero, the carry will be set, and the tsths will be executed. This
>> clears the carry and sets the Z flag based on bit 20.
>
> No, not at all. The carry is not affected. And that's the point of the 
> tst instruction here rather than a cmp: it sets the N and Z flags but 
> leaves C alone as there is no shifter involved.

No, the carry is always set by logical operations with a shifted immediate. It is only
unchanged if the immediate uses a zero rotate. So any shifted immediate > 255
will set the carry. This is detailed in the Arm Architecture Reference Manual, eg.
see the pseudo code for A32ExpandImm_C in LSL (immediate).

Wilco

Re: [Patch-86512]: Subnormal float support in armv7(with -msoft-float) for intrinsics

[Nicolas Pitre]

[-- Attachment #1: Type: text/plain, Size: 790 bytes --]

On Fri, 27 Jul 2018, Wilco Dijkstra wrote:

> Nicolas Pitre wrote:
> 
> >> However if r4 is non-zero, the carry will be set, and the tsths will be executed. This
> >> clears the carry and sets the Z flag based on bit 20.
> >
> > No, not at all. The carry is not affected. And that's the point of the 
> > tst instruction here rather than a cmp: it sets the N and Z flags but 
> > leaves C alone as there is no shifter involved.
> 
> No, the carry is always set by logical operations with a shifted immediate. It is only
> unchanged if the immediate uses a zero rotate. So any shifted immediate > 255
> will set the carry.

Holy cow !!!

Who knows all the bugs I must have created in the past 25 years having 
missed this particular subtlety.

Here's the updated patch with your suggestion.

[-- Attachment #2: Type: text/plain, Size: 2269 bytes --]

diff --git a/libgcc/ChangeLog b/libgcc/ChangeLog
index c13bf4cb2f6..c19d05c8a2e 100644
--- a/libgcc/ChangeLog
+++ b/libgcc/ChangeLog
@@ -1,3 +1,9 @@
+2018-07-26  Nicolas Pitre <nico@fluxnic.net>
+
+	* config/arm/ieee754-df.S: Don't shortcut denormal handling when
+	exponent goes negative. Update my email address.
+	* config/arm/ieee754-sf.S: Likewise.
+
 2018-07-05  James Clarke  <jrtc27@jrtc27.com>
 
 	* configure: Regenerated.
diff --git a/libgcc/config/arm/ieee754-df.S b/libgcc/config/arm/ieee754-df.S
index 8741aa99245..9a1b2dd2a4e 100644
--- a/libgcc/config/arm/ieee754-df.S
+++ b/libgcc/config/arm/ieee754-df.S
@@ -1,7 +1,7 @@
 /* ieee754-df.S double-precision floating point support for ARM
 
    Copyright (C) 2003-2018 Free Software Foundation, Inc.
-   Contributed by Nicolas Pitre (nico@cam.org)
+   Contributed by Nicolas Pitre (nico@fluxnic.net)
 
    This file is free software; you can redistribute it and/or modify it
    under the terms of the GNU General Public License as published by the
@@ -238,9 +238,10 @@ LSYM(Lad_a):
 	movs	ip, ip, lsl #1
 	adcs	xl, xl, xl
 	adc	xh, xh, xh
-	tst	xh, #0x00100000
-	sub	r4, r4, #1
-	bne	LSYM(Lad_e)
+	subs	r4, r4, #1
+	do_it	hs
+	cmphs	xh, #0x00100000
+	bhs	LSYM(Lad_e)
 
 	@ No rounding necessary since ip will always be 0 at this point.
 LSYM(Lad_l):
diff --git a/libgcc/config/arm/ieee754-sf.S b/libgcc/config/arm/ieee754-sf.S
index d80d5e9080c..b8a81279a3c 100644
--- a/libgcc/config/arm/ieee754-sf.S
+++ b/libgcc/config/arm/ieee754-sf.S
@@ -1,7 +1,7 @@
 /* ieee754-sf.S single-precision floating point support for ARM
 
    Copyright (C) 2003-2018 Free Software Foundation, Inc.
-   Contributed by Nicolas Pitre (nico@cam.org)
+   Contributed by Nicolas Pitre (nico@fluxnic.net)
 
    This file is free software; you can redistribute it and/or modify it
    under the terms of the GNU General Public License as published by the
@@ -168,10 +168,11 @@ LSYM(Lad_e):
 LSYM(Lad_a):
 	movs	r1, r1, lsl #1
 	adc	r0, r0, r0
-	tst	r0, #0x00800000
-	sub	r2, r2, #1
-	bne	LSYM(Lad_e)
-	
+	subs	r2, r2, #1
+	do_it	hs
+	cmphs	r0, #0x00800000
+	bhs	LSYM(Lad_e)
+
 	@ No rounding necessary since r1 will always be 0 at this point.
 LSYM(Lad_l):
 

Re: [Patch-86512]: Subnormal float support in armv7(with -msoft-float) for intrinsics

[Richard Earnshaw (lists)]

On 27/07/18 17:45, Nicolas Pitre wrote:
> On Fri, 27 Jul 2018, Wilco Dijkstra wrote:
> 
>> Nicolas Pitre wrote:
>>
>>>> However if r4 is non-zero, the carry will be set, and the tsths will be executed. This
>>>> clears the carry and sets the Z flag based on bit 20.
>>>
>>> No, not at all. The carry is not affected. And that's the point of the 
>>> tst instruction here rather than a cmp: it sets the N and Z flags but 
>>> leaves C alone as there is no shifter involved.
>>
>> No, the carry is always set by logical operations with a shifted immediate. It is only
>> unchanged if the immediate uses a zero rotate. So any shifted immediate > 255
>> will set the carry.
> 
> Holy cow !!!
> 
> Who knows all the bugs I must have created in the past 25 years having 
> missed this particular subtlety.
> 
> Here's the updated patch with your suggestion.
> 

Thanks.

Committed.

R.

> 
> fix.patch
> 
> 
> diff --git a/libgcc/ChangeLog b/libgcc/ChangeLog
> index c13bf4cb2f6..c19d05c8a2e 100644
> --- a/libgcc/ChangeLog
> +++ b/libgcc/ChangeLog
> @@ -1,3 +1,9 @@
> +2018-07-26  Nicolas Pitre <nico@fluxnic.net>
> +
> +	* config/arm/ieee754-df.S: Don't shortcut denormal handling when
> +	exponent goes negative. Update my email address.
> +	* config/arm/ieee754-sf.S: Likewise.
> +
>  2018-07-05  James Clarke  <jrtc27@jrtc27.com>
>  
>  	* configure: Regenerated.
> diff --git a/libgcc/config/arm/ieee754-df.S b/libgcc/config/arm/ieee754-df.S
> index 8741aa99245..9a1b2dd2a4e 100644
> --- a/libgcc/config/arm/ieee754-df.S
> +++ b/libgcc/config/arm/ieee754-df.S
> @@ -1,7 +1,7 @@
>  /* ieee754-df.S double-precision floating point support for ARM
>  
>     Copyright (C) 2003-2018 Free Software Foundation, Inc.
> -   Contributed by Nicolas Pitre (nico@cam.org)
> +   Contributed by Nicolas Pitre (nico@fluxnic.net)
>  
>     This file is free software; you can redistribute it and/or modify it
>     under the terms of the GNU General Public License as published by the
> @@ -238,9 +238,10 @@ LSYM(Lad_a):
>  	movs	ip, ip, lsl #1
>  	adcs	xl, xl, xl
>  	adc	xh, xh, xh
> -	tst	xh, #0x00100000
> -	sub	r4, r4, #1
> -	bne	LSYM(Lad_e)
> +	subs	r4, r4, #1
> +	do_it	hs
> +	cmphs	xh, #0x00100000
> +	bhs	LSYM(Lad_e)
>  
>  	@ No rounding necessary since ip will always be 0 at this point.
>  LSYM(Lad_l):
> diff --git a/libgcc/config/arm/ieee754-sf.S b/libgcc/config/arm/ieee754-sf.S
> index d80d5e9080c..b8a81279a3c 100644
> --- a/libgcc/config/arm/ieee754-sf.S
> +++ b/libgcc/config/arm/ieee754-sf.S
> @@ -1,7 +1,7 @@
>  /* ieee754-sf.S single-precision floating point support for ARM
>  
>     Copyright (C) 2003-2018 Free Software Foundation, Inc.
> -   Contributed by Nicolas Pitre (nico@cam.org)
> +   Contributed by Nicolas Pitre (nico@fluxnic.net)
>  
>     This file is free software; you can redistribute it and/or modify it
>     under the terms of the GNU General Public License as published by the
> @@ -168,10 +168,11 @@ LSYM(Lad_e):
>  LSYM(Lad_a):
>  	movs	r1, r1, lsl #1
>  	adc	r0, r0, r0
> -	tst	r0, #0x00800000
> -	sub	r2, r2, #1
> -	bne	LSYM(Lad_e)
> -	
> +	subs	r2, r2, #1
> +	do_it	hs
> +	cmphs	r0, #0x00800000
> +	bhs	LSYM(Lad_e)
> +
>  	@ No rounding necessary since r1 will always be 0 at this point.
>  LSYM(Lad_l):
>  
> 

[Patch-86512]: Subnormal float support in armv7(with -msoft-float) for intrinsics

[Umesh Kalappa]

[-- Attachment #1: Type: text/plain, Size: 2524 bytes --]

Hi Nagy/Ramana,

Please help us to review the attached patch and do let me know your comments .

No regress in the  gcc.target  suite for arm target.

Thank you
~Umesh

On Tue, Jul 17, 2018 at 4:01 PM, Umesh Kalappa <umesh.kalappa0@gmail.com> wrote:
> Will do, thanks.
> Thanks
>
> On Tue, Jul 17, 2018, 3:24 PM Ramana Radhakrishnan
> <ramana.gcc@googlemail.com> wrote:
>>
>> On Tue, Jul 17, 2018 at 10:41 AM, Umesh Kalappa
>> <umesh.kalappa0@gmail.com> wrote:
>> > Hi Nagy,
>> >
>> > Please  help us with your comments on the attached patch for the issue
>> > (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86512)
>> >
>> > Thank you and waiting for your inputs on the same.
>>
>>
>> Patches should be sent to gcc-patches@gcc.gnu.org with a clear
>> description of what the patch hopes to
>> achieve and why this is correct, how was it tested and if a regression
>> test needs to be added - add one please.
>> Please read https://gcc.gnu.org/contribute.html before sending a patch.
>>
>> This is the wrong list to send patches to.
>>
>> regards
>> Ramana
>> > ~Umesh
>> >
>> > On Fri, Jul 13, 2018 at 1:22 PM, Umesh Kalappa
>> > <umesh.kalappa0@gmail.com> wrote:
>> >> Thank you and issue  raised at
>> >> gcc-patches@gcc.gnu.org
>> >>
>> >> ~Umesh
>> >>
>> >> On Thu, Jul 12, 2018 at 9:33 PM, Szabolcs Nagy <szabolcs.nagy@arm.com>
>> >> wrote:
>> >>> On 12/07/18 16:20, Umesh Kalappa wrote:
>> >>>>
>> >>>> Hi everyone,
>> >>>>
>> >>>> we have our source base ,that was compiled for armv7 on gcc8.1 with
>> >>>> soft-float and for following input
>> >>>>
>> >>>> a=0x0010000000000000
>> >>>> b=0x0000000000000001
>> >>>>
>> >>>>   result = a - b ;
>> >>>>
>> >>>> we are getting the result as "0x000ffffffffffffe" and with
>> >>>> -mhard-float (disabled the flush to zero mode ) we are getting the
>> >>>> result as ""0x000fffffffffffff" as expected.
>> >>>>
>> >>>
>> >>> please submit it as a bug report to bugzilla
>> >>>
>> >>>
>> >>>> while debugging the soft-float code,we see that ,the compiler calls
>> >>>> the intrinsic "__aeabi_dsub" with arm calling conventions i.e passing
>> >>>> "a" in r0 and r1 registers and respectively for "b".
>> >>>>
>> >>>> we are investigating the routine "__aeabi_dsub" that comes from
>> >>>> libgcc
>> >>>> for incorrect result  and meanwhile we would like to know that
>> >>>>
>> >>>> a)do libgcc routines/intrinsic for float operations support or
>> >>>> consider the subnormal values ? ,if so how we can enable the same.
>> >>>>
>> >>>> Thank you
>> >>>> ~Umesh
>> >>>>
>> >>>

[-- Attachment #2: pr86512.patch --]
[-- Type: application/octet-stream, Size: 2084 bytes --]

Index: gcc/testsuite/ChangeLog
===================================================================
--- gcc/testsuite/ChangeLog	(revision 262850)
+++ gcc/testsuite/ChangeLog	(working copy)
@@ -1,3 +1,8 @@
+2018-07-18  Umesh Kalappa <umesh.kalappa0@gmail.com>
+
+	PR libgcc/86512
+	* gcc.target/arm/pr86512.c :New test.
+
 2018-07-18  Richard Biener  <rguenther@suse.de>
 
 	PR debug/86523
Index: gcc/testsuite/gcc.target/arm/pr86512.c
===================================================================
--- gcc/testsuite/gcc.target/arm/pr86512.c	(nonexistent)
+++ gcc/testsuite/gcc.target/arm/pr86512.c	(working copy)
@@ -0,0 +1,30 @@
+/* { dg-do run } */
+/* { dg-options "-O0 -msoft-float" } */
+
+#include<stdlib.h>
+#include<stdint.h>
+#define PRIx64    "llx"
+
+typedef union
+{
+    double d;
+    uint64_t i;
+} u;
+
+int main()
+{
+  u smallestPositiveNormal, smallesPositiveSubnormal, expectedResult, result;
+
+  smallesPositiveSubnormal.i = 1;
+
+  smallestPositiveNormal.i = 0x0010000000000000;
+  expectedResult.i = 0x000fffffffffffff;
+  result.d = smallestPositiveNormal.d - smallesPositiveSubnormal.d;
+
+
+  if (result.i != expectedResult.i)
+	abort();
+	
+  return 0;
+}
+
Index: libgcc/ChangeLog
===================================================================
--- libgcc/ChangeLog	(revision 262850)
+++ libgcc/ChangeLog	(working copy)
@@ -1,3 +1,9 @@
+2018-07-18  Umesh Kalappa <umesh.kalappa0@gmail.com>
+	
+	PR libgcc/86512 
+	* config/arm/ieee754-df.S :Don't normalise the denormal result.
+
+
 2018-07-05  James Clarke  <jrtc27@jrtc27.com>
 
 	* configure: Regenerated.
Index: libgcc/config/arm/ieee754-df.S
===================================================================
--- libgcc/config/arm/ieee754-df.S	(revision 262850)
+++ libgcc/config/arm/ieee754-df.S	(working copy)
@@ -203,8 +203,11 @@
 #endif
 
 	@ Determine how to normalize the result.
+	@ if result is denormal i.e (exp)=0,then don't normalise the result,
 LSYM(Lad_p):
 	cmp	xh, #0x00100000
+	blt	LSYM(Lad_e)
+	cmp	xh, #0x00100000
 	bcc	LSYM(Lad_a)
 	cmp	xh, #0x00200000
 	bcc	LSYM(Lad_e)

Re: [Patch-86512]: Subnormal float support in armv7(with -msoft-float) for intrinsics

[Umesh Kalappa]

Reminder !!!

~Umesh


On Wed, Jul 18, 2018 at 6:01 PM, Umesh Kalappa <umesh.kalappa0@gmail.com> wrote:
> Hi Nagy/Ramana,
>
> Please help us to review the attached patch and do let me know your comments .
>
> No regress in the  gcc.target  suite for arm target.
>
> Thank you
> ~Umesh
>
> On Tue, Jul 17, 2018 at 4:01 PM, Umesh Kalappa <umesh.kalappa0@gmail.com> wrote:
>> Will do, thanks.
>> Thanks
>>
>> On Tue, Jul 17, 2018, 3:24 PM Ramana Radhakrishnan
>> <ramana.gcc@googlemail.com> wrote:
>>>
>>> On Tue, Jul 17, 2018 at 10:41 AM, Umesh Kalappa
>>> <umesh.kalappa0@gmail.com> wrote:
>>> > Hi Nagy,
>>> >
>>> > Please  help us with your comments on the attached patch for the issue
>>> > (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86512)
>>> >
>>> > Thank you and waiting for your inputs on the same.
>>>
>>>
>>> Patches should be sent to gcc-patches@gcc.gnu.org with a clear
>>> description of what the patch hopes to
>>> achieve and why this is correct, how was it tested and if a regression
>>> test needs to be added - add one please.
>>> Please read https://gcc.gnu.org/contribute.html before sending a patch.
>>>
>>> This is the wrong list to send patches to.
>>>
>>> regards
>>> Ramana
>>> > ~Umesh
>>> >
>>> > On Fri, Jul 13, 2018 at 1:22 PM, Umesh Kalappa
>>> > <umesh.kalappa0@gmail.com> wrote:
>>> >> Thank you and issue  raised at
>>> >> gcc-patches@gcc.gnu.org
>>> >>
>>> >> ~Umesh
>>> >>
>>> >> On Thu, Jul 12, 2018 at 9:33 PM, Szabolcs Nagy <szabolcs.nagy@arm.com>
>>> >> wrote:
>>> >>> On 12/07/18 16:20, Umesh Kalappa wrote:
>>> >>>>
>>> >>>> Hi everyone,
>>> >>>>
>>> >>>> we have our source base ,that was compiled for armv7 on gcc8.1 with
>>> >>>> soft-float and for following input
>>> >>>>
>>> >>>> a=0x0010000000000000
>>> >>>> b=0x0000000000000001
>>> >>>>
>>> >>>>   result = a - b ;
>>> >>>>
>>> >>>> we are getting the result as "0x000ffffffffffffe" and with
>>> >>>> -mhard-float (disabled the flush to zero mode ) we are getting the
>>> >>>> result as ""0x000fffffffffffff" as expected.
>>> >>>>
>>> >>>
>>> >>> please submit it as a bug report to bugzilla
>>> >>>
>>> >>>
>>> >>>> while debugging the soft-float code,we see that ,the compiler calls
>>> >>>> the intrinsic "__aeabi_dsub" with arm calling conventions i.e passing
>>> >>>> "a" in r0 and r1 registers and respectively for "b".
>>> >>>>
>>> >>>> we are investigating the routine "__aeabi_dsub" that comes from
>>> >>>> libgcc
>>> >>>> for incorrect result  and meanwhile we would like to know that
>>> >>>>
>>> >>>> a)do libgcc routines/intrinsic for float operations support or
>>> >>>> consider the subnormal values ? ,if so how we can enable the same.
>>> >>>>
>>> >>>> Thank you
>>> >>>> ~Umesh
>>> >>>>
>>> >>>

Re: [Patch-86512]: Subnormal float support in armv7(with -msoft-float) for intrinsics

[Szabolcs Nagy]

On 18/07/18 13:31, Umesh Kalappa wrote:
> Hi Nagy/Ramana,
> 
> Please help us to review the attached patch and do let me know your comments .
> 
> No regress in the  gcc.target  suite for arm target.
> 

have you submitted a copyright assignment form yet?
https://gcc.gnu.org/contribute.html

> Index: gcc/testsuite/ChangeLog
> ===================================================================
> --- gcc/testsuite/ChangeLog	(revision 262850)
> +++ gcc/testsuite/ChangeLog	(working copy)
> @@ -1,3 +1,8 @@
> +2018-07-18  Umesh Kalappa<umesh.kalappa0@gmail.com>
> +
> +	PR libgcc/86512
> +	* gcc.target/arm/pr86512.c :New test.
> +

normally changelog should be part of the mail message and
not included in the diff as it will conflict with other changes
and the whitespace formatting around : and before the email
address is wrong.

> Index: gcc/testsuite/gcc.target/arm/pr86512.c
> ===================================================================
> --- gcc/testsuite/gcc.target/arm/pr86512.c	(nonexistent)
> +++ gcc/testsuite/gcc.target/arm/pr86512.c	(working copy)
> @@ -0,0 +1,30 @@
> +/* { dg-do run } */
> +/* { dg-options "-O0 -msoft-float" } */
> +
> +#include<stdlib.h>
> +#include<stdint.h>
> +#define PRIx64    "llx"

this macro is not needed.

> +
> +typedef union
> +{
> +    double d;
> +    uint64_t i;
> +} u;
> +
> +int main()
> +{
> +  u smallestPositiveNormal, smallesPositiveSubnormal, expectedResult, result;
> +
> +  smallesPositiveSubnormal.i = 1;
> +
> +  smallestPositiveNormal.i = 0x0010000000000000;
> +  expectedResult.i = 0x000fffffffffffff;
> +  result.d = smallestPositiveNormal.d - smallesPositiveSubnormal.d;
> +
> +
> +  if (result.i != expectedResult.i)
> +	abort();
> +	
> +  return 0;
> +}
> +
> Index: libgcc/ChangeLog
> ===================================================================
> --- libgcc/ChangeLog	(revision 262850)
> +++ libgcc/ChangeLog	(working copy)
> @@ -1,3 +1,9 @@
> +2018-07-18  Umesh Kalappa<umesh.kalappa0@gmail.com>
> +	
> +	PR libgcc/86512
> +	* config/arm/ieee754-df.S :Don't normalise the denormal result.
> +

same as above.

> --- libgcc/config/arm/ieee754-df.S	(revision 262850)
> +++ libgcc/config/arm/ieee754-df.S	(working copy)
> @@ -203,8 +203,11 @@
>   #endif
>   
>   	@ Determine how to normalize the result.
> +	@ if result is denormal i.e (exp)=0,then don't normalise the result,
>   LSYM(Lad_p):
>   	cmp	xh, #0x00100000
> +	blt	LSYM(Lad_e)
> +	cmp	xh, #0x00100000
>   	bcc	LSYM(Lad_a)

i think you don't need to retest the same thing,
blt won't clobber the condition codes.

as for the technical contents, sorry i have not
reviewed it in detail, the blt might not be the
right check since it's signed compare..
(but i'm not even sure why is there an assembly
implementation for this, generic c code should do
a good job.)

>   	cmp	xh, #0x00200000
>   	bcc	LSYM(Lad_e)
>