Re: [PATCH] Only set rounding mode in SET_RESTORE_ROUND_53BIT

[Andreas Jaeger <aj at suse dot com>]

On 06/09/2013 09:56 PM, Andreas Jaeger wrote:
> On 05/30/2013 10:59 AM, Siddhesh Poyarekar wrote:
> > On Wed, May 29, 2013 at 07:33:19PM +0530, Siddhesh Poyarekar wrote:
> > > I'm trying to understand the code that sets and restores the floating
> > > point state to try and eek out some performance from it.  I haven't
> > > been able to figure out the need for SET_RESTORE_ROUND_53BIT vs
> > > SET_RESTORE_ROUND_53 for x86_64 - I can see that it's needed for x87:
> > >
> > > SET_RESTORE_ROUND (used in exp, log) only sets and restores the
> > > rounding mode and does not touch the exception masks.  This was
> > > holdexcept_setround, i.e. the same implementation as
> > > SET_RESTORE_ROUND_53BIT before, but I assume the clearing of exception
> > > mask, etc. was removed because it could result in delay in raising
> > > floating point exceptions resulting from some operations within these
> > > functions if exceptions are masked.
> > >
> > > SET_RESTORE_ROUND_53BIT (used in sin/cos) sets the rounding mode,
> > > exception mask and clears all exception flags.  Why can't we simply
> > > get away with setting and restoring just the rounding mode like in
> > > exp, log, etc?  That opens up a good opportunity for optimization
> > > where I could avoid most of the set/restore code in the default case.
> > > sin and cos run almost twice as fast in some cases.
> >
> > So to put all this in code, the below patch is what I intend to do.
> > Instead of modifying exception flags, SET_RESTORE_ROUND_53BIT only
> > sets rounding mode and floating point mode (in x87) and restores it on
> > exit from the lexical block.  I tested x86_64 as well as i386 with
> > --disable-sse to verify that this does not cause any regressions in
> > the testsuite.  Would this change be correct?
>
> Richard, could you comment on this since you introduced the macros with
> commit eb92c487?
>
> Siddesh, this looks fine to me, let's give Richard a chance to review
> this as well,


I just saw that Richard answered elsewhere on this one and this patch is 
already in - great!

Sorry, I'm catching up after two weeks of vacation and missed the reply 
at first,

Andreas
--
 Andreas Jaeger aj@{suse.com,opensuse.org} Twitter/Identica: jaegerandi
  SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
   GF: Jeff Hawn,Jennifer Guild,Felix Imendörffer,HRB16746 (AG Nürnberg)
    GPG fingerprint = 93A3 365E CE47 B889 DF7F  FED1 389A 563C C272 A126