I haven't had any real code segment to explain why the change to float to double is needed for the claim, i.e. enable 64bit porting, and better precision. I have to do this in the abstract. I don't see any relationship between float and double to 64 bit porting. As for precision, the example given is "probability". For compiler optimization, it is true that probability is widely used to help heuristics, but I have yet to see an example where one needs beyond 2 decimal point precision. I'm gonna have to say, this change from float to double is not useful other than making data size larger, which is in general not a good thing. Sun
On Fri, Aug 3, 2012 at 7:38 AM, Sun Chan <sun.c...@gmail.com> wrote: > for the float/double issue, can you include a code snippet that will > affect by this change (you have mentioned probability, for example) > Thx! > Sun > > On Fri, Aug 3, 2012 at 7:19 AM, Gilmore, Doug <doug.gilm...@amd.com> wrote: >> Sorry for the delay in my reply. >>> -----Original Message----- >>> From: "C. Bergström" [mailto:cbergst...@pathscale.com] >>> Sent: Saturday, July 28, 2012 11:09 PM >>> To: Gilmore, Doug >>> Cc: open64-devel >>> Subject: Re: [Open64-devel] Review request: fixing issues when building >>> the compiler as 64-bit binaries >>> >>> >>> 0007 - I wouldn't agree to changing float to double. Determining the >>> actual correct size uint64_t or something is probably the most future >>> proof and portable. >> A floating point format is needed since calculations involving >> probabilities and frequencies are being performed. >> >> Using single precision exposes too many situations where ties can >> occur or a 1LP change in an FP calculation can the effect of a >> comparison result. Using double precisions drastically reduces the >> chance that these situations will arise. >>> Did you check if this also impacts compiler >>> performance? >> >From the timing runs I didn't see any. >>> >>> nit: You've also slipped in a small configuration change >>> >>> + CCNAME += -mfpmath=sse -msse2 >>> + C++NAME += -mfpmath=sse -msse2 >>> + F90 += -mfpmath=sse -msse2 >>> --------- >>> Speaking from experience - I'd need to know more details of why you >>> felt this change is even necessary. >>> -------------- >> I explained this in the commit log message: >> >> When compiling with GCC at 32, add -mfpmath=sse -msse2, which >> prevents i387 FP instructions from being generated, otherwise the >> values for probabilities and frequencies can easily diverge between >> the 32 and 64 bit builds of the compiler. >> >> I'll move this comment to gcommondefs and make a brief note referring >> to the comment in the commit log message. >>> >>> 0006 - The FFE clean-up is 494KB! I've copied below log of all the >>> issues it fixes. It seems clear that for proper review they should be >>> individually pulled out. >>> ------------- >>> Adapted code conditionally compiled under _TARGET32, (now >>> referred to _TARGET32_OR_64) and code compiled under _HOST32 >>> now referred to_HOST32_OR_64) for use when building the >>> compiler in 64 bit mode. >>> >>> Note that _TARGET32 was a misnomer, since the 32 bit FFE >>> build could target both 32 and 64 bit targets. >>> >>> The end result is the 64 bit build of the FFE produces >>> the same WHIRL and nearly the same debugging/trace output as >>> the 32 bit build. >>> >>> The key (and quite obtrusive) change: >>> >>> Instead of just declaring variables with type long, >>> use type names to describe variables for various storage: >>> >>> ... >> The key to tracking down issues that causes differences in WHIRL >> generated by the 32/64 bit builds of the FFE was to make the debug >> output of the FFE comparable. >> >> The typing cleanup change along with the tracing changes in i_cvrt.c >> (add calls to cwh_stab_dbg_id()/adding the -ustb_id option flag.) are >> the crucial changes. Otherwise there is just too much noise in the >> debug file output in the 32/64 bit builds of the FFE. >> >> The cwh_stab_dbg_id() is too intertwined with the typing change to break >> out separately separately. >> >> The change of adding use INIT_OPND_TYPE to initialize variables of >> type opnd_type has 604 occurances. Given the change innocuous, I hope >> that reviewers won't require the change to be pulled as a separate >> commit. >> >> The other issues are localized changes that can be broken out. >> >> I can attended to that, though there will still be one quite large >> change that will need to be reviewed and committed. >> >> Doug >> >> >> ------------------------------------------------------------------------------ >> Live Security Virtual Conference >> Exclusive live event will cover all the ways today's security and >> threat landscape has changed and how IT managers can respond. Discussions >> will include endpoint security, mobile security and the latest in malware >> threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/ >> _______________________________________________ >> Open64-devel mailing list >> Open64-devel@lists.sourceforge.net >> https://lists.sourceforge.net/lists/listinfo/open64-devel ------------------------------------------------------------------------------ Live Security Virtual Conference Exclusive live event will cover all the ways today's security and threat landscape has changed and how IT managers can respond. Discussions will include endpoint security, mobile security and the latest in malware threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/ _______________________________________________ Open64-devel mailing list Open64-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/open64-devel
