|
|
DescriptionAdd strtod test cases
Adds test cases from
http://www.exploringbinary.com/how-strtod-works-and-sometimes-doesnt/.
These currently fail on (see ps#1):
- linux_chromium_gn_chromeos_rel
- linux_chromium_asan_rel_ng
- linux_chromium_chromeos_ozone_rel_ng
- linux_chromium_rel_ng
- cast_shell_linux
but despite history appear to be fine on Windows (as of VS2015). As they
were a bit of fiddling to get up and running, and we don't appear to have
any similar tests for dmg_fp, add these tests so that we can hopefully
drop dmg_fp in the future once std::strtod works more consistently
everywhere. https://crbug.com/95729 suggests another solution where it'd
be good to have these tests.
BUG=593512, 615142, 616062, 588726, 95729
Committed: https://crrev.com/8d7634d843a6b981d4b9bebe8f94412b6bccc3be
Cr-Commit-Position: refs/heads/master@{#399948}
Patch Set 1 #Patch Set 2 : don't change to std #
Total comments: 5
Patch Set 3 : use hexfloat instead #Patch Set 4 : raw fp as uint64 #
Total comments: 2
Patch Set 5 : bit_cast #Patch Set 6 : bit_cast for real #Messages
Total messages: 27 (14 generated)
Description was changed from ========== Switch from dmg_fp::strtod to std::strtod Adds test cases from http://www.exploringbinary.com/how-strtod-works-and-sometimes-doesnt/ and switches DoubleToString() to use std::strtod. (Doesn't change other dmg_fp usage, yet.) BUG=593512,615142,616062,588726,95729 ========== to ========== Switch from dmg_fp::strtod to std::strtod Adds test cases from http://www.exploringbinary.com/how-strtod-works-and-sometimes-doesnt/. These currently fail on: - linux_chromium_gn_chromeos_rel - linux_chromium_chromeos_ozone_rel_ng - cast_shell_linux but despite history appear to be fine on Windows. As they were a bit of work to get up and running, and we don't appear to have any similar tests for dmg_fp, add these tests so that we can hopefully drop dmg_fp in the future once std::strtod works more consistently everywhere. BUG=593512,615142,616062,588726,95729 ==========
Description was changed from ========== Switch from dmg_fp::strtod to std::strtod Adds test cases from http://www.exploringbinary.com/how-strtod-works-and-sometimes-doesnt/. These currently fail on: - linux_chromium_gn_chromeos_rel - linux_chromium_chromeos_ozone_rel_ng - cast_shell_linux but despite history appear to be fine on Windows. As they were a bit of work to get up and running, and we don't appear to have any similar tests for dmg_fp, add these tests so that we can hopefully drop dmg_fp in the future once std::strtod works more consistently everywhere. BUG=593512,615142,616062,588726,95729 ========== to ========== Add strtod test cases Adds test cases from http://www.exploringbinary.com/how-strtod-works-and-sometimes-doesnt/. These currently fail on: - linux_chromium_gn_chromeos_rel - linux_chromium_chromeos_ozone_rel_ng - cast_shell_linux but despite history appear to be fine on Windows. As they were a bit of work to get up and running, and we don't appear to have any similar tests for dmg_fp, add these tests so that we can hopefully drop dmg_fp in the future once std::strtod works more consistently everywhere. BUG=593512,615142,616062,588726,95729 ==========
Description was changed from ========== Add strtod test cases Adds test cases from http://www.exploringbinary.com/how-strtod-works-and-sometimes-doesnt/. These currently fail on: - linux_chromium_gn_chromeos_rel - linux_chromium_chromeos_ozone_rel_ng - cast_shell_linux but despite history appear to be fine on Windows. As they were a bit of work to get up and running, and we don't appear to have any similar tests for dmg_fp, add these tests so that we can hopefully drop dmg_fp in the future once std::strtod works more consistently everywhere. BUG=593512,615142,616062,588726,95729 ========== to ========== Add strtod test cases Adds test cases from http://www.exploringbinary.com/how-strtod-works-and-sometimes-doesnt/. These currently fail on: - linux_chromium_gn_chromeos_rel - linux_chromium_chromeos_ozone_rel_ng - cast_shell_linux but despite history appear to be fine on Windows. As they were a bit of work to get up and running, and we don't appear to have any similar tests for dmg_fp, add these tests so that we can hopefully drop dmg_fp in the future once std::strtod works more consistently everywhere. BUG=593512,615142,616062,588726,95729 ==========
Description was changed from ========== Add strtod test cases Adds test cases from http://www.exploringbinary.com/how-strtod-works-and-sometimes-doesnt/. These currently fail on: - linux_chromium_gn_chromeos_rel - linux_chromium_chromeos_ozone_rel_ng - cast_shell_linux but despite history appear to be fine on Windows. As they were a bit of work to get up and running, and we don't appear to have any similar tests for dmg_fp, add these tests so that we can hopefully drop dmg_fp in the future once std::strtod works more consistently everywhere. BUG=593512,615142,616062,588726,95729 ========== to ========== Add strtod test cases Adds test cases from http://www.exploringbinary.com/how-strtod-works-and-sometimes-doesnt/. These currently fail on: - linux_chromium_gn_chromeos_rel - linux_chromium_chromeos_ozone_rel_ng - cast_shell_linux but despite history appear to be fine on Windows. As they were a bit of fiddling to get up and running, and we don't appear to have any similar tests for dmg_fp, add these tests so that we can hopefully drop dmg_fp in the future once std::strtod works more consistently everywhere. BUG=593512,615142,616062,588726,95729 ==========
Description was changed from ========== Add strtod test cases Adds test cases from http://www.exploringbinary.com/how-strtod-works-and-sometimes-doesnt/. These currently fail on: - linux_chromium_gn_chromeos_rel - linux_chromium_chromeos_ozone_rel_ng - cast_shell_linux but despite history appear to be fine on Windows. As they were a bit of fiddling to get up and running, and we don't appear to have any similar tests for dmg_fp, add these tests so that we can hopefully drop dmg_fp in the future once std::strtod works more consistently everywhere. BUG=593512,615142,616062,588726,95729 ========== to ========== Add strtod test cases Adds test cases from http://www.exploringbinary.com/how-strtod-works-and-sometimes-doesnt/. These currently fail on: - linux_chromium_gn_chromeos_rel - linux_chromium_chromeos_ozone_rel_ng - cast_shell_linux but despite history appear to be fine on Windows (as of VS2015). As they were a bit of fiddling to get up and running, and we don't appear to have any similar tests for dmg_fp, add these tests so that we can hopefully drop dmg_fp in the future once std::strtod works more consistently everywhere. BUG=593512,615142,616062,588726,95729 ==========
scottmg@chromium.org changed reviewers: + thakis@chromium.org
Description was changed from ========== Add strtod test cases Adds test cases from http://www.exploringbinary.com/how-strtod-works-and-sometimes-doesnt/. These currently fail on: - linux_chromium_gn_chromeos_rel - linux_chromium_chromeos_ozone_rel_ng - cast_shell_linux but despite history appear to be fine on Windows (as of VS2015). As they were a bit of fiddling to get up and running, and we don't appear to have any similar tests for dmg_fp, add these tests so that we can hopefully drop dmg_fp in the future once std::strtod works more consistently everywhere. BUG=593512,615142,616062,588726,95729 ========== to ========== Add strtod test cases Adds test cases from http://www.exploringbinary.com/how-strtod-works-and-sometimes-doesnt/. These currently fail on: - linux_chromium_gn_chromeos_rel - linux_chromium_chromeos_ozone_rel_ng - linux_chromium_rel_ng - cast_shell_linux but despite history appear to be fine on Windows (as of VS2015). As they were a bit of fiddling to get up and running, and we don't appear to have any similar tests for dmg_fp, add these tests so that we can hopefully drop dmg_fp in the future once std::strtod works more consistently everywhere. BUG=593512,615142,616062,588726,95729 ==========
Description was changed from ========== Add strtod test cases Adds test cases from http://www.exploringbinary.com/how-strtod-works-and-sometimes-doesnt/. These currently fail on: - linux_chromium_gn_chromeos_rel - linux_chromium_chromeos_ozone_rel_ng - linux_chromium_rel_ng - cast_shell_linux but despite history appear to be fine on Windows (as of VS2015). As they were a bit of fiddling to get up and running, and we don't appear to have any similar tests for dmg_fp, add these tests so that we can hopefully drop dmg_fp in the future once std::strtod works more consistently everywhere. BUG=593512,615142,616062,588726,95729 ========== to ========== Add strtod test cases Adds test cases from http://www.exploringbinary.com/how-strtod-works-and-sometimes-doesnt/. These currently fail on: - linux_chromium_gn_chromeos_rel - linux_chromium_asan_rel_ng - linux_chromium_chromeos_ozone_rel_ng - linux_chromium_rel_ng - cast_shell_linux but despite history appear to be fine on Windows (as of VS2015). As they were a bit of fiddling to get up and running, and we don't appear to have any similar tests for dmg_fp, add these tests so that we can hopefully drop dmg_fp in the future once std::strtod works more consistently everywhere. BUG=593512,615142,616062,588726,95729 ==========
Description was changed from ========== Add strtod test cases Adds test cases from http://www.exploringbinary.com/how-strtod-works-and-sometimes-doesnt/. These currently fail on: - linux_chromium_gn_chromeos_rel - linux_chromium_asan_rel_ng - linux_chromium_chromeos_ozone_rel_ng - linux_chromium_rel_ng - cast_shell_linux but despite history appear to be fine on Windows (as of VS2015). As they were a bit of fiddling to get up and running, and we don't appear to have any similar tests for dmg_fp, add these tests so that we can hopefully drop dmg_fp in the future once std::strtod works more consistently everywhere. BUG=593512,615142,616062,588726,95729 ========== to ========== Add strtod test cases Adds test cases from http://www.exploringbinary.com/how-strtod-works-and-sometimes-doesnt/. These currently fail on (see ps#1): - linux_chromium_gn_chromeos_rel - linux_chromium_asan_rel_ng - linux_chromium_chromeos_ozone_rel_ng - linux_chromium_rel_ng - cast_shell_linux but despite history appear to be fine on Windows (as of VS2015). As they were a bit of fiddling to get up and running, and we don't appear to have any similar tests for dmg_fp, add these tests so that we can hopefully drop dmg_fp in the future once std::strtod works more consistently everywhere. BUG=593512,615142,616062,588726,95729 ==========
Description was changed from ========== Add strtod test cases Adds test cases from http://www.exploringbinary.com/how-strtod-works-and-sometimes-doesnt/. These currently fail on (see ps#1): - linux_chromium_gn_chromeos_rel - linux_chromium_asan_rel_ng - linux_chromium_chromeos_ozone_rel_ng - linux_chromium_rel_ng - cast_shell_linux but despite history appear to be fine on Windows (as of VS2015). As they were a bit of fiddling to get up and running, and we don't appear to have any similar tests for dmg_fp, add these tests so that we can hopefully drop dmg_fp in the future once std::strtod works more consistently everywhere. BUG=593512,615142,616062,588726,95729 ========== to ========== Add strtod test cases Adds test cases from http://www.exploringbinary.com/how-strtod-works-and-sometimes-doesnt/. These currently fail on (see ps#1): - linux_chromium_gn_chromeos_rel - linux_chromium_asan_rel_ng - linux_chromium_chromeos_ozone_rel_ng - linux_chromium_rel_ng - cast_shell_linux but despite history appear to be fine on Windows (as of VS2015). As they were a bit of fiddling to get up and running, and we don't appear to have any similar tests for dmg_fp, add these tests so that we can hopefully drop dmg_fp in the future once std::strtod works more consistently everywhere. https://crbug.com/95729 suggests another solution where it'd be good to have these tests. BUG=593512,615142,616062,588726,95729 ==========
https://codereview.chromium.org/2044643005/diff/20001/base/strings/string_num... File base/strings/string_number_conversions_unittest.cc (right): https://codereview.chromium.org/2044643005/diff/20001/base/strings/string_num... base/strings/string_number_conversions_unittest.cc:808: // http://www.exploringbinary.com/converting-floating-point-numbers-to-binary-st... that doesn't say anything about license or "ok to use" :-/ https://codereview.chromium.org/2044643005/diff/20001/base/strings/string_num... base/strings/string_number_conversions_unittest.cc:864: "100000101111001101100111011000101011101110110000110100.0"}, could we use hexfloats for the expecteds instead?
(ps: hooray for doing this!)
https://codereview.chromium.org/2044643005/diff/20001/base/strings/string_num... File base/strings/string_number_conversions_unittest.cc (right): https://codereview.chromium.org/2044643005/diff/20001/base/strings/string_num... base/strings/string_number_conversions_unittest.cc:808: // http://www.exploringbinary.com/converting-floating-point-numbers-to-binary-st... On 2016/06/08 00:45:09, Nico wrote: > that doesn't say anything about license or "ok to use" :-/ Hmm, true. I'll see if I can find something similar with a license, or otherwise try to get in touch with the author. https://codereview.chromium.org/2044643005/diff/20001/base/strings/string_num... base/strings/string_number_conversions_unittest.cc:864: "100000101111001101100111011000101011101110110000110100.0"}, On 2016/06/08 00:45:09, Nico wrote: > could we use hexfloats for the expecteds instead? VS doesn't seem to support hex floating point literals (at least in C++, I didn't check a .c file specifically) Or do you mean just using a base-16 string representation so they're not so long?
https://codereview.chromium.org/2044643005/diff/20001/base/strings/string_num... File base/strings/string_number_conversions_unittest.cc (right): https://codereview.chromium.org/2044643005/diff/20001/base/strings/string_num... base/strings/string_number_conversions_unittest.cc:864: "100000101111001101100111011000101011101110110000110100.0"}, On 2016/06/08 02:28:18, scottmg wrote: > On 2016/06/08 00:45:09, Nico wrote: > > could we use hexfloats for the expecteds instead? > > VS doesn't seem to support hex floating point literals (at least in C++, I > didn't check a .c file specifically) No such luck. d:\src\x>type fp.c #include <stdio.h> int main() { double x = 0x1.82db34012b251p-77; printf("%a, %e\n", x, x); } d:\src\x>cl /nologo /Bv fp.c Compiler Passes: C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\BIN\amd64_x86\cl.exe: Version 19.00.23918.0 C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\BIN\amd64_x86\c1.dll: Version 19.00.23918.0 C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\BIN\amd64_x86\c1xx.dll: Version 19.00.23918.0 C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\BIN\amd64_x86\c2.dll: Version 19.00.23918.0 C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\BIN\amd64_x86\link.exe: Version 14.00.23918.0 C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\BIN\amd64\mspdb140.dll: Version 14.00.23918.0 C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\BIN\amd64_x86\1033\clui.dll: Version 19.00.23918.0 fp.c fp.c(3): error C2059: syntax error: 'bad suffix on number' > Or do you mean just using a base-16 string representation so they're not so > long?
On 2016/06/08 02:36:04, scottmg wrote: > https://codereview.chromium.org/2044643005/diff/20001/base/strings/string_num... > File base/strings/string_number_conversions_unittest.cc (right): > > https://codereview.chromium.org/2044643005/diff/20001/base/strings/string_num... > base/strings/string_number_conversions_unittest.cc:864: > "100000101111001101100111011000101011101110110000110100.0"}, > On 2016/06/08 02:28:18, scottmg wrote: > > On 2016/06/08 00:45:09, Nico wrote: > > > could we use hexfloats for the expecteds instead? > > > > VS doesn't seem to support hex floating point literals (at least in C++, I > > didn't check a .c file specifically) > > No such luck. > > d:\src\x>type fp.c > #include <stdio.h> > int main() { > double x = 0x1.82db34012b251p-77; > printf("%a, %e\n", x, x); > } > > d:\src\x>cl /nologo /Bv fp.c > Compiler Passes: > C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\BIN\amd64_x86\cl.exe: > Version 19.00.23918.0 > C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\BIN\amd64_x86\c1.dll: > Version 19.00.23918.0 > C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\BIN\amd64_x86\c1xx.dll: > Version 19.00.23918.0 > C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\BIN\amd64_x86\c2.dll: > Version 19.00.23918.0 > C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\BIN\amd64_x86\link.exe: > Version 14.00.23918.0 > C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\BIN\amd64\mspdb140.dll: > Version 14.00.23918.0 > C:\Program Files (x86)\Microsoft Visual Studio > 14.0\VC\BIN\amd64_x86\1033\clui.dll: Version 19.00.23918.0 > > fp.c > fp.c(3): error C2059: syntax error: 'bad suffix on number' > It does support %a in formatting though. Odd! OK, I'll rework to use that instead. > > Or do you mean just using a base-16 string representation so they're not so > > long?
Updated to just compare the raw double representation (no point in going back to a string or hexfloat representation since we're just testing strtod). https://codereview.chromium.org/2063273002 is the same tests, but with std::strtod substituted for dmg_fp::strtod to see where it fails still.
lgtm with one line changed. It's kind of impressive that string->float conversions are deterministic through all bits on all platforms! https://codereview.chromium.org/2044643005/diff/60001/base/strings/string_num... File base/strings/string_number_conversions_unittest.cc (right): https://codereview.chromium.org/2044643005/diff/60001/base/strings/string_num... base/strings/string_number_conversions_unittest.cc:809: return *reinterpret_cast<uint64_t*>(&fp); please use base/bit_cast.h instead
> It's kind of impressive that string->float conversions are deterministic through > all bits on all platforms! Only with dmg_fp unfortunately! :) https://codereview.chromium.org/2044643005/diff/60001/base/strings/string_num... File base/strings/string_number_conversions_unittest.cc (right): https://codereview.chromium.org/2044643005/diff/60001/base/strings/string_num... base/strings/string_number_conversions_unittest.cc:809: return *reinterpret_cast<uint64_t*>(&fp); On 2016/06/15 09:06:59, Nico (traveling...slow) wrote: > please use base/bit_cast.h instead Done. (inline)
The CQ bit was checked by scottmg@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from thakis@chromium.org Link to the patchset: https://codereview.chromium.org/2044643005/#ps100001 (title: "bit_cast for real")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2044643005/100001
Message was sent while issue was closed.
Description was changed from ========== Add strtod test cases Adds test cases from http://www.exploringbinary.com/how-strtod-works-and-sometimes-doesnt/. These currently fail on (see ps#1): - linux_chromium_gn_chromeos_rel - linux_chromium_asan_rel_ng - linux_chromium_chromeos_ozone_rel_ng - linux_chromium_rel_ng - cast_shell_linux but despite history appear to be fine on Windows (as of VS2015). As they were a bit of fiddling to get up and running, and we don't appear to have any similar tests for dmg_fp, add these tests so that we can hopefully drop dmg_fp in the future once std::strtod works more consistently everywhere. https://crbug.com/95729 suggests another solution where it'd be good to have these tests. BUG=593512,615142,616062,588726,95729 ========== to ========== Add strtod test cases Adds test cases from http://www.exploringbinary.com/how-strtod-works-and-sometimes-doesnt/. These currently fail on (see ps#1): - linux_chromium_gn_chromeos_rel - linux_chromium_asan_rel_ng - linux_chromium_chromeos_ozone_rel_ng - linux_chromium_rel_ng - cast_shell_linux but despite history appear to be fine on Windows (as of VS2015). As they were a bit of fiddling to get up and running, and we don't appear to have any similar tests for dmg_fp, add these tests so that we can hopefully drop dmg_fp in the future once std::strtod works more consistently everywhere. https://crbug.com/95729 suggests another solution where it'd be good to have these tests. BUG=593512,615142,616062,588726,95729 ==========
Message was sent while issue was closed.
Committed patchset #6 (id:100001)
Message was sent while issue was closed.
CQ bit was unchecked
Message was sent while issue was closed.
Description was changed from ========== Add strtod test cases Adds test cases from http://www.exploringbinary.com/how-strtod-works-and-sometimes-doesnt/. These currently fail on (see ps#1): - linux_chromium_gn_chromeos_rel - linux_chromium_asan_rel_ng - linux_chromium_chromeos_ozone_rel_ng - linux_chromium_rel_ng - cast_shell_linux but despite history appear to be fine on Windows (as of VS2015). As they were a bit of fiddling to get up and running, and we don't appear to have any similar tests for dmg_fp, add these tests so that we can hopefully drop dmg_fp in the future once std::strtod works more consistently everywhere. https://crbug.com/95729 suggests another solution where it'd be good to have these tests. BUG=593512,615142,616062,588726,95729 ========== to ========== Add strtod test cases Adds test cases from http://www.exploringbinary.com/how-strtod-works-and-sometimes-doesnt/. These currently fail on (see ps#1): - linux_chromium_gn_chromeos_rel - linux_chromium_asan_rel_ng - linux_chromium_chromeos_ozone_rel_ng - linux_chromium_rel_ng - cast_shell_linux but despite history appear to be fine on Windows (as of VS2015). As they were a bit of fiddling to get up and running, and we don't appear to have any similar tests for dmg_fp, add these tests so that we can hopefully drop dmg_fp in the future once std::strtod works more consistently everywhere. https://crbug.com/95729 suggests another solution where it'd be good to have these tests. BUG=593512,615142,616062,588726,95729 Committed: https://crrev.com/8d7634d843a6b981d4b9bebe8f94412b6bccc3be Cr-Commit-Position: refs/heads/master@{#399948} ==========
Message was sent while issue was closed.
Patchset 6 (id:??) landed as https://crrev.com/8d7634d843a6b981d4b9bebe8f94412b6bccc3be Cr-Commit-Position: refs/heads/master@{#399948} |