Closed Bug 343998 Opened 15 years ago Closed 15 years ago
.7 landing broke windows ce .
js3250.lib(jsmath.obj) : error LNK2019: unresolved external symbol copysign referenced in function math_atan2 copysign isn't around. using the same workaround for the bustage in VC6 and VC7 seams to work fine. patch coming up.
This patch uses the existing |js_copysign| function impl. in js/. Alternatively, I could implement copysign in the windows ce shunt layer. We could simply drop off "&& !defined WINCE" from the #elif, but I think it is more clear being verbose.
Attachment #228579 - Flags: review?
Attachment #228579 - Flags: review? → review?(brendan)
OS: Windows XP → Windows CE
Hardware: PC → PocketPC
Comment on attachment 228579 [details] [diff] [review] Patch v.1 What defines js_copysign for WINCE? Isn't there a libm or libc copysign or _copysign or whatever that we can use? /be
on the devices i have tested against _copysign is available. I verified that _copysign for WINCE doesn't have the same problems outlined in bug 329383. Also, i have verified that with the latest sdk, it is perfectly fine to use the builtin tan and exp functions.
Comment on attachment 228700 [details] [diff] [review] patch v.2 >Index: jslibmath.h >=================================================================== >RCS file: /cvsroot/mozilla/js/src/jslibmath.h,v >retrieving revision 22.214.171.124 >diff -u -1 -0 -r126.96.36.199 jslibmath.h >--- jslibmath.h 7 Jul 2006 02:12:02 -0000 188.8.131.52 >+++ jslibmath.h 10 Jul 2006 18:52:01 -0000 >@@ -70,21 +70,23 @@ > > #define fd_acos acos > #define fd_asin asin > #define fd_atan atan > #define fd_atan2 atan2 > #define fd_ceil ceil > > /* The right copysign function is not always named the same thing. */ > #if __GNUC__ >= 4 || (__GNUC__ == 3 && __GNUC_MINOR__ >= 4) > #define fd_copysign __builtin_copysign >-#elif defined _WIN32 && !defined WINCE >+#elif defined WINCE >+#define fd_copysign _copysign >+#elif defined _WIN32 > #if _MSC_VER < 1400 > /* Try to work around apparent _copysign bustage in VC6 and VC7. */ > #define fd_copysign js_copysign > extern double js_copysign(double, double); > #else > #define fd_copysign _copysign > #endif > #else > #define fd_copysign copysign > #endif This is all within #if !JS_USE_FDLIBM_MATH (just noting). It looks ok, just wondering why you can't share the #define fd_copysign _copysign code now, by changing from #elif defined _WIN32 && !defined WINCE to just testing for "Windows (32 or CE)": #elif defined _WIN32 IOW, fall into the common code, including the _MSC_VER < 1400 test. (Is _WIN32 the right macro?) >@@ -106,21 +108,21 @@ > * Use math routines in fdlibm. > */ > > #undef __P > #ifdef __STDC__ > #define __P(p) p > #else > #define __P(p) () > #endif > >-#if (defined _WIN32 && !defined WINCE) || defined SUNOS4 >+#if defined _WIN32 || defined SUNOS4 This is in the big #else for that #if !JS_USE_FDLIBM_MATH -- the major comment introducing the guts of this #else says "Use math routines in fdlibm". Did you test compiling with JS_USE_FDLIBM_MATH defined too? > #define fd_acos acos > #define fd_asin asin > #define fd_atan atan > #define fd_cos cos > #define fd_sin sin > #define fd_tan tan > #define fd_exp exp > #define fd_log log > #define fd_sqrt sqrt Reason I ask is because the next three significant lines are: extern double fd_atan2 __P((double, double)); extern double fd_copysign __P((double, double)); extern double fd_pow __P((double, double)); which means JS_USE_FDLIBM_MATH defined as 1 implies most transcendentals are *not* from fdlibm, but atan2, pow, and the underlying copysign are. That's not quite right if we can use _copysign from the OS/SDK. The hard cases are atan2 and pow, where signed zeroes and other bugs exist. It'd be good to test these too. /be
I did not want to depend on the _MSC_VER macro here. For the MS compiler with the ppc/sp sdk, this isn't a problem at all (the macro is defined). I am worried about using the Intel WINCE arm compiler (which probably does not use this macro). To get the build off the floor (tbox has been burning for a few days), lets just ignore this change for now: -#if (defined _WIN32 && !defined WINCE) || defined SUNOS4 +#if defined _WIN32 || defined SUNOS4 If in agreement, i will open up a new bug to test and follow up.
Yeah, that sounds good -- it means JS_USE_FDLIBM_MATH is defined for Windows builds, or at least for WINCE builds -- can you confirm? My point was to test both forks of the #if !JS_USE_FDLIBM_MATH river, and to try to unify with existing WIN32 ifdefs. If you can do that in this bug, I'm good with not filing a followup. /be
this simply defines fd_copysign to the right thing on windows ce as discussed above.
pretty sure that |JS_USE_FDLIBM_MATH| is never defined on windows (32 or ce)
Comment on attachment 228720 [details] [diff] [review] fixes wince build bustage only. r=me, thanks. /be
Attachment #228720 - Flags: review?(brendan) → review+
Comment on attachment 228720 [details] [diff] [review] fixes wince build bustage only. a=schrep since b1 is out the door. Trivial WinCE fix
Attachment #228720 - Flags: approval1.8.1? → approval1.8.1+
patch landed on 1.8 branch (not sure what keyword to add for something post ff2a1). Checking in jslibmath.h; /cvsroot/mozilla/js/src/jslibmath.h,v <-- jslibmath.h new revision: 184.108.40.206; previous revision: 220.127.116.11 done This patch should also land on trunk. brendan, I can't check this into mozilla/js. Can you do the honor or point me to someone with the right stuff.
I checked this in for Doug: mozilla/js/src/jslibmath.h 3.22
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.