craig.topper added inline comments.
================
Comment at: lib/Sema/SemaChecking.cpp:110
+/// are usually useless
+static unsigned AdjustPrecision(unsigned precision) {
+ return (precision * 59 + 195) / 196;
----------------
scanon wrote:
> erichkeane wrote:
> > Hmm.... I don't terribly understand how this function works. Also, comment
> > above needs to end in a period.
> >
> > Can you elaborate further as to how this works? Those are 3 pretty suspect
> > looking magic numbers...
> It's attempting to compute the number of good base-10 digits (59/196 ~=
> log2(10)). We should really just make APFloat print the shortest
> round-trippable digit sequence instead. Yes, this is tricky to implement, but
> we don't need to implement it. There are two recent high-quality
> implementations available, which are both significantly faster than previous
> algorithms: Ryu and Swift's
> (https://github.com/apple/swift/blob/master/stdlib/public/runtime/SwiftDtoa.cpp).
> Swift's has the virtue of already being used in LLVM-family languages and
> having a tidy single-file implementation, but either would be perfectly
> usable, I think.
>
> Neither supports float128 yet, but we could simply drop them in for float,
> double, and float80.
Function names should start with lower case I think?
================
Comment at: lib/Sema/SemaChecking.cpp:111
+static unsigned AdjustPrecision(unsigned precision) {
+ return (precision * 59 + 195) / 196;
+}
----------------
Can we use divideCeil from llvm/Support/MathExtras.h?
================
Comment at: lib/Sema/SemaChecking.cpp:10881
+ SmallString<16> PrettySourceValue;
+ unsigned precision = llvm::APFloat::semanticsPrecision(*FloatSem);
+ precision = AdjustPrecision(precision);
----------------
Variable name should start with uppercase
https://reviews.llvm.org/D52835
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
