rsmith added inline comments.
================ Comment at: lib/Sema/SemaChecking.cpp:110 +/// are usually useless +static unsigned AdjustPrecision(unsigned precision) { + return (precision * 59 + 195) / 196; ---------------- xbolva00 wrote: > xbolva00 wrote: > > xbolva00 wrote: > > > craig.topper wrote: > > > > 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? > > > float80 has precision = 64 so we can put MAX value of unsigned long long > > > into it with no issues, or? > > Other static functions here use upper case, e.g. "PrettyPrintInRange", so > > I follow it :) > > float80 has precision = 64 so we can put MAX value of unsigned long long > > into it with no issues, or? > > ah, int128_t LLVM will need to carry an implementation of the "shortest round-trippable digit sequence" soon regardless of what we do here, because it's [part of the C++17 standard library](https://en.cppreference.com/w/cpp/utility/to_chars). We should aim for an implementation that can be shared by `APFloat` and libc++. https://reviews.llvm.org/D52835 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits