RESOLVED FIXED186023
[WTF] Add clz32 / clz64 for MSVC
https://bugs.webkit.org/show_bug.cgi?id=186023
Summary [WTF] Add clz32 / clz64 for MSVC
Yusuke Suzuki
Reported 2018-05-27 11:53:30 PDT
[WTF] Add clz32 / clz64 for MSVC
Attachments
Patch (4.79 KB, patch)
2018-05-27 11:55 PDT, Yusuke Suzuki
dbates: review+
Yusuke Suzuki
Comment 1 2018-05-27 11:55:05 PDT
Daniel Bates
Comment 2 2018-05-27 20:50:46 PDT
Comment on attachment 341428 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=341428&action=review > Source/WTF/wtf/MathExtras.h:534 > + // Visual Studio 2008 or upper have __lzcnt But we can't detect AVX at compile time. upper => later “ But” => “, but” What does AVX stand for? > Source/WTF/wtf/MathExtras.h:535 > + // So we use bsr to calculate clz Is bsr an industry standard shorthand for _BitScanReverse()? If not, I suggest writing it out. > Source/WTF/wtf/MathExtras.h:559 > + // Visual Studio 2008 or upper have __lzcnt But we can't detect AVX at compile time. Sam remark as line 534. > Source/WTF/wtf/MathExtras.h:560 > + // So we use bsr to calculate clz Same remark as for line 535.
Yusuke Suzuki
Comment 3 2018-05-28 05:24:16 PDT
Comment on attachment 341428 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=341428&action=review Thank you! >> Source/WTF/wtf/MathExtras.h:534 >> + // Visual Studio 2008 or upper have __lzcnt But we can't detect AVX at compile time. > > upper => later > “ But” => “, but” > > What does AVX stand for? AVX is Intel AVX, which introduces lzcnt instruction to the CPU. I've changed this to `Intel AVX`. >> Source/WTF/wtf/MathExtras.h:535 >> + // So we use bsr to calculate clz > > Is bsr an industry standard shorthand for _BitScanReverse()? If not, I suggest writing it out. bsr and lzcnt are Intel's instruction's name. _BitScanReverse is a compiler intrinsic for emitting this `bsr` instruction. I've changed this to `bit-scan-reverse` operation. >> Source/WTF/wtf/MathExtras.h:559 >> + // Visual Studio 2008 or upper have __lzcnt But we can't detect AVX at compile time. > > Sam remark as line 534. Fixed. >> Source/WTF/wtf/MathExtras.h:560 >> + // So we use bsr to calculate clz > > Same remark as for line 535. Fixed.
Yusuke Suzuki
Comment 4 2018-05-28 05:26:18 PDT
Radar WebKit Bug Importer
Comment 5 2018-05-28 05:27:15 PDT
Yusuke Suzuki
Comment 6 2018-05-29 10:56:52 PDT
Note You need to log in before you can comment on or make changes to this bug.