[llvm] r210428 - APFloat: x - NaN needs to flip the signbit of NaN when x is a number.

Steve (Numerics) Canon scanon at apple.com
Sun Jun 8 16:30:42 PDT 2014


I think splitting into fneg and fsub is a totally reasonable thing to do; they are distinct operations in the IEEE-754 model.  The only real gotchas with the current approach are the sign of 0 - 0 and 0 - NaN, but those are important cases to get right.  I'm no expert on the guts of LLVM though, so I have no idea what would actually be involved in such a change =)

– Steve

On Jun 8, 2014, at 2:00 PM, Owen Anderson <resistor at mac.com> wrote:

> Hi Steve,
> 
> What would be your opinion on splitting our floating point negation as a separate operation from subtraction in LLVM IR?  It seems like we’re paying the price for conflating them with bugs like this.
> 
> —Owen
> 
> On Jun 8, 2014, at 9:53 AM, Stephen Canon <scanon at apple.com> wrote:
> 
>> Author: scanon
>> Date: Sun Jun  8 11:53:31 2014
>> New Revision: 210428
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=210428&view=rev
>> Log:
>> APFloat: x - NaN needs to flip the signbit of NaN when x is a number.
>> 
>> Because we don't have a separate negate( ) function, 0 - NaN does double-duty as the IEEE-754 negate( ) operation, which (unlike most FP ops) *does* attach semantic meaning to the signbit of NaN.
>> 
>> Modified:
>>   llvm/trunk/lib/Support/APFloat.cpp
>>   llvm/trunk/unittests/ADT/APFloatTest.cpp
>> 
>> Modified: llvm/trunk/lib/Support/APFloat.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/APFloat.cpp?rev=210428&r1=210427&r2=210428&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Support/APFloat.cpp (original)
>> +++ llvm/trunk/lib/Support/APFloat.cpp Sun Jun  8 11:53:31 2014
>> @@ -1372,7 +1372,9 @@ APFloat::addOrSubtractSpecials(const APF
>>  case PackCategoriesIntoKey(fcZero, fcNaN):
>>  case PackCategoriesIntoKey(fcNormal, fcNaN):
>>  case PackCategoriesIntoKey(fcInfinity, fcNaN):
>> -    sign = false;
>> +    // We need to be sure to flip the sign here for subtraction because we
>> +    // don't have a separate negate operation so -NaN becomes 0 - NaN here.
>> +    sign = rhs.sign ^ subtract;
>>    category = fcNaN;
>>    copySignificand(rhs);
>>    return opOK;
>> 
>> Modified: llvm/trunk/unittests/ADT/APFloatTest.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/APFloatTest.cpp?rev=210428&r1=210427&r2=210428&view=diff
>> ==============================================================================
>> --- llvm/trunk/unittests/ADT/APFloatTest.cpp (original)
>> +++ llvm/trunk/unittests/ADT/APFloatTest.cpp Sun Jun  8 11:53:31 2014
>> @@ -1844,10 +1844,10 @@ TEST(APFloatTest, subtract) {
>>    { PInf, MInf, "inf", APFloat::opOK, APFloat::fcInfinity },
>>    { PInf, PZero, "inf", APFloat::opOK, APFloat::fcInfinity },
>>    { PInf, MZero, "inf", APFloat::opOK, APFloat::fcInfinity },
>> -    { PInf, QNaN, "nan", APFloat::opOK, APFloat::fcNaN },
>> +    { PInf, QNaN, "-nan", APFloat::opOK, APFloat::fcNaN },
>> #if 0
>> // See Note 1.
>> -    { PInf, SNaN, "nan", APFloat::opInvalidOp, APFloat::fcNaN },
>> +    { PInf, SNaN, "-nan", APFloat::opInvalidOp, APFloat::fcNaN },
>> #endif
>>    { PInf, PNormalValue, "inf", APFloat::opOK, APFloat::fcInfinity },
>>    { PInf, MNormalValue, "inf", APFloat::opOK, APFloat::fcInfinity },
>> @@ -1861,10 +1861,10 @@ TEST(APFloatTest, subtract) {
>>    { MInf, MInf, "nan", APFloat::opInvalidOp, APFloat::fcNaN },
>>    { MInf, PZero, "-inf", APFloat::opOK, APFloat::fcInfinity },
>>    { MInf, MZero, "-inf", APFloat::opOK, APFloat::fcInfinity },
>> -    { MInf, QNaN, "nan", APFloat::opOK, APFloat::fcNaN },
>> +    { MInf, QNaN, "-nan", APFloat::opOK, APFloat::fcNaN },
>> #if 0
>> // See Note 1.
>> -    { MInf, SNaN, "nan", APFloat::opInvalidOp, APFloat::fcNaN },
>> +    { MInf, SNaN, "-nan", APFloat::opInvalidOp, APFloat::fcNaN },
>> #endif
>>    { MInf, PNormalValue, "-inf", APFloat::opOK, APFloat::fcInfinity },
>>    { MInf, MNormalValue, "-inf", APFloat::opOK, APFloat::fcInfinity },
>> @@ -1878,10 +1878,10 @@ TEST(APFloatTest, subtract) {
>>    { PZero, MInf, "inf", APFloat::opOK, APFloat::fcInfinity },
>>    { PZero, PZero, "0x0p+0", APFloat::opOK, APFloat::fcZero },
>>    { PZero, MZero, "0x0p+0", APFloat::opOK, APFloat::fcZero },
>> -    { PZero, QNaN, "nan", APFloat::opOK, APFloat::fcNaN },
>> +    { PZero, QNaN, "-nan", APFloat::opOK, APFloat::fcNaN },
>> #if 0
>> // See Note 1.
>> -    { PZero, SNaN, "nan", APFloat::opInvalidOp, APFloat::fcNaN },
>> +    { PZero, SNaN, "-nan", APFloat::opInvalidOp, APFloat::fcNaN },
>> #endif
>>    { PZero, PNormalValue, "-0x1p+0", APFloat::opOK, APFloat::fcNormal },
>>    { PZero, MNormalValue, "0x1p+0", APFloat::opOK, APFloat::fcNormal },
>> @@ -1895,10 +1895,10 @@ TEST(APFloatTest, subtract) {
>>    { MZero, MInf, "inf", APFloat::opOK, APFloat::fcInfinity },
>>    { MZero, PZero, "-0x0p+0", APFloat::opOK, APFloat::fcZero },
>>    { MZero, MZero, "0x0p+0", APFloat::opOK, APFloat::fcZero },
>> -    { MZero, QNaN, "nan", APFloat::opOK, APFloat::fcNaN },
>> +    { MZero, QNaN, "-nan", APFloat::opOK, APFloat::fcNaN },
>> #if 0
>> // See Note 1.
>> -    { MZero, SNaN, "nan", APFloat::opInvalidOp, APFloat::fcNaN },
>> +    { MZero, SNaN, "-nan", APFloat::opInvalidOp, APFloat::fcNaN },
>> #endif
>>    { MZero, PNormalValue, "-0x1p+0", APFloat::opOK, APFloat::fcNormal },
>>    { MZero, MNormalValue, "0x1p+0", APFloat::opOK, APFloat::fcNormal },
>> @@ -1946,10 +1946,10 @@ TEST(APFloatTest, subtract) {
>>    { PNormalValue, MInf, "inf", APFloat::opOK, APFloat::fcInfinity },
>>    { PNormalValue, PZero, "0x1p+0", APFloat::opOK, APFloat::fcNormal },
>>    { PNormalValue, MZero, "0x1p+0", APFloat::opOK, APFloat::fcNormal },
>> -    { PNormalValue, QNaN, "nan", APFloat::opOK, APFloat::fcNaN },
>> +    { PNormalValue, QNaN, "-nan", APFloat::opOK, APFloat::fcNaN },
>> #if 0
>> // See Note 1.
>> -    { PNormalValue, SNaN, "nan", APFloat::opInvalidOp, APFloat::fcNaN },
>> +    { PNormalValue, SNaN, "-nan", APFloat::opInvalidOp, APFloat::fcNaN },
>> #endif
>>    { PNormalValue, PNormalValue, "0x0p+0", APFloat::opOK, APFloat::fcZero },
>>    { PNormalValue, MNormalValue, "0x1p+1", APFloat::opOK, APFloat::fcNormal },
>> @@ -1963,10 +1963,10 @@ TEST(APFloatTest, subtract) {
>>    { MNormalValue, MInf, "inf", APFloat::opOK, APFloat::fcInfinity },
>>    { MNormalValue, PZero, "-0x1p+0", APFloat::opOK, APFloat::fcNormal },
>>    { MNormalValue, MZero, "-0x1p+0", APFloat::opOK, APFloat::fcNormal },
>> -    { MNormalValue, QNaN, "nan", APFloat::opOK, APFloat::fcNaN },
>> +    { MNormalValue, QNaN, "-nan", APFloat::opOK, APFloat::fcNaN },
>> #if 0
>> // See Note 1.
>> -    { MNormalValue, SNaN, "nan", APFloat::opInvalidOp, APFloat::fcNaN },
>> +    { MNormalValue, SNaN, "-nan", APFloat::opInvalidOp, APFloat::fcNaN },
>> #endif
>>    { MNormalValue, PNormalValue, "-0x1p+1", APFloat::opOK, APFloat::fcNormal },
>>    { MNormalValue, MNormalValue, "0x0p+0", APFloat::opOK, APFloat::fcZero },
>> @@ -1980,10 +1980,10 @@ TEST(APFloatTest, subtract) {
>>    { PLargestValue, MInf, "inf", APFloat::opOK, APFloat::fcInfinity },
>>    { PLargestValue, PZero, "0x1.fffffep+127", APFloat::opOK, APFloat::fcNormal },
>>    { PLargestValue, MZero, "0x1.fffffep+127", APFloat::opOK, APFloat::fcNormal },
>> -    { PLargestValue, QNaN, "nan", APFloat::opOK, APFloat::fcNaN },
>> +    { PLargestValue, QNaN, "-nan", APFloat::opOK, APFloat::fcNaN },
>> #if 0
>> // See Note 1.
>> -    { PLargestValue, SNaN, "nan", APFloat::opInvalidOp, APFloat::fcNaN },
>> +    { PLargestValue, SNaN, "-nan", APFloat::opInvalidOp, APFloat::fcNaN },
>> #endif
>>    { PLargestValue, PNormalValue, "0x1.fffffep+127", APFloat::opInexact, APFloat::fcNormal },
>>    { PLargestValue, MNormalValue, "0x1.fffffep+127", APFloat::opInexact, APFloat::fcNormal },
>> @@ -1997,10 +1997,10 @@ TEST(APFloatTest, subtract) {
>>    { MLargestValue, MInf, "inf", APFloat::opOK, APFloat::fcInfinity },
>>    { MLargestValue, PZero, "-0x1.fffffep+127", APFloat::opOK, APFloat::fcNormal },
>>    { MLargestValue, MZero, "-0x1.fffffep+127", APFloat::opOK, APFloat::fcNormal },
>> -    { MLargestValue, QNaN, "nan", APFloat::opOK, APFloat::fcNaN },
>> +    { MLargestValue, QNaN, "-nan", APFloat::opOK, APFloat::fcNaN },
>> #if 0
>> // See Note 1.
>> -    { MLargestValue, SNaN, "nan", APFloat::opInvalidOp, APFloat::fcNaN },
>> +    { MLargestValue, SNaN, "-nan", APFloat::opInvalidOp, APFloat::fcNaN },
>> #endif
>>    { MLargestValue, PNormalValue, "-0x1.fffffep+127", APFloat::opInexact, APFloat::fcNormal },
>>    { MLargestValue, MNormalValue, "-0x1.fffffep+127", APFloat::opInexact, APFloat::fcNormal },
>> @@ -2014,10 +2014,10 @@ TEST(APFloatTest, subtract) {
>>    { PSmallestValue, MInf, "inf", APFloat::opOK, APFloat::fcInfinity },
>>    { PSmallestValue, PZero, "0x1p-149", APFloat::opOK, APFloat::fcNormal },
>>    { PSmallestValue, MZero, "0x1p-149", APFloat::opOK, APFloat::fcNormal },
>> -    { PSmallestValue, QNaN, "nan", APFloat::opOK, APFloat::fcNaN },
>> +    { PSmallestValue, QNaN, "-nan", APFloat::opOK, APFloat::fcNaN },
>> #if 0
>> // See Note 1.
>> -    { PSmallestValue, SNaN, "nan", APFloat::opInvalidOp, APFloat::fcNaN },
>> +    { PSmallestValue, SNaN, "-nan", APFloat::opInvalidOp, APFloat::fcNaN },
>> #endif
>>    { PSmallestValue, PNormalValue, "-0x1p+0", APFloat::opInexact, APFloat::fcNormal },
>>    { PSmallestValue, MNormalValue, "0x1p+0", APFloat::opInexact, APFloat::fcNormal },
>> @@ -2031,10 +2031,10 @@ TEST(APFloatTest, subtract) {
>>    { MSmallestValue, MInf, "inf", APFloat::opOK, APFloat::fcInfinity },
>>    { MSmallestValue, PZero, "-0x1p-149", APFloat::opOK, APFloat::fcNormal },
>>    { MSmallestValue, MZero, "-0x1p-149", APFloat::opOK, APFloat::fcNormal },
>> -    { MSmallestValue, QNaN, "nan", APFloat::opOK, APFloat::fcNaN },
>> +    { MSmallestValue, QNaN, "-nan", APFloat::opOK, APFloat::fcNaN },
>> #if 0
>> // See Note 1.
>> -    { MSmallestValue, SNaN, "nan", APFloat::opInvalidOp, APFloat::fcNaN },
>> +    { MSmallestValue, SNaN, "-nan", APFloat::opInvalidOp, APFloat::fcNaN },
>> #endif
>>    { MSmallestValue, PNormalValue, "-0x1p+0", APFloat::opInexact, APFloat::fcNormal },
>>    { MSmallestValue, MNormalValue, "0x1p+0", APFloat::opInexact, APFloat::fcNormal },
>> @@ -2048,10 +2048,10 @@ TEST(APFloatTest, subtract) {
>>    { PSmallestNormalized, MInf, "inf", APFloat::opOK, APFloat::fcInfinity },
>>    { PSmallestNormalized, PZero, "0x1p-126", APFloat::opOK, APFloat::fcNormal },
>>    { PSmallestNormalized, MZero, "0x1p-126", APFloat::opOK, APFloat::fcNormal },
>> -    { PSmallestNormalized, QNaN, "nan", APFloat::opOK, APFloat::fcNaN },
>> +    { PSmallestNormalized, QNaN, "-nan", APFloat::opOK, APFloat::fcNaN },
>> #if 0
>> // See Note 1.
>> -    { PSmallestNormalized, SNaN, "nan", APFloat::opInvalidOp, APFloat::fcNaN },
>> +    { PSmallestNormalized, SNaN, "-nan", APFloat::opInvalidOp, APFloat::fcNaN },
>> #endif
>>    { PSmallestNormalized, PNormalValue, "-0x1p+0", APFloat::opInexact, APFloat::fcNormal },
>>    { PSmallestNormalized, MNormalValue, "0x1p+0", APFloat::opInexact, APFloat::fcNormal },
>> @@ -2065,10 +2065,10 @@ TEST(APFloatTest, subtract) {
>>    { MSmallestNormalized, MInf, "inf", APFloat::opOK, APFloat::fcInfinity },
>>    { MSmallestNormalized, PZero, "-0x1p-126", APFloat::opOK, APFloat::fcNormal },
>>    { MSmallestNormalized, MZero, "-0x1p-126", APFloat::opOK, APFloat::fcNormal },
>> -    { MSmallestNormalized, QNaN, "nan", APFloat::opOK, APFloat::fcNaN },
>> +    { MSmallestNormalized, QNaN, "-nan", APFloat::opOK, APFloat::fcNaN },
>> #if 0
>> // See Note 1.
>> -    { MSmallestNormalized, SNaN, "nan", APFloat::opInvalidOp, APFloat::fcNaN },
>> +    { MSmallestNormalized, SNaN, "-nan", APFloat::opInvalidOp, APFloat::fcNaN },
>> #endif
>>    { MSmallestNormalized, PNormalValue, "-0x1p+0", APFloat::opInexact, APFloat::fcNormal },
>>    { MSmallestNormalized, MNormalValue, "0x1p+0", APFloat::opInexact, APFloat::fcNormal },
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 





More information about the llvm-commits mailing list