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

 
 

You are not allowed to post comments on this tracker with your current authentication level.

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

Submitter:  Ruud <ruud62>
Submitted:  Thu 20 Nov 2008 10:17:12 AM UTC
   
 
Category:  Library Severity:  3 - Normal
Priority:  5 - Normal Item Group:  libm code
Status:  Fixed Assigned to:  dmix
Percent Complete:  100% Open/Closed:  Closed
Release:  1.6.3 Fixed Release:  None

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>
Group administrator
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>
Group administrator
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>
Group 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>
Group 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>
Group 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>
Group administrator
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>
Group 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>
Group administrator
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>
Group 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>
Group administrator
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>
Group Member

 

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

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
  • -email is unavailable- added by dmix (Posted a comment)
  • -email is unavailable- added by ruud62 (Submitted the item)
  •  

    Follow 10 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2008-12-02 dmix Percent Complete0% 100%
        Open/ClosedOpen Closed
    2008-11-30 ruud62 Attached File- Added fp_rempio2.S, #16948
    2008-11-23 ruud62 Attached File- Added fp_rempio2.S, #16904
    2008-11-23 dmix Priority7 - High 5 - Normal
        StatusIn Progress Fixed
    2008-11-21 dmix Priority5 - Normal 7 - High
        StatusNone In Progress
        Assigned toNone dmix
    2008-11-20 ruud62 Attached File- Added fp_rempio2.S, #16885

    Back to the top

    Powered by Savane 3.13-d3ae.
    Corresponding source code