[llvm] r189442 - Change default # of digits for APFloat::toString

[Charles Davis]

On Aug 27, 2013, at 11:23 PM, Eli Friedman wrote:

> Author: efriedma
> Date: Wed Aug 28 00:23:51 2013
> New Revision: 189442
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=189442&view=rev
> Log:
> Change default # of digits for APFloat::toString
> 
> 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.
This is causing (I believe) the following Clang test failures for at least me:

    Clang :: CXX/expr/expr.const/p2-0x.cpp
    Clang :: CXX/temp/temp.spec/p5.cpp
    Clang :: PCH/exprs.c
    Clang :: PCH/floating-literal.c
    Clang :: PCH/objc_literals.m
    Clang :: Rewriter/objc-modern-boxing.mm
    Clang :: Rewriter/objc-modern-numeric-literal.mm
    Clang :: Sema/array-init.c
    Clang :: Sema/knr-def-call.c
    Clang :: SemaCXX/warn-literal-conversion.cpp
    Clang :: SemaTemplate/member-template-access-expr.cpp

As far as I can tell, all the failures are caused by the greater precision printed out. Here's the output from the first one:

FAIL: Clang :: CXX/expr/expr.const/p2-0x.cpp (603 of 15277)
******************** TEST 'Clang :: CXX/expr/expr.const/p2-0x.cpp' FAILED ********************
Script:
--
/Users/chip/llvm-git/build1/Release+Asserts/bin/clang -cc1 -internal-isystem /Users/chip/llvm-git/build1/Release+Asserts/bin/../lib/clang/3.4/include -fsyntax-only -std=c++11 -pedantic -verify -fcxx-exceptions /Users/chip/llvm-git/tools/clang/test/CXX/expr/expr.const/p2-0x.cpp -fconstexpr-depth 128 -triple i686-pc-linux-gnu
--
Exit Code: 1

Command Output (stderr):
--
error: 'note' diagnostics expected but not seen:
  File /Users/chip/llvm-git/tools/clang/test/CXX/expr/expr.const/p2-0x.cpp Line 139: value 1.0E+300 is outside the range of representable values of type 'float'
error: 'note' diagnostics seen but not expected:
  File /Users/chip/llvm-git/tools/clang/test/CXX/expr/expr.const/p2-0x.cpp Line 139: value 1.0000000000000001E+300 is outside the range of representable values of type 'float'
2 errors generated.

--

Did you remember to also update Clang?

Chip

> 
> 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=189442&r1=189441&r2=189442&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Support/APFloat.cpp (original)
> +++ llvm/trunk/lib/Support/APFloat.cpp Wed Aug 28 00:23:51 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=189442&r1=189441&r2=189442&view=diff
> ==============================================================================
> --- llvm/trunk/unittests/ADT/APFloatTest.cpp (original)
> +++ llvm/trunk/unittests/ADT/APFloatTest.cpp Wed Aug 28 00:23:51 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
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits