Hi Eli, I noticed that your commit introduces a regression in the LLDB test suite because expression evaluation of a floating point constant like 2.234f results in a value like 2.23399997. I suspect that this occurs because 2.234f is really just the closest number to 2.23399997 that can be represented using floating point precision. I noticed that your commit increases the default number of digits in the precision of APFloat. I can see how that's useful when performing intermediate computation, but I would have expected APFloat::toString to cleverly avoid reality.
The attached black magic fixes the regressions in the IRInterpreter used for expression evaluation. However, I'm wondering if the correct fix is to revert your commit (i.e. in favor of mode to select the default precision as nominal precision versus active precision). Cheers, - Ashok -----Original Message----- From: [email protected] [mailto:[email protected]] On Behalf Of Eli Friedman Sent: Thursday, August 29, 2013 7:45 PM To: [email protected] Subject: [llvm] r189624 - Change default # of digits for APFloat::toString Author: efriedma Date: Thu Aug 29 18:44:34 2013 New Revision: 189624 URL: http://llvm.org/viewvc/llvm-project?rev=189624&view=rev Log: Change default # of digits for APFloat::toString This is a re-commit of r189442; I'll follow up with clang changes. The previous default was almost, but not quite enough digits to represent a floating-point value in a manner which preserves the representation when it's read back in. The larger default is much less confusing. I spent some time looking into printing exactly the right number of digits if a precision isn't specified, but it's kind of complicated, and I'm not really sure I understand what APFloat::toString is supposed to output for FormatPrecision != 0 (or maybe the current API specification is just silly, not sure which). I have a WIP patch if anyone is interested. 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=189624&r1=189623&r2=189624&view=diff ============================================================================== --- llvm/trunk/lib/Support/APFloat.cpp (original) +++ llvm/trunk/lib/Support/APFloat.cpp Thu Aug 29 18:44:34 2013 @@ -3546,11 +3546,14 @@ void APFloat::toString(SmallVectorImpl<c // Set FormatPrecision if zero. We want to do this before we // truncate trailing zeros, as those are part of the precision. if (!FormatPrecision) { - // It's an interesting question whether to use the nominal - // precision or the active precision here for denormals. + // We use enough digits so the number can be round-tripped back to an + // APFloat. The formula comes from "How to Print Floating-Point Numbers + // Accurately" by Steele and White. + // FIXME: Using a formula based purely on the precision is conservative; + // we can print fewer digits depending on the actual value being printed. - // FormatPrecision = ceil(significandBits / lg_2(10)) - FormatPrecision = (semantics->precision * 59 + 195) / 196; + // FormatPrecision = 2 + floor(significandBits / lg_2(10)) + FormatPrecision = 2 + semantics->precision * 59 / 196; } // Ignore trailing binary zeros. Modified: llvm/trunk/unittests/ADT/APFloatTest.cpp URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/APFloatTest.cpp?rev=189624&r1=189623&r2=189624&view=diff ============================================================================== --- llvm/trunk/unittests/ADT/APFloatTest.cpp (original) +++ llvm/trunk/unittests/ADT/APFloatTest.cpp Thu Aug 29 18:44:34 2013 @@ -866,10 +866,11 @@ TEST(APFloatTest, toString) { ASSERT_EQ("0.0101", convertToString(1.01E-2, 5, 2)); ASSERT_EQ("0.0101", convertToString(1.01E-2, 4, 2)); ASSERT_EQ("1.01E-2", convertToString(1.01E-2, 5, 1)); - ASSERT_EQ("0.7853981633974483", convertToString(0.78539816339744830961, 0, 3)); - ASSERT_EQ("4.940656458412465E-324", convertToString(4.9406564584124654e-324, 0, 3)); - ASSERT_EQ("873.1834", convertToString(873.1834, 0, 1)); - ASSERT_EQ("8.731834E+2", convertToString(873.1834, 0, 0)); + ASSERT_EQ("0.78539816339744828", + convertToString(0.78539816339744830961, 0, 3)); + ASSERT_EQ("4.9406564584124654E-324", + convertToString(4.9406564584124654e-324, 0, 3)); + ASSERT_EQ("873.18340000000001", convertToString(873.1834, 0, 1)); + ASSERT_EQ("8.7318340000000001E+2", convertToString(873.1834, 0, 0)); + ASSERT_EQ("1.7976931348623157E+308", + convertToString(1.7976931348623157E+308, 0, 0)); } TEST(APFloatTest, toInteger) { _______________________________________________ llvm-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
TestExpr.patch
Description: TestExpr.patch
_______________________________________________ lldb-dev mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev
