bugAVR C Runtime Library - Bugs: bug #24890, Small error in libm source...

 
 

bug #24890: Small error in libm source fp_rempio2.S

Submitted by:  Ruud <ruud62>
Submitted on:  Thu 20 Nov 2008 10:17:12 AM UTC  
 
Category: LibrarySeverity: 3 - Normal
Priority: 5 - NormalItem Group: libm code
Status: FixedPercent Complete: 100%
Assigned to: Dmitry Xmelkov <dmix>Open/Closed: Closed
Release: 1.6.3Fixed Release: None

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission Jump to the original submission)

Tue 02 Dec 2008 01:51:31 PM UTC, comment #10:

Last variant (11 words saved summary) is commited in 1_6 and HEAD branches.

Dmitry Xmelkov <dmix>
Project AdministratorIn charge of this item.
Mon 01 Dec 2008 03:07:50 PM UTC, comment #9:

Fine!
In result (I have also reduce fp_mpack() by 2 words) the
size of libm is reduced by 11 words.

Testing in progress...

Dmitry Xmelkov <dmix>
Project AdministratorIn charge of this item.
Sun 30 Nov 2008 11:00:21 AM UTC, comment #8:

Sorry for floading this list

Dmitry,
You can also get rid of the com ZL at the end by using inc ZL instead of clc in the loop, this will save 1 code word and reduces the cycles (though very little)
See attachment..
Regards
Ruud

(file #16948)

Ruud <ruud62>
Project Member
Sun 30 Nov 2008 10:28:26 AM UTC, comment #7:

Stupid me, of course the FUNCTION should be in.. There is a label above the ENTRY.

Ruud

Ruud <ruud62>
Project Member
Sun 30 Nov 2008 09:59:23 AM UTC, comment #6:

Thanx Dmitry

Sorry for removing the clt (unintentionally), it should clearly be in! Don't quite understand why the FUNCTION statement is needed since I'm quite new to the libc macro's. Anyway that won't influence the size or performance. I especially like the __fp_mpack_finite idea, can be used by a lot of other modules too I imagine..
Regards
Ruud

Ruud <ruud62>
Project Member
Sun 30 Nov 2008 02:22:22 AM UTC, comment #5:

Thanks, Ruud!
You are right. The simplificion of initial comparison and
the reorganization of loop save a few words and cycles.
And after division the smallest value is really 0x0E.
(I have add the test program of this fact into source as
comment.) After testing I shall commit yours variant with
changes:
1. Macro 'FUNCTION' and 'clt' statement are restored.
Without first a relocation error is possible. The second is
needed for negative args.
2. I have add extra entry '__fp_mpack_finite' and jump to it
to save few cycles.

Testing in progress...

Dmitry Xmelkov <dmix>
Project AdministratorIn charge of this item.
Sun 23 Nov 2008 02:03:14 PM UTC, comment #4:

Of course a well writen proggy would keep the argument below pi/2 but still... without the initial compare the function would take about 5 more cycles for values between 1 and pi/2. And it is shorter and faster (and faster for all other values).. Pls considder the attached routine..

grtz
Ruud

(file #16904)

Ruud <ruud62>
Project Member
Sun 23 Nov 2008 01:03:41 AM UTC, comment #3:

Fixed in HEAD and 1_6 branches.

Idea of first comparison was that good quality programs
use big angles (beyond of 180 grad) with trigonometric
functions very rarely.
I shall thinks about your's suggestion. So, I do not close
this item.

Thanks, Dmitry.

Dmitry Xmelkov <dmix>
Project AdministratorIn charge of this item.
Fri 21 Nov 2008 09:34:38 AM UTC, comment #2:

Hi DMitry
You may also want to leave out

; compare A with Pi/2
...
ldi rB1, hi8(SPLIT_PIO2 + 1)
ldi rB2, hlo8(SPLIT_PIO2 + 1)
ldi rB3, hhi8(SPLIT_PIO2 + 1)
cpi rA0, lo8(SPLIT_PIO2 + 1)
cpc rA1, rB1
cpc rA2, rB2
cpc rA3, rB3
brlo .L_pack ; A < Pi/2 ==> return A

Or replace it with a exponent test only.. This will in general increase the performance and is shorter code.

Also the answer to the question:

/* FIXME: Is it possible to obtain a zero value in rA2.1.0.E ?
I do't known. So, there is a check in exponent decrement. */

I do know, it is NO.. The minimum value of RA2....= 0x0000000E

:-)trust me

Anyway I wanted to suggest revising the sine, cosine routines because they aren't all that accurate and can be speeded up considerably. This would also introduce a small change in fp_rempio2 (and some other functions which rely on powsodd, so isn't it perhaps better if you let me handle fp_rempio2 ???

Regards
Ruud

ps. Sorry for the long text

Ruud <ruud62>
Project Member
Fri 21 Nov 2008 04:28:18 AM UTC, comment #1:

Thanks, Ruud!
This is my misprint. I will correct it at the nearest time.
Regards, Dmitry.

Dmitry Xmelkov <dmix>
Project AdministratorIn charge of this item.
Thu 20 Nov 2008 10:17:12 AM UTC, original submission:

The line:

#define SPLIT_PIO2 0x7FC80FDA /* split(HI40_PIO2)*/

Should be:

#define SPLIT_PIO2 0x7FC90FDA /* split(HI40_PIO2)*/

Though this doesn't affect the outcome of the function, it does in rare cases influence the performance.

Greetz
Ruud

Things were a bit messy yesterday, so if this bug report has already come trough I apologize.

Ruud <ruud62>
Project Member

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

Attached Files
file #16948:  fp_rempio2.S added by ruud62 (1KiB - text/plain)
file #16904:  fp_rempio2.S added by ruud62 (1KiB - text/plain)
file #16885:  fp_rempio2.S added by ruud62 (3KiB - text/plain)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by dmix (Posted a comment)
  • -unavailable- added by ruud62 (Submitted the item)
  •  

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 10 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Tue 02 Dec 2008 01:51:31 PM UTCdmixPercent Complete0%=>100%
      Open/ClosedOpen=>Closed
    Sun 30 Nov 2008 11:00:21 AM UTCruud62Attached File-=>Added fp_rempio2.S, #16948
    Sun 23 Nov 2008 02:03:13 PM UTCruud62Attached File-=>Added fp_rempio2.S, #16904
    Sun 23 Nov 2008 01:03:41 AM UTCdmixPriority7 - High=>5 - Normal
      StatusIn Progress=>Fixed
    Fri 21 Nov 2008 04:28:18 AM UTCdmixPriority5 - Normal=>7 - High
      StatusNone=>In Progress
      Assigned toNone=>dmix
    Thu 20 Nov 2008 10:17:12 AM UTCruud62Attached File-=>Added fp_rempio2.S, #16885

    Back to the top


    Powered by Savane 3.1-cleanup1