[PATCH] LNT ARM sqlite3

Daniel Dunbar daniel at zuster.org
Thu Mar 28 11:10:28 PDT 2013


Hi Renato,


On Tue, Mar 26, 2013 at 10:26 AM, Renato Golin <rengolin at systemcall.org>wrote:

> Hi folks,
>
> Been working on sqlite this week and found a few issues.
>
> First, the results from avg(b) is different on x86_64 and ARM because
> "long double" is f128 on the former and f64 on the latter in regards
> to vxprintf() function. That alone would warrant more care while
> writing code, but it seems that vxprintf() is the best example I've
> seen so far on what NOT to do with floating points, big or small.
>
> Since that function has a lot of dependencies, I was forced to
> improvise and came up with the "sqlite3.patch" attached. It was then
> that I found another bug, in fpcmp.
>
> I was getting the values close enough, so I set FP_TOLERANCE to 1e-8
> but fpcmp was printing me errors within 1e+3!! That was no FP bug, and
> upon investigation, I found that fpcmp was comparing two different
> lines...
>
> Not wanting to go down that route, I prepared the second patch
> (sqlite3-results.patch) which simple apply the SQL function
> "round(avg(b), 8)" and removed the FP comparison altogether, achieving
> the same result without hitting the fpcmp bug.
>
> I know it was cowardice, but I didn't want to fix yet another bug (on
> unknown territory) while getting the test-suite green, sorry about
> that... :(
>

I'm fine with this form of cowardice, as long as the changes can be
reasonably expected to not hide a potential correctness bug (and sqlite is
not a benchmark I anticipate to be the one to expose FP math errors). I
made a large number of these changes to try and stabilize the output of
benchmarks as part of moving to using reference outputs.

Please still file the bug with the fpcmp utility, with the two inputs that
show the problem.

One thing I do find a little odd is that all of the tests in the test suite
pass on ARM on iOS, so I'm a little curious what platform/architecture
difference causes you to see this. Any ideas?

 - Daniel


> So, applying sqlite3-results.patch will make it pass on both x86_64
> and ARM, and most likely any other 32-bit and 64-bit machines out
> there. Any objections?
>
> cheers,
> --renato
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130328/0b035ec9/attachment.html>


More information about the llvm-commits mailing list