[llvm-commits] [llvm] r57185 - /llvm/trunk/lib/Support/APFloat.cpp

[Neil Booth]

Dale Johannesen wrote:-

> 
> On Oct 6, 2008, at 11:51 AMPDT, Dan Gohman wrote:
> >>
> >> +    fs = (shift < 0) ? opInexact : opOK;
> >>    // gcc forces the Quiet bit on, which means (float)(double)
> >> (float_sNan)
> >>    // does not give you back the same bits.  This is dubious, and we
> >>    // don't currently do it.  You're really supposed to get
> >>    // an invalid operation signal at runtime, but nobody does that.
> >> -    fs = opOK;
> >
> > Shouldn't converting a NaN set the opInvalidOp flag?
> 
> According to IEEE754, converting a SNan sets opInvalid, but we aren't  
> really trying for an accurate implementation of SNan's (nor does the  
> OS).
> 
> For converting a QNan, what it says is:
> "Every operation involving one or two input NaNs, none of them  
> signaling, shall signal no exception but, if a FP result is to be  
> delivered, shall deliver as its result a QNaN, which should be one of  
> the input NaNs.  Note that format conversions might be unable to  
> deliver the same NaN."
> 
> The last sentence applies here, and I'm not sure of its intent.   At  
> face value it's a statement that it may be impossible to implement the  
> standard in this case, which seems unlikely.   IMO an Inexact result  
> seems compatible with the way things work otherwise.
> 
> To confuse things further, the value in the PR is not even an IEEE  
> NaN, but a pseudo-NaN in Intel jargon.  It is outside all standards.

The intent of APFloat as I wrote it was to replicate a conforming
IEEE-754 CPU's results in software, with the return result indicating
those exceptions that would occur under IEEE-754.  IBM double double
was later hacked on in an incomplete way.

The paragraph you quote is unambiguous; these NaN to NaN conversions
should always return fsOK because it "shall signal no exception".
This is how the code worked before your change.

Perhaps LLVM needs APFloat to evolve beyond IEEE-754; I'm not sure.
But I suggest you think hard about that, and if you decide to go that
way you at least use a new flag for your extensions.  You should
also update the comments in the .cpp and .h files appropriately,
and review what other changes, if any, to other operations are needed
to accomdate your new semantics.

Neil.