From a5699a1e8b1119a5fbba7c184612f51ff428984d Mon Sep 17 00:00:00 2001 From: Dale Johannesen Date: Wed, 21 Jan 2009 20:32:55 +0000 Subject: [PATCH] Do not use host floating point types when emitting ASCII IR; loading and storing these can change the bits of NaNs on some hosts. Remove or add warnings at a few other places using host floating point; this is a bad thing to do in general. llvm-svn: 62712 --- include/llvm/ADT/FoldingSet.h | 2 -- include/llvm/Support/MathExtras.h | 8 ++++++-- lib/Support/APFloat.cpp | 3 ++- lib/Support/FoldingSet.cpp | 6 ------ lib/VMCore/AsmWriter.cpp | 14 ++++++++++++-- .../2009-01-19-fmod-constant-float-specials.ll | 4 ++-- 6 files changed, 22 insertions(+), 15 deletions(-) diff --git a/include/llvm/ADT/FoldingSet.h b/include/llvm/ADT/FoldingSet.h index a69197f3377..d6010adfb9f 100644 --- a/include/llvm/ADT/FoldingSet.h +++ b/include/llvm/ADT/FoldingSet.h @@ -225,8 +225,6 @@ public: void AddInteger(unsigned long I); void AddInteger(long long I); void AddInteger(unsigned long long I); - void AddFloat(float F); - void AddDouble(double D); void AddString(const std::string &String); void AddString(const char* String); diff --git a/include/llvm/Support/MathExtras.h b/include/llvm/Support/MathExtras.h index 8a89d85cd58..d55fb80268a 100644 --- a/include/llvm/Support/MathExtras.h +++ b/include/llvm/Support/MathExtras.h @@ -361,7 +361,9 @@ inline float BitsToFloat(uint32_t Bits) { } /// DoubleToBits - This function takes a double and returns the bit -/// equivalent 64-bit integer. +/// equivalent 64-bit integer. Note that copying doubles around +/// changes the bits of NaNs on some hosts, notably x86, so this +/// routine cannot be used if these bits are needed. inline uint64_t DoubleToBits(double Double) { union { uint64_t L; @@ -372,7 +374,9 @@ inline uint64_t DoubleToBits(double Double) { } /// FloatToBits - This function takes a float and returns the bit -/// equivalent 32-bit integer. +/// equivalent 32-bit integer. Note that copying floats around +/// changes the bits of NaNs on some hosts, notably x86, so this +/// routine cannot be used if these bits are needed. inline uint32_t FloatToBits(float Float) { union { uint32_t I; diff --git a/lib/Support/APFloat.cpp b/lib/Support/APFloat.cpp index c296770385f..d8d414d7ea5 100644 --- a/lib/Support/APFloat.cpp +++ b/lib/Support/APFloat.cpp @@ -599,7 +599,8 @@ APFloat::copySignificand(const APFloat &rhs) } /* Make this number a NaN, with an arbitrary but deterministic value - for the significand. */ + for the significand. If double or longer, this is a signalling NaN, + which may not be ideal. */ void APFloat::makeNaN(void) { diff --git a/lib/Support/FoldingSet.cpp b/lib/Support/FoldingSet.cpp index d2b02f240c9..3a1a0cd842e 100644 --- a/lib/Support/FoldingSet.cpp +++ b/lib/Support/FoldingSet.cpp @@ -61,12 +61,6 @@ void FoldingSetNodeID::AddInteger(unsigned long long I) { if ((uint64_t)(int)I != I) Bits.push_back(unsigned(I >> 32)); } -void FoldingSetNodeID::AddFloat(float F) { - Bits.push_back(FloatToBits(F)); -} -void FoldingSetNodeID::AddDouble(double D) { - AddInteger(DoubleToBits(D)); -} void FoldingSetNodeID::AddString(const char *String) { unsigned Size = static_cast(strlen(String)); diff --git a/lib/VMCore/AsmWriter.cpp b/lib/VMCore/AsmWriter.cpp index b59ec0863b3..6a17516be26 100644 --- a/lib/VMCore/AsmWriter.cpp +++ b/lib/VMCore/AsmWriter.cpp @@ -640,6 +640,7 @@ static void WriteConstantInt(raw_ostream &Out, const Constant *CV, // make sure that we only output it in exponential format if we can parse // the value back and get the same value. // + bool ignored; bool isDouble = &CFP->getValueAPF().getSemantics()==&APFloat::IEEEdouble; double Val = isDouble ? CFP->getValueAPF().convertToDouble() : CFP->getValueAPF().convertToFloat(); @@ -659,11 +660,20 @@ static void WriteConstantInt(raw_ostream &Out, const Constant *CV, } } // Otherwise we could not reparse it to exactly the same value, so we must - // output the string in hexadecimal format! + // output the string in hexadecimal format! Note that loading and storing + // floating point types changes the bits of NaNs on some hosts, notably + // x86, so we must not use these types. assert(sizeof(double) == sizeof(uint64_t) && "assuming that double is 64 bits!"); char Buffer[40]; - Out << "0x" << utohex_buffer(uint64_t(DoubleToBits(Val)), Buffer+40); + APFloat apf = CFP->getValueAPF(); + // Floats are represented in ASCII IR as double, convert. + if (!isDouble) + apf.convert(APFloat::IEEEdouble, APFloat::rmNearestTiesToEven, + &ignored); + Out << "0x" << + utohex_buffer(uint64_t(apf.bitcastToAPInt().getZExtValue()), + Buffer+40); return; } diff --git a/test/Transforms/InstCombine/2009-01-19-fmod-constant-float-specials.ll b/test/Transforms/InstCombine/2009-01-19-fmod-constant-float-specials.ll index 47f2573a180..cc001f0334c 100644 --- a/test/Transforms/InstCombine/2009-01-19-fmod-constant-float-specials.ll +++ b/test/Transforms/InstCombine/2009-01-19-fmod-constant-float-specials.ll @@ -1,8 +1,8 @@ ; RUN: llvm-as < %s | opt -simplifycfg -instcombine | llvm-dis | grep 0x7FF8000000000000 | count 7 -; RUN: llvm-as < %s | opt -simplifycfg -instcombine | llvm-dis | grep 0x7FF80000FFFFFFFF | count 5 +; RUN: llvm-as < %s | opt -simplifycfg -instcombine | llvm-dis | grep 0x7FF00000FFFFFFFF | count 5 ; RUN: llvm-as < %s | opt -simplifycfg -instcombine | llvm-dis | grep {0\\.0} | count 3 ; RUN: llvm-as < %s | opt -simplifycfg -instcombine | llvm-dis | grep {3\\.5} | count 1 -; XFAIL: x86_64 +; ; ModuleID = 'apf.c' target datalayout = "e-p:32:32:32-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:32:64-f32:32:32-f64:32:64-v64:64:64-v128:128:128-a0:0:64-f80:128:128"