[webkit-reviews] review granted: [Bug 230437] Add LLVM/FLang's int128_t implementation to WTF : [Attachment 438537] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Sat Sep 18 00:09:15 PDT 2021
Yusuke Suzuki <ysuzuki at apple.com> has granted Philip Chimento
<philip.chimento at gmail.com>'s request for review:
Bug 230437: Add LLVM/FLang's int128_t implementation to WTF
https://bugs.webkit.org/show_bug.cgi?id=230437
Attachment 438537: Patch
https://bugs.webkit.org/attachment.cgi?id=438537&action=review
--- Comment #4 from Yusuke Suzuki <ysuzuki at apple.com> ---
Comment on attachment 438537
--> https://bugs.webkit.org/attachment.cgi?id=438537
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=438537&action=review
r=me with comments
> Source/WTF/wtf/CMakeLists.txt:106
> + Int128.h
Add this header to xcodeproj.
> Source/WTF/wtf/CMakeLists.txt:117
> + LeadingZeroBitCount.h
Add this header to xcodeproj.
> Source/WTF/wtf/LeadingZeroBitCount.h:41
> +static constexpr std::uint64_t deBruijn {0x07edd5e59a4e28c2};
> +static constexpr std::uint8_t mapping[64] {63, 0, 58, 1, 59, 47, 53, 2, 60,
39,
> + 48, 27, 54, 33, 42, 3, 61, 51, 37, 40, 49, 18, 28, 20, 55, 30, 34, 11,
43,
> + 14, 22, 4, 62, 57, 46, 52, 38, 26, 32, 41, 50, 36, 17, 19, 29, 10, 13,
21,
> + 56, 45, 25, 31, 35, 16, 9, 12, 44, 24, 15, 8, 23, 7, 6, 5};
> +} // namespace
You should create cpp file, and putting these constant arrays in C++ side.
Otherwise, each translation unit will materialize this duplicate array, and
enlarging binary size.
> Source/WTF/wtf/LeadingZeroBitCount.h:43
> +inline constexpr int LeadingZeroBitCount(std::uint64_t x)
Rename it to leadingZeroBitCount
> Source/WTF/wtf/LeadingZeroBitCount.h:64
> +inline constexpr int LeadingZeroBitCount(std::uint32_t x)
Ditto.
> Source/WTF/wtf/LeadingZeroBitCount.h:69
> +inline constexpr int LeadingZeroBitCount(std::uint16_t x)
Ditto
> Source/WTF/wtf/LeadingZeroBitCount.h:86
> +namespace detail {
> +static constexpr std::uint8_t eightBitLeadingZeroBitCount[256] {8, 7, 6, 6,
5, 5,
> + 5, 5, 4, 4, 4, 4, 4, 4, 4, 4, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3,
3,
> + 3, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2,
2,
> + 2, 2, 2, 2, 2, 2, 2, 2, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,
1,
> + 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,
1,
> + 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 0,
0,
> + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
0,
> + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
0,
> + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
0,
> + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
0,
> + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
0};
> +}
Ditto.
> Source/WTF/wtf/LeadingZeroBitCount.h:88
> +inline constexpr int LeadingZeroBitCount(std::uint8_t x)
Ditto.
> Source/WTF/wtf/LeadingZeroBitCount.h:93
> +template <typename A> inline constexpr int BitsNeededFor(A x)
Rename it to bitsNeededFor.
> Tools/TestWebKitAPI/CMakeLists.txt:57
> + Tests/WTF/Int128.cpp
> Tests/WTF/IntegerToStringConversion.cpp
> Tests/WTF/IteratorRange.cpp
> Tests/WTF/JSONValue.cpp
> Tests/WTF/LEBDecoder.cpp
> + Tests/WTF/LeadingZeroBitCount.cpp
Add them to xcodeproj to run these tests on macOS / iOS too.
More information about the webkit-reviews
mailing list