Bug 196162

Summary:

Refactor clz/ctz and fix getLSBSet.

Product:

WebKit

Reporter:

Keith Miller <keith_miller>

Component:

New Bugs

Assignee:

Keith Miller <keith_miller>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

benjamin, cdumez, cmarcelo, commit-queue, dbates, ews-watchlist, fujii.hironori, mark.lam, msaboff, saam, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none
Patch for landing	none

Keith Miller

Reported 2019-03-22 15:51:32 PDT

Refactor clz/ctz and fix getLSBSet.

Attachments
Patch (15.62 KB, patch) 2019-03-22 16:04 PDT, Keith Miller	no flags	Details Formatted Diff Diff
Patch (15.64 KB, patch) 2019-03-22 16:21 PDT, Keith Miller	no flags	Details Formatted Diff Diff
Patch (15.77 KB, patch) 2019-03-22 16:25 PDT, Keith Miller	no flags	Details Formatted Diff Diff
Patch for landing (15.97 KB, patch) 2019-03-23 17:54 PDT, Keith Miller	no flags	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Keith Miller

Comment 1 2019-03-22 16:04:38 PDT

Created attachment 365770 [details] Patch

Saam Barati

Comment 2 2019-03-22 16:13:31 PDT

Comment on attachment 365770 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=365770&action=review > Source/WTF/ChangeLog:12 > + numeric type. Since these methods work on any type we don't need > + to have an, incorrect, implementation of getLSBSet. This patch > + also adds getMSBSet as there may be users who want that in the > + future. I'd explain a bit more here. - The old implementation of getLSBSet was really getMSBSet > Tools/TestWebKitAPI/Tests/WTF/MathExtras.cpp:435 > nit: I think writing out these numbers in binary would make it easier to follow.

Keith Miller

Comment 3 2019-03-22 16:21:17 PDT

Created attachment 365774 [details] Patch

Keith Miller

Comment 4 2019-03-22 16:25:28 PDT

Created attachment 365775 [details] Patch

Keith Miller

Comment 5 2019-03-23 17:54:04 PDT

Created attachment 365822 [details] Patch for landing

WebKit Commit Bot

Comment 6 2019-03-23 18:32:12 PDT

Comment on attachment 365822 [details] Patch for landing Clearing flags on attachment: 365822 Committed r243418: <https://trac.webkit.org/changeset/243418>

WebKit Commit Bot

Comment 7 2019-03-23 18:32:14 PDT

All reviewed patches have been landed. Closing bug.

Radar WebKit Bug Importer

Comment 8 2019-03-23 18:34:23 PDT

<rdar://problem/49191697>

Fujii Hironori

Comment 9 2019-03-24 19:17:32 PDT

It breaks x64 Windows builds. https://build.webkit.org/builders/WinCairo%2064-bit%20WKL%20Debug%20%28Build%29/builds/7601/steps/compile-webkit/logs/stdio > [21/1544] Building CXX object Tools\TestWebKitAPI\CMakeFiles\TestWTFLib.dir\Tests\WTF\MathExtras.cpp.obj > FAILED: Tools/TestWebKitAPI/CMakeFiles/TestWTFLib.dir/Tests/WTF/MathExtras.cpp.obj > "C:\Program Files (x86)\Microsoft Visual Studio\2017\BuildTools\VC\Tools\MSVC\14.14.26428\bin\Hostx64\x64\cl.exe" (...) /FdTools\TestWebKitAPI\CMakeFiles\TestWTFLib.dir\ /FS -c ..\..\Tools\TestWebKitAPI\Tests\WTF\MathExtras.cpp > C:\WebKit-BuildWorker\wincairo-wkl-debug\build\WebKitBuild\Debug\DerivedSources\ForwardingHeaders\wtf/MathExtras.h(636): error C2065: 'number': undeclared identifier

WebKit Commit Bot

Comment 10 2019-03-24 19:45:43 PDT

Re-opened since this is blocked by bug 196191

Keith Miller

Comment 11 2019-03-24 19:48:42 PDT

(In reply to Fujii Hironori from comment #9) > It breaks x64 Windows builds. > > https://build.webkit.org/builders/WinCairo%2064- > bit%20WKL%20Debug%20%28Build%29/builds/7601/steps/compile-webkit/logs/stdio > > > [21/1544] Building CXX object Tools\TestWebKitAPI\CMakeFiles\TestWTFLib.dir\Tests\WTF\MathExtras.cpp.obj > > FAILED: Tools/TestWebKitAPI/CMakeFiles/TestWTFLib.dir/Tests/WTF/MathExtras.cpp.obj > > "C:\Program Files (x86)\Microsoft Visual Studio\2017\BuildTools\VC\Tools\MSVC\14.14.26428\bin\Hostx64\x64\cl.exe" (...) /FdTools\TestWebKitAPI\CMakeFiles\TestWTFLib.dir\ /FS -c ..\..\Tools\TestWebKitAPI\Tests\WTF\MathExtras.cpp > > C:\WebKit-BuildWorker\wincairo-wkl-debug\build\WebKitBuild\Debug\DerivedSources\ForwardingHeaders\wtf/MathExtras.h(636): error C2065: 'number': undeclared identifier Whoops, I think it's a simple fix. I forgot to change the variable name for windows...

Keith Miller

Comment 12 2019-03-24 19:53:58 PDT

Should hopefully be fixed with: https://trac.webkit.org/changeset/243429

Note You need to log in before you can comment on or make changes to this bug.