Bug 196162

Summary: Refactor clz/ctz and fix getLSBSet.
Product: WebKit Reporter: Keith Miller <keith_miller>
Component: New BugsAssignee: Keith Miller <keith_miller>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, cdumez, cmarcelo, commit-queue, dbates, ews-watchlist, Hironori.Fujii, mark.lam, msaboff, saam, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch for landing none

Description Keith Miller 2019-03-22 15:51:32 PDT
Refactor clz/ctz and fix getLSBSet.
Comment 1 Keith Miller 2019-03-22 16:04:38 PDT
Created attachment 365770 [details]
Patch
Comment 2 Saam Barati 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.
Comment 3 Keith Miller 2019-03-22 16:21:17 PDT
Created attachment 365774 [details]
Patch
Comment 4 Keith Miller 2019-03-22 16:25:28 PDT
Created attachment 365775 [details]
Patch
Comment 5 Keith Miller 2019-03-23 17:54:04 PDT
Created attachment 365822 [details]
Patch for landing
Comment 6 WebKit Commit Bot 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>
Comment 7 WebKit Commit Bot 2019-03-23 18:32:14 PDT
All reviewed patches have been landed.  Closing bug.
Comment 8 Radar WebKit Bug Importer 2019-03-23 18:34:23 PDT
<rdar://problem/49191697>
Comment 9 Fujii Hironori 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
Comment 10 WebKit Commit Bot 2019-03-24 19:45:43 PDT
Re-opened since this is blocked by bug 196191
Comment 11 Keith Miller 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...
Comment 12 Keith Miller 2019-03-24 19:53:58 PDT
Should hopefully be fixed with: https://trac.webkit.org/changeset/243429