xbolva00 added inline comments.
================
Comment at: lib/Sema/SemaChecking.cpp:110
+/// are usually useless
+static unsigned AdjustPrecision(unsigned precision) {
+ return (precision * 59 + 195) / 196;
----------------
rsmith wrote:
> 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++.
You are right. But anyway, it should not block this patch, no? I can put TODO
comment there.
https://reviews.llvm.org/D52835
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
