Index: include/clang/AST/Type.h =================================================================== --- include/clang/AST/Type.h +++ include/clang/AST/Type.h @@ -6615,9 +6615,8 @@ // Get the decimal string representation of a fixed point type, represented // as a scaled integer. -void FixedPointValueToString(SmallVectorImpl &Str, - const llvm::APSInt &Val, - unsigned Scale, unsigned Radix); +void FixedPointValueToString(SmallVectorImpl &Str, llvm::APSInt Val, + unsigned Scale); } // namespace clang Index: lib/AST/Expr.cpp =================================================================== --- lib/AST/Expr.cpp +++ lib/AST/Expr.cpp @@ -785,7 +785,7 @@ // which is 43 characters. SmallString<64> S; FixedPointValueToString( - S, llvm::APSInt::getUnsigned(getValue().getZExtValue()), Scale, Radix); + S, llvm::APSInt::getUnsigned(getValue().getZExtValue()), Scale); return S.str(); } Index: lib/AST/ExprConstant.cpp =================================================================== --- lib/AST/ExprConstant.cpp +++ lib/AST/ExprConstant.cpp @@ -9547,8 +9547,7 @@ if (Value.isSigned() && Value.isMinSignedValue() && E->canOverflow()) { SmallString<64> S; FixedPointValueToString(S, Value, - Info.Ctx.getTypeInfo(E->getType()).Width, - /*Radix=*/10); + Info.Ctx.getTypeInfo(E->getType()).Width); Info.CCEDiag(E, diag::note_constexpr_overflow) << S << E->getType(); if (Info.noteUndefinedBehavior()) return false; } Index: lib/AST/Type.cpp =================================================================== --- lib/AST/Type.cpp +++ lib/AST/Type.cpp @@ -4026,17 +4026,26 @@ } void clang::FixedPointValueToString(SmallVectorImpl &Str, - const llvm::APSInt &Val, unsigned Scale, - unsigned Radix) { - llvm::APSInt ScaleVal = llvm::APSInt::getUnsigned(1ULL << Scale); - llvm::APSInt IntPart = Val / ScaleVal; - llvm::APSInt FractPart = Val % ScaleVal; - llvm::APSInt RadixInt = llvm::APSInt::getUnsigned(Radix); - - IntPart.toString(Str, Radix); + llvm::APSInt Val, unsigned Scale) { + if (Val.isSigned() && Val.isNegative() && Val != -Val) { + Val = -Val; + Str.push_back('-'); + } + + llvm::APSInt IntPart = Val >> Scale; + + // Add 4 digits to hold the value after multiplying 10 (the radix) + unsigned Width = Val.getBitWidth() + 4; + llvm::APInt FractPart = Val.zextOrTrunc(Scale).zext(Width); + llvm::APInt FractPartMask = llvm::APInt::getAllOnesValue(Scale).zext(Width); + llvm::APInt RadixInt = llvm::APInt(Width, 10); + + IntPart.toString(Str, /*radix=*/10); Str.push_back('.'); do { - (FractPart * RadixInt / ScaleVal).toString(Str, Radix); - FractPart = (FractPart * RadixInt) % ScaleVal; - } while (FractPart.getExtValue()); + (FractPart * RadixInt) + .lshr(Scale) + .toString(Str, /*radix=*/10, Val.isSigned()); + FractPart = (FractPart * RadixInt) & FractPartMask; + } while (FractPart != 0); } Index: test/Frontend/fixed_point_to_string.c =================================================================== --- /dev/null +++ test/Frontend/fixed_point_to_string.c @@ -0,0 +1,21 @@ +// RUN: %clang_cc1 -ast-dump -ffixed-point %s | FileCheck %s +// RUN: %clang_cc1 -ast-dump -ffixed-point -fpadding-on-unsigned-fixed-point %s | FileCheck %s + +/** + * Check the same values are printed in the AST regardless of if unsigned types + * have the same number of fractional bits as signed types. + */ + +unsigned short _Accum u_short_accum = 0.5uhk; +unsigned _Accum u_accum = 0.5uk; +unsigned long _Accum u_long_accum = 0.5ulk; +unsigned short _Fract u_short_fract = 0.5uhr; +unsigned _Fract u_fract = 0.5ur; +unsigned long _Fract u_long_fract = 0.5ulr; + +//CHECK: FixedPointLiteral {{.*}} 'unsigned short _Accum' 0.5 +//CHECK: FixedPointLiteral {{.*}} 'unsigned _Accum' 0.5 +//CHECK: FixedPointLiteral {{.*}} 'unsigned long _Accum' 0.5 +//CHECK: FixedPointLiteral {{.*}} 'unsigned short _Fract' 0.5 +//CHECK: FixedPointLiteral {{.*}} 'unsigned _Fract' 0.5 +//CHECK: FixedPointLiteral {{.*}} 'unsigned long _Fract' 0.5 Index: unittests/Frontend/CMakeLists.txt =================================================================== --- unittests/Frontend/CMakeLists.txt +++ unittests/Frontend/CMakeLists.txt @@ -5,6 +5,7 @@ add_clang_unittest(FrontendTests ASTUnitTest.cpp CompilerInstanceTest.cpp + FixedPointString.cpp FrontendActionTest.cpp CodeGenActionTest.cpp ParsedSourceLocationTest.cpp Index: unittests/Frontend/FixedPointString.cpp =================================================================== --- /dev/null +++ unittests/Frontend/FixedPointString.cpp @@ -0,0 +1,107 @@ +#include "clang/AST/Type.h" +#include "llvm/ADT/APSInt.h" +#include "llvm/ADT/SmallString.h" +#include "gtest/gtest.h" + +using clang::FixedPointValueToString; +using llvm::APSInt; +using llvm::SmallString; + +namespace { + +TEST(FixedPointString, DifferentTypes) { + SmallString<64> S; + FixedPointValueToString(S, APSInt::get(320), 7); + ASSERT_STREQ(S.c_str(), "2.5"); + + S.clear(); + FixedPointValueToString(S, APSInt::get(0), 7); + ASSERT_STREQ(S.c_str(), "0.0"); + + // signed short _Accum + S.clear(); + FixedPointValueToString(S, APSInt::getMaxValue(16, /*Unsigned=*/false), 7); + ASSERT_STREQ(S.c_str(), "255.9921875"); + + // signed _Accum + S.clear(); + FixedPointValueToString(S, APSInt::getMaxValue(32, /*Unsigned=*/false), 15); + ASSERT_STREQ(S.c_str(), "65535.999969482421875"); + + // signed long _Accum + S.clear(); + FixedPointValueToString(S, APSInt::getMaxValue(64, /*Unsigned=*/false), 31); + ASSERT_STREQ(S.c_str(), "4294967295.9999999995343387126922607421875"); + + // unsigned short _Accum + S.clear(); + FixedPointValueToString(S, APSInt::getMaxValue(16, /*Unsigned=*/true), 8); + ASSERT_STREQ(S.c_str(), "255.99609375"); + + // unsigned _Accum + S.clear(); + FixedPointValueToString(S, APSInt::getMaxValue(32, /*Unsigned=*/true), 16); + ASSERT_STREQ(S.c_str(), "65535.9999847412109375"); + + // unsigned long _Accum + S.clear(); + FixedPointValueToString(S, APSInt::getMaxValue(64, /*Unsigned=*/true), 32); + ASSERT_STREQ(S.c_str(), "4294967295.99999999976716935634613037109375"); + + // signed short _Fract + S.clear(); + FixedPointValueToString(S, APSInt::getMaxValue(8, /*Unsigned=*/false), 7); + ASSERT_STREQ(S.c_str(), "0.9921875"); + + // signed _Fract + S.clear(); + FixedPointValueToString(S, APSInt::getMaxValue(16, /*Unsigned=*/false), 15); + ASSERT_STREQ(S.c_str(), "0.999969482421875"); + + // signed long _Fract + S.clear(); + FixedPointValueToString(S, APSInt::getMaxValue(32, /*Unsigned=*/false), 31); + ASSERT_STREQ(S.c_str(), "0.9999999995343387126922607421875"); + + // unsigned short _Fract + S.clear(); + FixedPointValueToString(S, APSInt::getMaxValue(8, /*Unsigned=*/true), 8); + ASSERT_STREQ(S.c_str(), "0.99609375"); + + // unsigned _Fract + S.clear(); + FixedPointValueToString(S, APSInt::getMaxValue(16, /*Unsigned=*/true), 16); + ASSERT_STREQ(S.c_str(), "0.9999847412109375"); + + // unsigned long _Fract + S.clear(); + FixedPointValueToString(S, APSInt::getMaxValue(32, /*Unsigned=*/true), 32); + ASSERT_STREQ(S.c_str(), "0.99999999976716935634613037109375"); +} + +TEST(FixedPointString, Negative) { + SmallString<64> S; + FixedPointValueToString(S, APSInt::get(-320), 7); + ASSERT_STREQ(S.c_str(), "-2.5"); + + S.clear(); + FixedPointValueToString(S, APSInt::get(-64), 7); + ASSERT_STREQ(S.c_str(), "-0.5"); + + // signed short _Accum + S.clear(); + FixedPointValueToString(S, APSInt::getMinValue(16, /*Unsigned=*/false), 7); + ASSERT_STREQ(S.c_str(), "-256.0"); + + // signed _Accum + S.clear(); + FixedPointValueToString(S, APSInt::getMinValue(32, /*Unsigned=*/false), 15); + ASSERT_STREQ(S.c_str(), "-65536.0"); + + // signed long _Accum + S.clear(); + FixedPointValueToString(S, APSInt::getMinValue(64, /*Unsigned=*/false), 31); + ASSERT_STREQ(S.c_str(), "-4294967296.0"); +} + +} // namespace