This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: RFC: x86: Emit rsqrt with only -ffast-math


On 11/04/2009 06:14 PM, Michael Matz wrote:
Hi,

On Tue, 3 Nov 2009, Uros Bizjak wrote:

introduce a new flag for just the last transformation. What do people
think?
There were some problems with the patch that enabled rsqrt
transformation in -ffast-math, see PR34709 [1]. The root cause was
analysed and the fix was proposed [2], but unfortunately, the fix was
never applied.
That testcase explicitely checks for outputs being +-Inf and gives Inf as
input in one case.  As you noted in your followup there [1], this is
outside the scope of -ffinite-math.  Hence that after the change NaN is
given is completely okay.  The testcase wasn't the root cause for 481.wrf
breaking.  The root cause was to transform sqrt(a/b) into 1/sqrt(b/a) at
all, which I indeed fixed (and added a testcase for already).  That
transformation lead to the followup problem that suddenly behaviour at Inf
mattered, when it really shouldn't.

So, I don't think we are obliged to make this testcase work as the author
expected (it will change behaviour with my proposed patch).  It would
unnecessarily slow down the rsqrt+NR expansion.

The patch itself is OK with the testcase from [2], but please be
prepared for eventual surprises ...
So, given the above, still OK, even without that testcase?  I of course
checked that 481.wrf doesn't break (neither the other cpu2006 or
polyhedron benchmarks).  And regstrapped on x86_64-linux, with all
languages+Ada.

[1] http://gcc.gnu.org/ml/gcc-patches/2008-01/msg00750.html


Yes, following your analysis, the patch is OK.


Thanks,
Uros.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]