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

Reply via email to